Merge pull request #30043 from openedx/mikix/progress-visible-grades
fix: have progress MFE API recalculate visible course grade
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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()]
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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"""
|
||||
|
||||
Reference in New Issue
Block a user