On 05/10/2015 08:22 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire <[email protected]>
# Date 1427573281 -3600
#      Sat Mar 28 21:08:01 2015 +0100
# Node ID b7e259611340434bead43af0327e5be53275319b
# Parent  877fa67247ecfe9cb01a6b70bfaeee265bb65b9e
helpers: add automatic logging to Flash

"add automatic logging of Flash messages shown to users"?

Log all messages displayed through a flash to the user.
The log level equals the flash category if it is defined, or 'info'
otherwise.

Subsequent patches can change the log level for individual flash calls to
make the log more useful.

diff --git a/kallithea/lib/helpers.py b/kallithea/lib/helpers.py
--- a/kallithea/lib/helpers.py
+++ b/kallithea/lib/helpers.py
@@ -406,6 +406,43 @@ class _Message(object):
class Flash(_Flash): + def __call__(self, message, category=None, ignore_duplicate=False, log_category=None):

Perhaps just make category default to 'info' and avoid special handling of None? Or do we explicitly and for good reasons pass None and need handling of that?

+        '''Show a message to the user

"and make sure it also is logged if debug is enabled" ... or how would you describe the intended functionality?

+        category: notice (default), warning, error, success
+        log_category: info, warning, error, debug
+
+        If log_category is not set, the following mapping applies:
+
+        category | log_category
+        ---------+-------------
+         None    | info
+         notice  | info
+         warning | info
+         error   | error
+         success | debug

From another point of view: What is relevant info to the user is not necessarily relevant for the admin to get in the log, unless debug logging is enabled. User errors are also not application errors and should thus not be logged as errors. Thus, all these flash messages should always use debug logging (as hinted in my proposed function description above) ... or at least default to it.

BUT I think I support your point of view. Some thoughts on this "issue" can perhaps be added to the commit message.

(Nice docstring!)

+
+        '''
+        log_function = {
+            'debug': log.debug,
+            'info': log.info,
+            'warning': log.warning,
+            'error': log.error,
+        }
+
+        if not log_category:

if log_category is None

+            transform = {
+                None: 'info',
+                'notice': 'info',
+                'warning': 'info',
+                'error': 'error',
+                'success': 'debug',
+            }
+            log_category = transform[category]
+
+        log_function[log_category]('Flash: %s' % message)

I would skip the naming of the log_function and transform dicts and just use them directly. I think that would be more readable. (If not, it could be argued that these dicts are consts and should be defined outside the function.)

+        super(Flash, self).__call__(message, category, ignore_duplicate)
+
      def pop_messages(self):
          """Return all accumulated messages and delete them from the session.

/Mads
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Reply via email to