From d40fa6ddbcff73e1b4d063f6356a07cd321e2125 Mon Sep 17 00:00:00 2001 From: Anna Gavrilman Date: Wed, 8 Mar 2023 15:19:38 +0100 Subject: [PATCH] Dashboard: Adding "You passed this course" message with no certificates (#5310) --- dashboard/api.py | 7 +++-- dashboard/api_test.py | 16 ++++++----- dashboard/utils.py | 28 +++++++++++++++++-- dashboard/utils_test.py | 12 ++++---- exams/api.py | 2 +- exams/api_test.py | 10 +++---- .../management/commands/alter_data_test.py | 4 +-- .../CouponNotificationDialog_test.js | 3 +- .../dashboard/courses/StatusMessages.js | 13 ++++----- .../dashboard/courses/StatusMessages_test.js | 5 ++-- static/js/factories/dashboard.js | 3 +- static/js/flow/programTypes.js | 1 + 12 files changed, 65 insertions(+), 39 deletions(-) diff --git a/dashboard/api.py b/dashboard/api.py index 4f76ff7567..f3fa8ccb6a 100644 --- a/dashboard/api.py +++ b/dashboard/api.py @@ -247,7 +247,8 @@ def get_info_for_course(course, mmtrack): "is_elective": ElectiveCourse.objects.filter(course=course).exists(), "has_exam": course.has_exam, "certificate_url": get_certificate_url(mmtrack, course), - "overall_grade": mmtrack.get_overall_final_grade_for_course(course) + "overall_grade": mmtrack.get_overall_final_grade_for_course(course), + "is_passed": mmtrack.has_passed_course(course) } def _add_run(run, mmtrack_, status): @@ -303,7 +304,7 @@ def _add_run(run, mmtrack_, status): _add_run(run_status.course_run, mmtrack, CourseStatus.CURRENTLY_ENROLLED) # check if we need to check the certificate elif run_status.status == CourseRunStatus.CHECK_IF_PASSED: - if not mmtrack.has_passed_course(run_status.course_run.edx_course_key): + if not mmtrack.has_passed_course_run(run_status.course_run.edx_course_key): _add_run(run_status.course_run, mmtrack, CourseRunStatus.NOT_PASSED) else: _add_run(run_status.course_run, mmtrack, CourseStatus.PASSED) @@ -329,7 +330,7 @@ def _add_run(run, mmtrack_, status): # the first one (or two in some cases) has been added with the logic before for run_status in run_statuses: if run_status.status == CourseRunStatus.CHECK_IF_PASSED: - if mmtrack.has_passed_course(run_status.course_run.edx_course_key): + if mmtrack.has_passed_course_run(run_status.course_run.edx_course_key): # in this case the user might have passed the course also in the past _add_run(run_status.course_run, mmtrack, CourseStatus.PASSED) else: diff --git a/dashboard/api_test.py b/dashboard/api_test.py index 972e75dc6a..cf4f6d2284 100644 --- a/dashboard/api_test.py +++ b/dashboard/api_test.py @@ -901,6 +901,7 @@ def setUp(self): # default behavior for some mmtrack mocked methods self.mmtrack.get_course_proctorate_exam_results.return_value = [] self.mmtrack.get_overall_final_grade_for_course.return_value = "" + self.mmtrack.has_passed_course.return_value = False def assert_course_equal( self, @@ -915,7 +916,7 @@ def assert_course_equal( has_to_pay=False, has_exam=False, is_elective=False, - proct_exams=None + proct_exams=None, ): """Helper to format the course info""" proct_exams = proct_exams or [] @@ -939,6 +940,7 @@ def assert_course_equal( "has_exam": has_exam, "certificate_url": "", "overall_grade": "", + "is_passed": False } # remove the runs part: assumed checked with the mock assertion del course_data_from_call['runs'] @@ -1142,7 +1144,7 @@ def test_info_not_enrolled_not_passed_not_offered( self, mock_schedulable, mock_format, mock_get_cert, mock_future_exams, mock_has_to_pay, mock_exam_course_key): """test for get_info_for_course for course with run not passed and nothing offered""" self.mmtrack.configure_mock(**{ - 'has_passed_course.return_value': False, + 'has_passed_course_run.return_value': False, 'is_enrolled_mmtrack.return_value': True }) with patch( @@ -1169,7 +1171,7 @@ def test_info_grade( self, mock_schedulable, mock_format, mock_get_cert, mock_future_exams, mock_has_to_pay, mock_exam_course_key): """test for get_info_for_course for course with a course current and another not passed""" self.mmtrack.configure_mock(**{ - 'has_passed_course.return_value': False, + 'has_passed_course_run.return_value': False, 'is_enrolled_mmtrack.return_value': True }) with patch( @@ -1198,7 +1200,7 @@ def test_info_check_but_not_passed( test for get_info_for_course in case a check if the course has been passed is required """ self.mmtrack.configure_mock(**{ - 'has_passed_course.return_value': False, + 'has_passed_course_run.return_value': False, 'is_enrolled_mmtrack.return_value': True }) with patch( @@ -1255,7 +1257,7 @@ def test_info_check_but_not_passed_no_next( test for get_info_for_course in case a check if the course has been passed is required for the course, the course has not been passed and there is no next run """ - self.mmtrack.configure_mock(**{'has_passed_course.return_value': False}) + self.mmtrack.configure_mock(**{'has_passed_course_run.return_value': False}) with patch( 'dashboard.api.get_status_for_courserun', autospec=True, @@ -1283,7 +1285,7 @@ def test_info_check_passed( is required for the course and the course has been passed """ self.mmtrack.configure_mock(**{ - 'has_passed_course.return_value': True, + 'has_passed_course_run.return_value': True, 'is_enrolled_mmtrack.return_value': True }) with patch( @@ -1433,7 +1435,7 @@ def test_info_read_cert_for_all_no_next( test for get_info_for_course in case the less recent course is flagged to be checked if passed """ self.mmtrack.configure_mock(**{ - 'has_passed_course.return_value': True, + 'has_passed_course_run.return_value': True, 'is_enrolled_mmtrack.return_value': True }) with patch( diff --git a/dashboard/utils.py b/dashboard/utils.py index 5dd85dd73a..aea29620bf 100644 --- a/dashboard/utils.py +++ b/dashboard/utils.py @@ -358,18 +358,42 @@ def paid_but_missed_deadline(self, course_run): return True return False - def has_passed_course(self, edx_course_key): + def has_passed_course_run(self, edx_course_key): """ Returns whether the user has passed a course run. Args: edx_course_key (str): an edX course run key Returns: - bool: whether the user has passed the course + bool: whether the user has passed the course_run """ final_grade = self.get_final_grade(edx_course_key) return final_grade.passed if final_grade else False + def has_passed_course(self, course): + """ + Returns true if the user has passed this course overall + Args: + course (Course): course instance + + Returns: + bool: whether the user has passed the course + """ + if self.has_exams: + course_cert = MicromastersCourseCertificate.objects.filter( + user=self.user, + course_id=course.id).exists() + if course_cert: + return True + else: + return FinalGrade.objects.filter( + user=self.user, + course_run__course_id=course.id, + course_run__start_date__gt=datetime.datetime(2022, 9, 1, tzinfo=pytz.UTC), + ).passed().exists() + else: + return self.final_grade_qset.filter(course_run__course_id=course.id).passed().exists() + def get_final_grade_percent(self, edx_course_key): """ Returns the course final grade number for the user if she passed. diff --git a/dashboard/utils_test.py b/dashboard/utils_test.py index 112994eeff..6d6604c14b 100644 --- a/dashboard/utils_test.py +++ b/dashboard/utils_test.py @@ -259,9 +259,9 @@ def test_is_enrolled_mmtrack_fa(self): assert mmtrack.is_enrolled_mmtrack(course_id) is True @ddt.data(True, False) - def test_has_passed_course(self, final_grade_passed): + def test_has_passed_course_run(self, final_grade_passed): """ - Test that has_passed_course returns True when a passed FinalGrade exists + Test that has_passed_course_run returns True when a passed FinalGrade exists """ final_grade = FinalGradeFactory.create( user=self.user, @@ -273,18 +273,18 @@ def test_has_passed_course(self, final_grade_passed): program=self.program, edx_user_data=self.cached_edx_user_data ) - assert mmtrack.has_passed_course(final_grade.course_run.edx_course_key) is final_grade_passed + assert mmtrack.has_passed_course_run(final_grade.course_run.edx_course_key) is final_grade_passed - def test_has_passed_course_no_grade(self): + def test_has_passed_course_run_no_grade(self): """ - Test that has_passed_course returns False when no FinalGrade exists + Test that has_passed_course_run returns False when no FinalGrade exists """ mmtrack = MMTrack( user=self.user, program=self.program, edx_user_data=self.cached_edx_user_data ) - assert mmtrack.has_passed_course('random-course-id') is False + assert mmtrack.has_passed_course_run('random-course-id') is False def test_get_final_grade_percent(self): """ diff --git a/exams/api.py b/exams/api.py index 7a8485674d..a9a2281aea 100644 --- a/exams/api.py +++ b/exams/api.py @@ -73,7 +73,7 @@ def authorize_for_exam_run(user, course_run, exam_run): ExamProfile.objects.get_or_create(profile=mmtrack.user.profile) # if they didn't pass, they don't get authorized - if not mmtrack.has_passed_course(course_run.edx_course_key): + if not mmtrack.has_passed_course_run(course_run.edx_course_key): errors_message = MESSAGE_NOT_PASSED_OR_EXIST_TEMPLATE.format( user=mmtrack.user.username, course_id=course_run.edx_course_key diff --git a/exams/api_test.py b/exams/api_test.py index 7a48666152..ff0d330608 100644 --- a/exams/api_test.py +++ b/exams/api_test.py @@ -97,7 +97,7 @@ def test_exam_authorization_for_inactive_user(self): create_order(user, self.course_run) mmtrack = get_mmtrack(user, self.program) self.assertTrue(mmtrack.has_paid(self.course_run.edx_course_key)) - self.assertTrue(mmtrack.has_passed_course(self.course_run.edx_course_key)) + self.assertTrue(mmtrack.has_passed_course_run(self.course_run.edx_course_key)) # Neither user has exam profile nor authorization. assert ExamProfile.objects.filter(profile=mmtrack.user.profile).exists() is False @@ -123,7 +123,7 @@ def test_exam_authorization(self): create_order(self.user, self.course_run) mmtrack = get_mmtrack(self.user, self.program) self.assertTrue(mmtrack.has_paid(self.course_run.edx_course_key)) - self.assertTrue(mmtrack.has_passed_course(self.course_run.edx_course_key)) + self.assertTrue(mmtrack.has_passed_course_run(self.course_run.edx_course_key)) # Neither user has exam profile nor authorization. assert ExamProfile.objects.filter(profile=mmtrack.user.profile).exists() is False @@ -148,7 +148,7 @@ def test_exam_authorization_attempts_consumed(self): create_order(self.user, self.course_run) mmtrack = get_mmtrack(self.user, self.program) self.assertTrue(mmtrack.has_paid(self.course_run.edx_course_key)) - self.assertTrue(mmtrack.has_passed_course(self.course_run.edx_course_key)) + self.assertTrue(mmtrack.has_passed_course_run(self.course_run.edx_course_key)) old_run = ExamRunFactory.create(course=self.course_run.course) ExamAuthorizationFactory.create_batch( ATTEMPTS_PER_PAID_RUN_OLD, @@ -273,14 +273,14 @@ def test_exam_authorization_when_not_passed_course(self): test exam_authorization when user has not passed course but paid. """ create_order(self.user, self.course_run) - with patch('dashboard.utils.MMTrack.has_passed_course', autospec=True, return_value=False): + with patch('dashboard.utils.MMTrack.has_passed_course_run', autospec=True, return_value=False): mmtrack = get_mmtrack(self.user, self.program) expected_errors_message = MESSAGE_NOT_PASSED_OR_EXIST_TEMPLATE.format( user=mmtrack.user.username, course_id=self.course_run.edx_course_key ) assert mmtrack.has_paid(self.course_run.edx_course_key) is True - assert mmtrack.has_passed_course(self.course_run.edx_course_key) is False + assert mmtrack.has_passed_course_run(self.course_run.edx_course_key) is False # Neither user has exam profile nor authorization. assert ExamProfile.objects.filter(profile=mmtrack.user.profile).exists() is False diff --git a/seed_data/management/commands/alter_data_test.py b/seed_data/management/commands/alter_data_test.py index d6e5d05cbe..b739cc6d20 100644 --- a/seed_data/management/commands/alter_data_test.py +++ b/seed_data/management/commands/alter_data_test.py @@ -35,7 +35,7 @@ def test_set_to_passed(self, course_run_program_type): grade = Decimal('0.75') set_to_passed(user=self.user, course_run=course_run, grade=grade) mmtrack = get_mmtrack(self.user, course_run.course.program) - assert mmtrack.has_passed_course(course_run.edx_course_key) + assert mmtrack.has_passed_course_run(course_run.edx_course_key) assert int(mmtrack.get_final_grade_percent(course_run.edx_course_key)) == (grade * 100) @ddt.data('fa', 'non_fa') @@ -45,7 +45,7 @@ def test_set_to_failed(self, course_run_program_type): grade = Decimal('0.55') set_to_failed(user=self.user, course_run=course_run, grade=grade) mmtrack = get_mmtrack(self.user, course_run.course.program) - assert not mmtrack.has_passed_course(course_run.edx_course_key) + assert not mmtrack.has_passed_course_run(course_run.edx_course_key) assert int(mmtrack.get_final_grade_percent(course_run.edx_course_key)) == (grade * 100) @ddt.data('fa', 'non_fa') diff --git a/static/js/components/CouponNotificationDialog_test.js b/static/js/components/CouponNotificationDialog_test.js index 0e365e0063..17b4e4f3c9 100644 --- a/static/js/components/CouponNotificationDialog_test.js +++ b/static/js/components/CouponNotificationDialog_test.js @@ -78,7 +78,8 @@ const COURSE: Course = { proctorate_exams_grades: [], is_elective: false, certificate_url: "", - overall_grade: "" + overall_grade: "", + is_passed: false } const PROGRAM: AvailableProgram = { diff --git a/static/js/components/dashboard/courses/StatusMessages.js b/static/js/components/dashboard/courses/StatusMessages.js index a2cc85e2ba..4616df06e3 100644 --- a/static/js/components/dashboard/courses/StatusMessages.js +++ b/static/js/components/dashboard/courses/StatusMessages.js @@ -34,7 +34,7 @@ import { hasCanUpgradeCourseRun, hasMissedDeadlineCourseRun } from "./util" -import { hasPassingExamGrade, hasPassedCourseRun } from "../../../lib/grades" +import { hasPassedCourseRun } from "../../../lib/grades" import { formatPrettyDateTimeAmPmTz, parseDateString } from "../../../util/date" type Message = { @@ -113,7 +113,6 @@ export const calculateMessages = (props: CalculateMessagesProps) => { const exams = course.has_exam const paid = firstRun.has_paid - const passedExam = hasPassingExamGrade(course) const paymentDueDate = moment( R.defaultTo("", firstRun.course_upgrade_deadline) ) @@ -200,12 +199,10 @@ export const calculateMessages = (props: CalculateMessagesProps) => { }) } // course is passed and paid but no certificate yet - if (firstRun["status"] === "passed" && paid && !course.certificate_url) { - if (!exams || (exams && passedExam)) { - messages.push({ - message: "You passed this course." - }) - } + if (!course.certificate_url && course.is_passed) { + messages.push({ + message: "You passed this course." + }) } // Course is running, user has already paid, if ( diff --git a/static/js/components/dashboard/courses/StatusMessages_test.js b/static/js/components/dashboard/courses/StatusMessages_test.js index accc7e221f..a4bd2bc04d 100644 --- a/static/js/components/dashboard/courses/StatusMessages_test.js +++ b/static/js/components/dashboard/courses/StatusMessages_test.js @@ -307,9 +307,7 @@ describe("Course Status Messages", () => { it("should prompt when passed the course", () => { course.has_exam = true - course.can_schedule_exam = true - course.proctorate_exams_grades = [makeProctoredExamResult()] - course.proctorate_exams_grades[0].passed = true + course.is_passed = true const messages = calculateMessages(calculateMessagesProps).value assert.equal(messages[0]["message"], "You passed this course.") @@ -320,6 +318,7 @@ describe("Course Status Messages", () => { makeRunPast(course.runs[0]) makeRunPassed(course.runs[0]) makeRunPaid(course.runs[0]) + course.is_passed = true assertIsJust(calculateMessages(calculateMessagesProps), [ { message: "You passed this course." diff --git a/static/js/factories/dashboard.js b/static/js/factories/dashboard.js index 45d8819e20..97727c84d2 100644 --- a/static/js/factories/dashboard.js +++ b/static/js/factories/dashboard.js @@ -120,7 +120,8 @@ export const makeCourse = (positionInProgram: number): Course => { is_elective: false, proctorate_exams_grades: [], certificate_url: "", - overall_grade: "" + overall_grade: "", + is_passed: false } } diff --git a/static/js/flow/programTypes.js b/static/js/flow/programTypes.js index 6d390ff350..09c9d1644d 100644 --- a/static/js/flow/programTypes.js +++ b/static/js/flow/programTypes.js @@ -73,6 +73,7 @@ export type Course = { has_exam: boolean, certificate_url: string, overall_grade: string, + is_passed: boolean, } export type ProgramPageCourse = {