mirror of https://github.com/zulip/zulip.git
refactor: Create a user object in report.
To make it easier to check if there is user information to be used in the error report emails, we create a user object inside report. Now, to check if we have the user's full name, email, etc, we just need to do report['user']['user_full_name'] rather than check each information one by one, because if the value of one key in the report is different than None, all the others will be as well.
This commit is contained in:
parent
ed7a6d5e4d
commit
b4dd118aa1
|
@ -23,8 +23,8 @@ def logger_repr(report: Dict[str, Any]) -> str:
|
|||
return "Logger {logger_name}, from module {log_module} line {log_lineno}:".format(**report)
|
||||
|
||||
def user_info_str(report: Dict[str, Any]) -> str:
|
||||
if report['user_full_name'] and report['user_email']:
|
||||
user_info = "{user_full_name} ({user_email})".format(**report)
|
||||
if report.get('user') and report['user'].get('user_full_name'):
|
||||
user_info = "{user[user_full_name]} <{user[user_email]}>".format(**report)
|
||||
else:
|
||||
user_info = "Anonymous user (not logged in)"
|
||||
|
||||
|
@ -44,10 +44,11 @@ def notify_browser_error(report: Dict[str, Any]) -> None:
|
|||
email_browser_error(report)
|
||||
|
||||
def email_browser_error(report: Dict[str, Any]) -> None:
|
||||
email_subject = f"Browser error for {user_info_str(report)}"
|
||||
user_info = user_info_str(report)
|
||||
email_subject = f"Browser error for {user_info}"
|
||||
body = f"User: {user_info}"
|
||||
|
||||
body = """\
|
||||
User: {user_full_name} <{user_email}> on {deployment}
|
||||
body += """\
|
||||
|
||||
Message:
|
||||
{message}
|
||||
|
|
|
@ -49,8 +49,11 @@ def add_request_metadata(report: Dict[str, Any], request: HttpRequest) -> None:
|
|||
traceback.print_exc()
|
||||
user_full_name = None
|
||||
user_email = None
|
||||
report['user_email'] = user_email
|
||||
report['user_full_name'] = user_full_name
|
||||
|
||||
report['user'] = {
|
||||
'user_email': user_email,
|
||||
'user_full_name': user_full_name,
|
||||
}
|
||||
|
||||
exception_filter = get_exception_reporter_filter(request)
|
||||
try:
|
||||
|
|
|
@ -115,7 +115,8 @@ class AdminNotifyHandlerTest(ZulipTestCase):
|
|||
record.msg = 'message\nmoremesssage\nmore'
|
||||
|
||||
report = self.run_handler(record)
|
||||
self.assertIn("user_email", report)
|
||||
self.assertIn("user", report)
|
||||
self.assertIn("user_email", report["user"])
|
||||
self.assertIn("message", report)
|
||||
self.assertIn("stack_trace", report)
|
||||
self.assertEqual(report['stack_trace'], 'message\nmoremesssage\nmore')
|
||||
|
@ -129,7 +130,8 @@ class AdminNotifyHandlerTest(ZulipTestCase):
|
|||
assert isinstance(record, HasRequest)
|
||||
|
||||
report = self.run_handler(record)
|
||||
self.assertIn("user_email", report)
|
||||
self.assertIn("user", report)
|
||||
self.assertIn("user_email", report["user"])
|
||||
self.assertIn("message", report)
|
||||
self.assertIn("stack_trace", report)
|
||||
|
||||
|
@ -138,7 +140,7 @@ class AdminNotifyHandlerTest(ZulipTestCase):
|
|||
with patch("zerver.logging_handlers.add_request_metadata",
|
||||
side_effect=Exception("Unexpected exception!")):
|
||||
report = self.run_handler(record)
|
||||
self.assertNotIn("user_email", report)
|
||||
self.assertNotIn("user", report)
|
||||
self.assertIn("message", report)
|
||||
self.assertEqual(report["stack_trace"], "See /var/log/zulip/errors.log")
|
||||
|
||||
|
@ -146,7 +148,8 @@ class AdminNotifyHandlerTest(ZulipTestCase):
|
|||
record.request.user = AnonymousUser()
|
||||
report = self.run_handler(record)
|
||||
self.assertIn("host", report)
|
||||
self.assertIn("user_email", report)
|
||||
self.assertIn("user", report)
|
||||
self.assertIn("user_email", report["user"])
|
||||
self.assertIn("message", report)
|
||||
self.assertIn("stack_trace", report)
|
||||
|
||||
|
@ -158,7 +161,8 @@ class AdminNotifyHandlerTest(ZulipTestCase):
|
|||
report = self.run_handler(record)
|
||||
record.request.get_host = orig_get_host
|
||||
self.assertIn("host", report)
|
||||
self.assertIn("user_email", report)
|
||||
self.assertIn("user", report)
|
||||
self.assertIn("user_email", report["user"])
|
||||
self.assertIn("message", report)
|
||||
self.assertIn("stack_trace", report)
|
||||
|
||||
|
@ -169,7 +173,8 @@ class AdminNotifyHandlerTest(ZulipTestCase):
|
|||
report = self.run_handler(record)
|
||||
record.request.method = "GET"
|
||||
self.assertIn("host", report)
|
||||
self.assertIn("user_email", report)
|
||||
self.assertIn("user", report)
|
||||
self.assertIn("user_email", report["user"])
|
||||
self.assertIn("message", report)
|
||||
self.assertIn("stack_trace", report)
|
||||
|
||||
|
@ -186,7 +191,8 @@ class AdminNotifyHandlerTest(ZulipTestCase):
|
|||
record.exc_info = None
|
||||
report = self.run_handler(record)
|
||||
self.assertIn("host", report)
|
||||
self.assertIn("user_email", report)
|
||||
self.assertIn("user", report)
|
||||
self.assertIn("user_email", report["user"])
|
||||
self.assertIn("message", report)
|
||||
self.assertEqual(report["stack_trace"], 'No stack trace available')
|
||||
|
||||
|
@ -195,7 +201,8 @@ class AdminNotifyHandlerTest(ZulipTestCase):
|
|||
with patch("zerver.logging_handlers.traceback.print_exc"):
|
||||
report = self.run_handler(record)
|
||||
self.assertIn("host", report)
|
||||
self.assertIn("user_email", report)
|
||||
self.assertIn("user", report)
|
||||
self.assertIn("user_email", report["user"])
|
||||
self.assertIn("message", report)
|
||||
self.assertIn("stack_trace", report)
|
||||
|
||||
|
|
Loading…
Reference in New Issue