From 5b7677e44ca4dc2e20163160235b52c01d882ef0 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 22 Apr 2024 22:37:40 +0200 Subject: [PATCH 1/6] fix: reraise exceptions when debugging --- evap/evaluation/models.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index befc963aa..f3cdaddeb 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -958,6 +958,8 @@ def update_evaluations(cls): evaluation_results_evaluations.append(evaluation) evaluation.save() except Exception: # noqa: PERF203 + if settings.DEBUG: + raise logger.exception( 'An error occured when updating the state of evaluation "%s" (id %d).', evaluation, evaluation.id ) @@ -2109,6 +2111,8 @@ def send_to_user(self, user, subject_params, body_params, use_cc, additional_cc_ if send_separate_login_url: self.send_login_url_to_user(user) except Exception: + if settings.DEBUG: + raise logger.exception( 'An exception occurred when sending the following email to user "%s":\n%s\n', user.full_name_with_additional_info, From d934ff7f5f33487a197167cdf4edbab5f8483aa9 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 22 Apr 2024 22:40:10 +0200 Subject: [PATCH 2/6] translate message before formatting --- evap/evaluation/models.py | 10 +++++----- pyproject.toml | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index f3cdaddeb..79a1f9c8a 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -2061,17 +2061,17 @@ def send_to_users_in_evaluations(self, evaluations, recipient_groups, use_cc, re def send_to_user(self, user, subject_params, body_params, use_cc, additional_cc_users=(), request=None): if not user.email: - warning_message = ( - f"{user.full_name_with_additional_info} has no email address defined. Could not send email." + message = _("{} has no email address defined. Could not send email.").format( + user.full_name_with_additional_info ) # If this method is triggered by a cronjob changing evaluation states, the request is None. # In this case warnings should be sent to the admins via email (configured in the settings for logger.error). # If a request exists, the page is displayed in the browser and the message can be shown on the page (messages.warning). if request is not None: - logger.warning(warning_message) - messages.warning(request, _(warning_message)) + logger.warning(message) + messages.warning(request, message) else: - logger.error(warning_message) + logger.error(message) return cc_users = set(additional_cc_users) diff --git a/pyproject.toml b/pyproject.toml index 65206995f..13862533b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,7 @@ ignore = [ "COM812", # incompatible with formatter "N802", # not as mighty as pylint's invalid-name https://github.com/astral-sh/ruff/issues/7660 "PLR0913", # we can't determine a good limit for arguments. reviews should spot bad cases of this. - "PLR2004", "PLW2901" + "PLR2004", "PLW2901", "PLR0912" ] ignore-init-module-imports = true From 37bb82332b5970d4e01c471f1ece740745026aaf Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Apr 2024 22:21:06 +0200 Subject: [PATCH 3/6] use gettext_noop and extract level --- evap/evaluation/models.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 79a1f9c8a..3b218f0f8 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -27,7 +27,7 @@ from django.utils.functional import cached_property from django.utils.safestring import SafeData from django.utils.timezone import now -from django.utils.translation import gettext_lazy as _ +from django.utils.translation import gettext_lazy as _, gettext_noop from django_fsm import FSMIntegerField, transition from django_fsm.signals import post_transition @@ -2061,17 +2061,15 @@ def send_to_users_in_evaluations(self, evaluations, recipient_groups, use_cc, re def send_to_user(self, user, subject_params, body_params, use_cc, additional_cc_users=(), request=None): if not user.email: - message = _("{} has no email address defined. Could not send email.").format( - user.full_name_with_additional_info - ) + message = gettext_noop("{} has no email address defined. Could not send email.") + level = logging.ERROR # If this method is triggered by a cronjob changing evaluation states, the request is None. # In this case warnings should be sent to the admins via email (configured in the settings for logger.error). # If a request exists, the page is displayed in the browser and the message can be shown on the page (messages.warning). if request is not None: - logger.warning(message) - messages.warning(request, message) - else: - logger.error(message) + level = logging.WARNING + messages.warning(request, _(message).format(user.full_name_with_additional_info)) + logger.log(level, message.format(user.full_name_with_additional_info)) return cc_users = set(additional_cc_users) From 87c6de578b01931acff24f030d424bdd8aa051e2 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Apr 2024 22:22:30 +0200 Subject: [PATCH 4/6] use if-else construct --- evap/evaluation/models.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 3b218f0f8..5b0c27158 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -2062,14 +2062,15 @@ def send_to_users_in_evaluations(self, evaluations, recipient_groups, use_cc, re def send_to_user(self, user, subject_params, body_params, use_cc, additional_cc_users=(), request=None): if not user.email: message = gettext_noop("{} has no email address defined. Could not send email.") - level = logging.ERROR + log_message = message.format(user.full_name_with_additional_info) # If this method is triggered by a cronjob changing evaluation states, the request is None. # In this case warnings should be sent to the admins via email (configured in the settings for logger.error). # If a request exists, the page is displayed in the browser and the message can be shown on the page (messages.warning). if request is not None: - level = logging.WARNING + logger.warning(log_message) messages.warning(request, _(message).format(user.full_name_with_additional_info)) - logger.log(level, message.format(user.full_name_with_additional_info)) + else: + logger.error(log_message) return cc_users = set(additional_cc_users) From 706526084345991fac53c28dc21bb573ac1fc96a Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Apr 2024 22:26:43 +0200 Subject: [PATCH 5/6] reformat isort --- evap/evaluation/models.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 5b0c27158..a2895f2ca 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -27,7 +27,8 @@ from django.utils.functional import cached_property from django.utils.safestring import SafeData from django.utils.timezone import now -from django.utils.translation import gettext_lazy as _, gettext_noop +from django.utils.translation import gettext_lazy as _ +from django.utils.translation import gettext_noop from django_fsm import FSMIntegerField, transition from django_fsm.signals import post_transition From 540dc52dc71bbd618ff1bfb91b170669d7924202 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 6 May 2024 20:23:13 +0200 Subject: [PATCH 6/6] disable PLR0912 --- evap/staff/tools.py | 2 +- pyproject.toml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/evap/staff/tools.py b/evap/staff/tools.py index ac289daa5..5bdf8822e 100644 --- a/evap/staff/tools.py +++ b/evap/staff/tools.py @@ -100,7 +100,7 @@ def find_matching_internal_user_for_email(request, email): return matching_users[0] -def bulk_update_users(request, user_file_content, test_run): # noqa: PLR0912 +def bulk_update_users(request, user_file_content, test_run): # pylint: disable=too-many-branches,too-many-locals # user_file must have one user per line in the format "{username},{email}" imported_emails = {clean_email(line.decode().split(",")[1]) for line in user_file_content.splitlines()} diff --git a/pyproject.toml b/pyproject.toml index 13862533b..9654ad6ff 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,8 @@ ignore = [ "COM812", # incompatible with formatter "N802", # not as mighty as pylint's invalid-name https://github.com/astral-sh/ruff/issues/7660 "PLR0913", # we can't determine a good limit for arguments. reviews should spot bad cases of this. - "PLR2004", "PLW2901", "PLR0912" + "PLR2004", "PLW2901", + "PLR0912", # sevaral ruff bugs: https://www.github.com/astral-sh/ruff/issues/11313, https://www.github.com/astral-sh/ruff/issues/11205 ] ignore-init-module-imports = true