Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: have progress MFE API recalculate visible course grade

Browse Source 
Previously, the course grade returned would be the stored grade,
which is calculated for all content, not just the visible grades.
(Some grades are not yet released to the learner.)

This fix recalculates the overall grade before sending to the MFE,
so that it doesn't have to recompute it itself (without all the
particular logic that the platform uses when grading).

AA-1217
This commit is contained in:
Michael Terry
2022-03-11 09:01:13 -05:00
parent c32d3b2762
commit a162140492
6 changed files with 81 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
lms/djangoapps/course_home_api/progress/tests/test_views.py
View File

@@ -3,15 +3,16 @@ Tests for Progress Tab API in the Course Home API
"""
from datetime import datetime, timedelta
from unittest.mock import patch
import dateutil
import ddt
from django.conf import settings
from pytz import UTC
from unittest.mock import patch # lint-amnesty, pylint: disable=wrong-import-order
from django.urls import reverse
from django.utils.timezone import now
from edx_toggles.toggles.testutils import override_waffle_flag
from pytz import UTC
from xmodule.modulestore.tests.factories import ItemFactory
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
@@ -19,22 +20,25 @@ from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests
from lms.djangoapps.course_home_api.models import DisableProgressPageStackedConfig
from lms.djangoapps.course_home_api.toggles import COURSE_HOME_MICROFRONTEND_PROGRESS_TAB
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags
from lms.djangoapps.grades.constants import GradeOverrideFeatureEnum
from lms.djangoapps.grades.models import (
PersistentSubsectionGrade,
PersistentSubsectionGradeOverride
)
from lms.djangoapps.grades.tests.utils import answer_problem
from lms.djangoapps.verify_student.models import ManualVerification
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.course_date_signals.utils import MIN_DURATION
from openedx.core.djangolib.testing.utils import get_mock_request
from openedx.features.content_type_gating.helpers import CONTENT_GATING_PARTITION_ID, CONTENT_TYPE_GATE_GROUP_IDS
from openedx.features.content_type_gating.models import ContentTypeGatingConfig
from openedx.features.course_duration_limits.models import CourseDurationLimitConfig
from xmodule.modulestore.tests.factories import ItemFactory # lint-amnesty, pylint: disable=wrong-import-order
@ddt.ddt
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
class ProgressTabTestViews(BaseCourseHomeTests):
"""
Tests for the Progress Tab API
@@ -43,7 +47,14 @@ class ProgressTabTestViews(BaseCourseHomeTests):
super().setUp()
self.url = reverse('course-home:progress-tab', args=[self.course.id])
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def add_subsection_with_problem(self, **kwargs):
"""Makes a chapter -> problem chain, and sets up the subsection as requested, returning the problem"""
chapter = ItemFactory(parent=self.course, category='chapter')
subsection = ItemFactory(parent=chapter, category='sequential', graded=True, **kwargs)
vertical = ItemFactory(parent=subsection, category='vertical', graded=True)
problem = ItemFactory(parent=vertical, category='problem', graded=True)
return problem
@ddt.data(CourseMode.AUDIT, CourseMode.VERIFIED)
def test_get_authenticated_enrolled_user(self, enrollment_mode):
CourseEnrollment.enroll(self.user, self.course.id, enrollment_mode)
@@ -64,7 +75,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
elif enrollment_mode == CourseMode.AUDIT:
assert response.data['certificate_data']['cert_status'] == 'audit_passing'
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
@ddt.data(True, False)
def test_get_authenticated_user_not_enrolled(self, has_previously_enrolled):
if has_previously_enrolled:
@@ -74,13 +84,11 @@ class ProgressTabTestViews(BaseCourseHomeTests):
response = self.client.get(self.url)
assert response.status_code == 401
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_get_unauthenticated_user(self):
self.client.logout()
response = self.client.get(self.url)
assert response.status_code == 401
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_get_unknown_course(self):
url = reverse('course-home:progress-tab', args=['course-v1:unknown+course+2T2020'])
response = self.client.get(url)
@@ -93,7 +101,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
response = self.client.get(self.url)
assert response.status_code == 404
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_masquerade(self):
# Enroll a verified user
verified_user = UserFactory(is_staff=False)
@@ -112,7 +119,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
assert self.client.get(self.url).data.get('enrollment_mode') == 'verified'
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_has_scheduled_content_data(self):
CourseEnrollment.enroll(self.user, self.course.id)
future = now() + timedelta(days=30)
@@ -121,7 +127,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
assert response.status_code == 200
assert response.json()['has_scheduled_content']
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_end(self):
CourseEnrollment.enroll(self.user, self.course.id)
future = now() + timedelta(days=30)
@@ -132,7 +137,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
end = dateutil.parser.parse(response.json()['end']).replace(tzinfo=UTC)
assert end.date() == future.date()
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_user_has_passing_grade(self):
CourseEnrollment.enroll(self.user, self.course.id)
self.course.grade_cutoffs = {'Pass': 0}
@@ -141,7 +145,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
assert response.status_code == 200
assert response.json()['user_has_passing_grade']
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_verified_mode(self):
enrollment = CourseEnrollment.enroll(self.user, self.course.id)
CourseDurationLimitConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))
@@ -152,7 +155,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
'currency': 'USD', 'currency_symbol': '$', 'price': 149,
'sku': 'ABCD1234', 'upgrade_url': '/dashboard'}
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_page_respects_stacked_config(self):
CourseEnrollment.enroll(self.user, self.course.id)
course_overview = CourseOverview.get_from_id(self.course.id)
@@ -165,7 +167,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
response = self.client.get(self.url)
assert response.status_code == 404
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_learner_has_access(self):
chapter = ItemFactory(parent=self.course, category='chapter')
gated = ItemFactory(parent=chapter, category='sequential')
@@ -192,7 +193,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
assert not gated_score['learner_has_access']
assert ungated_score['learner_has_access']
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
def test_override_is_visible(self):
with persistent_grades_feature_flags(global_flag=True):
@@ -216,7 +216,7 @@ class ProgressTabTestViews(BaseCourseHomeTests):
created_grade = PersistentSubsectionGrade.update_or_create_grade(**params)
proctoring_failure_comment = "Failed Test Proctoring"
override = PersistentSubsectionGradeOverride.update_or_create_override(
PersistentSubsectionGradeOverride.update_or_create_override(
requesting_user=self.staff_user,
subsection_grade_model=created_grade,
earned_all_override=0.0,
@@ -236,7 +236,6 @@ class ProgressTabTestViews(BaseCourseHomeTests):
assert override_entry['system'] == GradeOverrideFeatureEnum.proctoring
assert override_entry['reason'] == proctoring_failure_comment
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_view_other_students_progress_page(self):
# Test the ability to view progress pages of other students by changing the url
CourseEnrollment.enroll(self.user, self.course.id)
@@ -261,13 +260,10 @@ class ProgressTabTestViews(BaseCourseHomeTests):
response = self.client.get(self.url)
assert response.data['username'] == other_user.username
@override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True)
def test_url_hidden_if_subsection_hide_after_due(self):
chapter = ItemFactory(parent=self.course, category='chapter')
yesterday = now() - timedelta(days=1)
hide_after_due_subsection = ItemFactory(
parent=chapter, category='sequential', hide_after_due=True, due=yesterday
)
ItemFactory(parent=chapter, category='sequential', hide_after_due=True, due=yesterday)
CourseEnrollment.enroll(self.user, self.course.id)
@@ -279,3 +275,37 @@ class ProgressTabTestViews(BaseCourseHomeTests):
hide_after_due_subsection = sections[1]['subsections'][0]
assert regular_subsection['url'] is not None
assert hide_after_due_subsection['url'] is None
@ddt.data(
(True, 0.7), # midterm and final are visible to staff
(False, 0.3), # just the midterm is visible to learners
)
@ddt.unpack
def test_course_grade_considers_subsection_grade_visibility(self, is_staff, expected_percent):
"""
Verify that the grade & is_passing info we send out is for visible grades only.
Assumes that grading policy is the default one (search for DEFAULT_GRADING_POLICY).
"""
if is_staff:
self.switch_to_staff()
CourseEnrollment.enroll(self.user, self.course.id)
tomorrow = now() + timedelta(days=1)
with self.store.bulk_operations(self.course.id):
never = self.add_subsection_with_problem(format='Homework', show_correctness='never')
always = self.add_subsection_with_problem(format='Midterm Exam', show_correctness='always')
past_due = self.add_subsection_with_problem(format='Final Exam', show_correctness='past_due', due=tomorrow)
answer_problem(self.course, get_mock_request(self.user), never)
answer_problem(self.course, get_mock_request(self.user), always)
answer_problem(self.course, get_mock_request(self.user), past_due)
# First, confirm the grade in the database - it should never change based on user state.
# This is midterm and final and a single problem added together.
assert CourseGradeFactory().read(self.user, self.course).percent == 0.72
response = self.client.get(self.url)
assert response.status_code == 200
assert response.data['course_grade']['percent'] == expected_percent
assert response.data['course_grade']['is_passing'] == (expected_percent >= 0.5)

3
lms/djangoapps/course_home_api/progress/views.py
View File

@@ -204,6 +204,9 @@ class ProgressTabView(RetrieveAPIView):
collected_block_structure = get_block_structure_manager(course_key).get_collected()
course_grade = CourseGradeFactory().read(student, collected_block_structure=collected_block_structure)
# recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not)
course_grade.update(visible_grades_only=True, has_staff_access=is_staff)
# Get has_scheduled_content data
transformers = BlockStructureTransformers()
transformers += [start_date.StartDateTransformer(), ContentTypeGateTransformer()]

2
lms/djangoapps/courseware/tests/test_submitting_problems.py
View File

@@ -282,7 +282,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
Returns list of scores: [<points on hw_1>, <points on hw_2>, ..., <points on hw_n>]
"""
return [
s.graded_total.earned for s in self.get_course_grade().graded_subsections_by_format['Homework'].values()
s.graded_total.earned for s in self.get_course_grade().graded_subsections_by_format()['Homework'].values()
]
def hw_grade(self, hw_url_name):

34
lms/djangoapps/grades/course_grade.py
View File

@@ -41,6 +41,18 @@ class CourseGradeBase:
self.passed,
)
def update(self, visible_grades_only=False, has_staff_access=False):
"""
Recalculates the grade for the course, with the given parameters.
Also updates subsection grades if self.force_update_subsections is true.
Arguments:
visible_grades_only: Only considers grades the user can see (via show_correctness subsection field)
has_staff_access: Used to help determine which grades are visible (if visible_grades_only=True)
"""
return self
@property
def attempted(self):
"""
@@ -71,8 +83,7 @@ class CourseGradeBase:
)
return self._get_subsection_grade(subsection)
@lazy
def graded_subsections_by_format(self):
def graded_subsections_by_format(self, visible_grades_only=False, has_staff_access=False):
"""
Returns grades for the subsections in the course in
a dict keyed by subsection format types.
@@ -80,7 +91,8 @@ class CourseGradeBase:
subsections_by_format = defaultdict(OrderedDict)
for chapter in self.chapter_grades.values():
for subsection_grade in chapter['sections']:
if subsection_grade.graded:
is_visible = not visible_grades_only or subsection_grade.show_grades(has_staff_access)
if subsection_grade.graded and is_visible:
graded_total = subsection_grade.graded_total
if graded_total.possible > 0:
subsections_by_format[subsection_grade.format][subsection_grade.location] = subsection_grade
@@ -156,14 +168,14 @@ class CourseGradeBase:
possible += child_possible
return earned, possible
@lazy
def grader_result(self):
def grader_result(self, visible_grades_only=False, has_staff_access=False):
"""
Returns the result from the course grader.
"""
course = self._prep_course_for_grading(self.course_data.course)
return course.grader.grade(
self.graded_subsections_by_format,
self.graded_subsections_by_format(visible_grades_only=visible_grades_only,
has_staff_access=has_staff_access),
generate_random_scores=settings.GENERATE_PROFILE_SCORES,
)
@@ -174,7 +186,7 @@ class CourseGradeBase:
DEPRECATED: To be removed as part of TNL-5291.
"""
# TODO(TNL-5291) Remove usages of this deprecated property.
grade_summary = self.grader_result
grade_summary = self.grader_result()
grade_summary['percent'] = self.percent
grade_summary['grade'] = self.letter_grade
return grade_summary
@@ -259,11 +271,10 @@ class CourseGrade(CourseGradeBase):
super().__init__(user, course_data, *args, **kwargs)
self._subsection_grade_factory = SubsectionGradeFactory(user, course_data=course_data)
def update(self):
def update(self, visible_grades_only=False, has_staff_access=False):
"""
Updates the grade for the course. Also updates subsection grades
if self.force_update_subsections is true, via the lazy call
to self.grader_result.
if self.force_update_subsections is true, via the call to self.grader_result.
"""
# TODO update this code to be more functional and readable.
# Currently, it is hard to follow since there are plenty of
@@ -271,7 +282,8 @@ class CourseGrade(CourseGradeBase):
# can be passed through and not confusingly stored and used
# at a later time.
grade_cutoffs = self.course_data.course.grade_cutoffs
self.percent = self._compute_percent(self.grader_result)
grader_result = self.grader_result(visible_grades_only=visible_grades_only, has_staff_access=has_staff_access)
self.percent = self._compute_percent(grader_result)
self.letter_grade = self._compute_letter_grade(grade_cutoffs, self.percent)
self.passed = self._compute_passed(grade_cutoffs, self.percent)
return self

1
lms/djangoapps/grades/course_grade_factory.py
View File

@@ -103,7 +103,6 @@ class CourseGradeFactory:
course_data = CourseData(
user=None, course=course, collected_block_structure=collected_block_structure, course_key=course_key,
)
stats_tags = [f'action:{course_data.course_key}'] # lint-amnesty, pylint: disable=unused-variable
for user in users:
yield self._iter_grade_result(user, course_data, force_update)

4
lms/djangoapps/instructor_task/tests/test_integration.py
View File

@@ -134,8 +134,8 @@ class TestRescoringTask(TestIntegrationTask):
expected_subsection_grade = expected_score
course_grade = CourseGradeFactory().read(user, self.course)
assert course_grade.graded_subsections_by_format['Homework'][self.problem_section.location].graded_total.earned\
== expected_subsection_grade
grade = course_grade.graded_subsections_by_format()['Homework'][self.problem_section.location].graded_total
assert grade.earned == expected_subsection_grade
def submit_rescore_all_student_answers(self, instructor, problem_url_name, only_if_higher=False):
"""Submits the particular problem for rescoring"""