From 649bd42f9cba34adcb82abbd9f20eef247296169 Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Tue, 24 Sep 2024 02:31:20 -0700 Subject: [PATCH 1/2] fix: updated edx.ace.message_sent event (#35498) * fix: updated edx.ace.message_sent event * fix: fixed pylint checks --- lms/djangoapps/bulk_email/signals.py | 30 ++++++++++++---------------- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 17 insertions(+), 21 deletions(-) diff --git a/lms/djangoapps/bulk_email/signals.py b/lms/djangoapps/bulk_email/signals.py index 9f6540651eeb..fb8749bf45a9 100644 --- a/lms/djangoapps/bulk_email/signals.py +++ b/lms/djangoapps/bulk_email/signals.py @@ -1,7 +1,6 @@ """ Signal handlers for the bulk_email app """ -from django.contrib.auth import get_user_model from django.dispatch import receiver from eventtracking import tracker @@ -32,29 +31,26 @@ def ace_email_sent_handler(sender, **kwargs): """ When an email is sent using ACE, this method will create an event to detect ace email success status """ - # Fetch the message object from kwargs, defaulting to None if not present - message = kwargs.get('message', None) - - user_model = get_user_model() - try: - user_id = user_model.objects.get(email=message.recipient.email_address).id - except user_model.DoesNotExist: - user_id = None - course_email = message.context.get('course_email', None) - course_id = message.context.get('course_id') + # Fetch the message dictionary from kwargs, defaulting to {} if not present + message = kwargs.get('message', {}) + recipient = message.get('recipient', {}) + message_name = message.get('name', None) + context = message.get('context', {}) + email_address = recipient.get('email', None) + user_id = recipient.get('user_id', None) + channel = message.get('channel', None) + course_id = context.get('course_id', None) if not course_id: + course_email = context.get('course_email', None) course_id = course_email.course_id if course_email else None - try: - channel = sender.__class__.__name__ - except AttributeError: - channel = 'Other' + tracker.emit( 'edx.ace.message_sent', { - 'message_type': message.name, + 'message_type': message_name, 'channel': channel, 'course_id': course_id, 'user_id': user_id, - 'user_email': message.recipient.email_address, + 'user_email': email_address, } ) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 4a65af081ce4..0b246816f559 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -401,7 +401,7 @@ drf-yasg==1.21.7 # via # django-user-tasks # edx-api-doc-tools -edx-ace==1.11.1 +edx-ace==1.11.2 # via -r requirements/edx/kernel.in edx-api-doc-tools==1.8.0 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index c5db40448d94..e16f83b049f7 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -657,7 +657,7 @@ drf-yasg==1.21.7 # -r requirements/edx/testing.txt # django-user-tasks # edx-api-doc-tools -edx-ace==1.11.1 +edx-ace==1.11.2 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index ade1d06afbfe..2f916b40c535 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -481,7 +481,7 @@ drf-yasg==1.21.7 # -r requirements/edx/base.txt # django-user-tasks # edx-api-doc-tools -edx-ace==1.11.1 +edx-ace==1.11.2 # via -r requirements/edx/base.txt edx-api-doc-tools==1.8.0 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 527c82dce45a..93a0313a1123 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -505,7 +505,7 @@ drf-yasg==1.21.7 # -r requirements/edx/base.txt # django-user-tasks # edx-api-doc-tools -edx-ace==1.11.1 +edx-ace==1.11.2 # via -r requirements/edx/base.txt edx-api-doc-tools==1.8.0 # via From 84d2ad95151d11960d76fb97bf484a070d6dfc5a Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Mon, 31 May 2021 17:45:03 +0200 Subject: [PATCH 2/2] fix: increase grades rounding precision Enabling the rounding in #16837 has been causing noticeable (up to 1 percentage point) differences between non-rounded subsection grades and a total grade for a course. This increases the grade precision to reduce the negative implications of double rounding. --- lms/djangoapps/grades/rest_api/v1/tests/test_views.py | 8 ++++---- lms/djangoapps/grades/scores.py | 4 ++-- lms/djangoapps/grades/tests/test_course_grade_factory.py | 8 ++++---- lms/djangoapps/grades/tests/test_subsection_grade.py | 2 +- xmodule/graders.py | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lms/djangoapps/grades/rest_api/v1/tests/test_views.py b/lms/djangoapps/grades/rest_api/v1/tests/test_views.py index cd2107ec7c29..656e7e6b4396 100644 --- a/lms/djangoapps/grades/rest_api/v1/tests/test_views.py +++ b/lms/djangoapps/grades/rest_api/v1/tests/test_views.py @@ -302,7 +302,7 @@ def setUpClass(cls): + [ { 'category': 'Homework', - 'detail': 'Homework Average = 0%', + 'detail': 'Homework Average = 0.00%', 'label': 'HW Avg', 'percent': 0.0, 'prominent': True } @@ -332,21 +332,21 @@ def setUpClass(cls): }, { 'category': 'Lab', - 'detail': 'Lab Average = 0%', + 'detail': 'Lab Average = 0.00%', 'label': 'Lab Avg', 'percent': 0.0, 'prominent': True }, { 'category': 'Midterm Exam', - 'detail': 'Midterm Exam = 0%', + 'detail': 'Midterm Exam = 0.00%', 'label': 'Midterm', 'percent': 0.0, 'prominent': True }, { 'category': 'Final Exam', - 'detail': 'Final Exam = 0%', + 'detail': 'Final Exam = 0.00%', 'label': 'Final', 'percent': 0.0, 'prominent': True diff --git a/lms/djangoapps/grades/scores.py b/lms/djangoapps/grades/scores.py index f621d85ea17b..38dd0dc18926 100644 --- a/lms/djangoapps/grades/scores.py +++ b/lms/djangoapps/grades/scores.py @@ -162,8 +162,8 @@ def compute_percent(earned, possible): Returns the percentage of the given earned and possible values. """ if possible > 0: - # Rounds to two decimal places. - return around(earned / possible, decimals=2) + # Rounds to four decimal places. + return around(earned / possible, decimals=4) else: return 0.0 diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 4e3bcde0ad95..2066b6cd0d7c 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -185,26 +185,26 @@ def test_course_grade_summary(self): 'section_breakdown': [ { 'category': 'Homework', - 'detail': 'Homework 1 - Test Sequential X with an & Ampersand - 50% (1/2)', + 'detail': 'Homework 1 - Test Sequential X with an & Ampersand - 50.00% (1/2)', 'label': 'HW 01', 'percent': 0.5 }, { 'category': 'Homework', - 'detail': 'Homework 2 - Test Sequential A - 0% (0/1)', + 'detail': 'Homework 2 - Test Sequential A - 0.00% (0/1)', 'label': 'HW 02', 'percent': 0.0 }, { 'category': 'Homework', - 'detail': 'Homework Average = 25%', + 'detail': 'Homework Average = 25.00%', 'label': 'HW Avg', 'percent': 0.25, 'prominent': True }, { 'category': 'NoCredit', - 'detail': 'NoCredit Average = 0%', + 'detail': 'NoCredit Average = 0.00%', 'label': 'NC Avg', 'percent': 0, 'prominent': True diff --git a/lms/djangoapps/grades/tests/test_subsection_grade.py b/lms/djangoapps/grades/tests/test_subsection_grade.py index 7dd39af4ece2..2398e7a71000 100644 --- a/lms/djangoapps/grades/tests/test_subsection_grade.py +++ b/lms/djangoapps/grades/tests/test_subsection_grade.py @@ -14,7 +14,7 @@ @ddt class SubsectionGradeTest(GradeTestBase): # lint-amnesty, pylint: disable=missing-class-docstring - @data((50, 100, .50), (59.49, 100, .59), (59.51, 100, .60), (59.50, 100, .60), (60.5, 100, .60)) + @data((50, 100, .5), (.5949, 100, .0059), (.5951, 100, .006), (.595, 100, .0059), (.605, 100, .006)) @unpack def test_create_and_read(self, mock_earned, mock_possible, expected_result): with mock_get_score(mock_earned, mock_possible): diff --git a/xmodule/graders.py b/xmodule/graders.py index a587204d682e..5261b9f4479a 100644 --- a/xmodule/graders.py +++ b/xmodule/graders.py @@ -387,7 +387,7 @@ def grade(self, grade_sheet, generate_random_scores=False): section_name = scores[i].display_name percentage = scores[i].percent_graded - summary_format = "{section_type} {index} - {name} - {percent:.0%} ({earned:.3n}/{possible:.3n})" + summary_format = "{section_type} {index} - {name} - {percent:.2%} ({earned:.3n}/{possible:.3n})" summary = summary_format.format( index=i + self.starting_index, section_type=self.section_type, @@ -421,7 +421,7 @@ def grade(self, grade_sheet, generate_random_scores=False): if len(breakdown) == 1: # if there is only one entry in a section, suppress the existing individual entry and the average, # and just display a single entry for the section. - total_detail = "{section_type} = {percent:.0%}".format( + total_detail = "{section_type} = {percent:.2%}".format( percent=total_percent, section_type=self.section_type, ) @@ -430,7 +430,7 @@ def grade(self, grade_sheet, generate_random_scores=False): 'detail': total_detail, 'category': self.category, 'prominent': True}, ] else: # Translators: "Homework Average = 0%" - total_detail = _("{section_type} Average = {percent:.0%}").format( + total_detail = _("{section_type} Average = {percent:.2%}").format( percent=total_percent, section_type=self.section_type )