-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat: migrate progress page data calculation logic from learning MFE and add 'never_but_include_grade' grade visibility option #37399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
ormsbee
merged 9 commits into
openedx:master
from
mitodl:anas/add-new-grade-visisbility-option
Oct 22, 2025
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
ffc7a96
feat: add never_but_include_grade visibility option
Anas12091101 5ae9c5a
fix: lint issues
Anas12091101 795b17e
test: add tests
Anas12091101 12ddf93
temp: empty commit to run the shell check again
Anas12091101 330dacc
feat: added progress page data calculation logic
Anas12091101 6731351
fix: lint issues
Anas12091101 10f083d
fix: pycodestyle issues
Anas12091101 796a658
docs: improved comment
Anas12091101 039fc51
fix: issues
Anas12091101 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,226 @@ | |
| Python APIs exposed for the progress tracking functionality of the course home API. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from django.contrib.auth import get_user_model | ||
| from opaque_keys.edx.keys import CourseKey | ||
| from openedx.core.lib.grade_utils import round_away_from_zero | ||
| from xmodule.graders import ShowCorrectness | ||
| from datetime import datetime, timezone | ||
|
|
||
| from lms.djangoapps.courseware.courses import get_course_blocks_completion_summary | ||
| from dataclasses import dataclass, field | ||
|
|
||
| User = get_user_model() | ||
|
|
||
|
|
||
| @dataclass | ||
| class _AssignmentBucket: | ||
| """Holds scores and visibility info for one assignment type. | ||
|
|
||
| Attributes: | ||
| assignment_type: Full assignment type name from the grading policy (for example, "Homework"). | ||
| num_total: The total number of assignments expected to contribute to the grade before any | ||
| drop-lowest rules are applied. | ||
| last_grade_publish_date: The most recent date when grades for all assignments of assignment_type | ||
| are released and included in the final grade. | ||
| scores: Per-subsection fractional scores (each value is ``earned / possible`` and falls in | ||
| the range 0–1). While awaiting published content we pad the list with zero placeholders | ||
| so that its length always matches ``num_total`` until real scores replace them. | ||
| visibilities: Mirrors ``scores`` index-for-index and records whether each subsection's | ||
| correctness feedback is visible to the learner (``True``), hidden (``False``), or not | ||
| yet populated (``None`` when the entry is a placeholder). | ||
| included: Tracks whether each subsection currently counts toward the learner's grade as | ||
| determined by ``SubsectionGrade.show_grades``. Values follow the same convention as | ||
| ``visibilities`` (``True`` / ``False`` / ``None`` placeholders). | ||
| assignments_created: Count of real subsections inserted into the bucket so far. Once this | ||
| reaches ``num_total``, all placeholder entries have been replaced with actual data. | ||
| """ | ||
| assignment_type: str | ||
| num_total: int | ||
| last_grade_publish_date: datetime | ||
| scores: list[float] = field(default_factory=list) | ||
| visibilities: list[bool | None] = field(default_factory=list) | ||
| included: list[bool | None] = field(default_factory=list) | ||
| assignments_created: int = 0 | ||
|
|
||
| @classmethod | ||
| def with_placeholders(cls, assignment_type: str, num_total: int, now: datetime): | ||
| """Create a bucket prefilled with placeholder (empty) entries.""" | ||
| return cls( | ||
| assignment_type=assignment_type, | ||
| num_total=num_total, | ||
| last_grade_publish_date=now, | ||
| scores=[0] * num_total, | ||
| visibilities=[None] * num_total, | ||
| included=[None] * num_total, | ||
| ) | ||
|
|
||
| def add_subsection(self, score: float, is_visible: bool, is_included: bool): | ||
| """Add a subsection’s score and visibility, replacing a placeholder if space remains.""" | ||
| if self.assignments_created < self.num_total: | ||
| if self.scores: | ||
| self.scores.pop(0) | ||
| if self.visibilities: | ||
| self.visibilities.pop(0) | ||
| if self.included: | ||
| self.included.pop(0) | ||
| self.scores.append(score) | ||
| self.visibilities.append(is_visible) | ||
| self.included.append(is_included) | ||
| self.assignments_created += 1 | ||
|
|
||
| def drop_lowest(self, num_droppable: int): | ||
| """Remove the lowest scoring subsections, up to the provided num_droppable.""" | ||
| while num_droppable > 0 and self.scores: | ||
| idx = self.scores.index(min(self.scores)) | ||
| self.scores.pop(idx) | ||
| self.visibilities.pop(idx) | ||
| self.included.pop(idx) | ||
| num_droppable -= 1 | ||
|
|
||
| def hidden_state(self) -> str: | ||
| """Return whether kept scores are all, some, or none hidden.""" | ||
| if not self.visibilities: | ||
| return 'none' | ||
| all_hidden = all(v is False for v in self.visibilities) | ||
| some_hidden = any(v is False for v in self.visibilities) | ||
| if all_hidden: | ||
| return 'all' | ||
| if some_hidden: | ||
| return 'some' | ||
| return 'none' | ||
|
Comment on lines
+86
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we create constants for all these states? |
||
|
|
||
| def averages(self) -> tuple[float, float]: | ||
| """Compute visible and included averages over kept scores. | ||
|
|
||
| Visible average uses only grades with visibility flag True in numerator; denominator is total | ||
| number of kept scores (mirrors legacy behavior). Included average uses only scores that are | ||
| marked included (show_grades True) in numerator with same denominator. | ||
|
|
||
| Returns: | ||
| (earned_visible, earned_all) tuple of floats (0-1 each). | ||
| """ | ||
| if not self.scores: | ||
| return 0.0, 0.0 | ||
| visible_scores = [s for i, s in enumerate(self.scores) if self.visibilities[i]] | ||
| included_scores = [s for i, s in enumerate(self.scores) if self.included[i]] | ||
| earned_visible = (sum(visible_scores) / len(self.scores)) if self.scores else 0.0 | ||
| earned_all = (sum(included_scores) / len(self.scores)) if self.scores else 0.0 | ||
| return earned_visible, earned_all | ||
|
|
||
|
|
||
| class _AssignmentTypeGradeAggregator: | ||
| """Collects and aggregates subsection grades by assignment type.""" | ||
|
|
||
| def __init__(self, course_grade, grading_policy: dict, has_staff_access: bool): | ||
| """Initialize with course grades, grading policy, and staff access flag.""" | ||
| self.course_grade = course_grade | ||
| self.grading_policy = grading_policy | ||
| self.has_staff_access = has_staff_access | ||
| self.now = datetime.now(timezone.utc) | ||
| self.policy_map = self._build_policy_map() | ||
| self.buckets: dict[str, _AssignmentBucket] = {} | ||
|
|
||
| def _build_policy_map(self) -> dict: | ||
| """Convert grading policy into a lookup of assignment type → policy info.""" | ||
| policy_map = {} | ||
| for policy in self.grading_policy.get('GRADER', []): | ||
| policy_map[policy.get('type')] = { | ||
| 'weight': policy.get('weight', 0.0), | ||
| 'short_label': policy.get('short_label', ''), | ||
| 'num_droppable': policy.get('drop_count', 0), | ||
| 'num_total': policy.get('min_count', 0), | ||
| } | ||
| return policy_map | ||
|
|
||
| def _bucket_for(self, assignment_type: str) -> _AssignmentBucket: | ||
| """Get or create a score bucket for the given assignment type.""" | ||
| bucket = self.buckets.get(assignment_type) | ||
| if bucket is None: | ||
| num_total = self.policy_map.get(assignment_type, {}).get('num_total', 0) or 0 | ||
| bucket = _AssignmentBucket.with_placeholders(assignment_type, num_total, self.now) | ||
| self.buckets[assignment_type] = bucket | ||
| return bucket | ||
|
|
||
| def collect(self): | ||
| """Gather subsection grades into their respective assignment buckets.""" | ||
| for chapter in self.course_grade.chapter_grades.values(): | ||
| for subsection_grade in chapter.get('sections', []): | ||
| if not getattr(subsection_grade, 'graded', False): | ||
| continue | ||
| assignment_type = getattr(subsection_grade, 'format', '') or '' | ||
| if not assignment_type: | ||
| continue | ||
| graded_total = getattr(subsection_grade, 'graded_total', None) | ||
| earned = getattr(graded_total, 'earned', 0.0) if graded_total else 0.0 | ||
| possible = getattr(graded_total, 'possible', 0.0) if graded_total else 0.0 | ||
| earned = 0.0 if earned is None else earned | ||
| possible = 0.0 if possible is None else possible | ||
| score = (earned / possible) if possible else 0.0 | ||
| is_visible = ShowCorrectness.correctness_available( | ||
| subsection_grade.show_correctness, subsection_grade.due, self.has_staff_access | ||
| ) | ||
| is_included = subsection_grade.show_grades(self.has_staff_access) | ||
| bucket = self._bucket_for(assignment_type) | ||
| bucket.add_subsection(score, is_visible, is_included) | ||
| visibilities_with_due_dates = [ShowCorrectness.PAST_DUE, ShowCorrectness.NEVER_BUT_INCLUDE_GRADE] | ||
| if subsection_grade.show_correctness in visibilities_with_due_dates: | ||
| if subsection_grade.due and subsection_grade.due > bucket.last_grade_publish_date: | ||
| bucket.last_grade_publish_date = subsection_grade.due | ||
|
|
||
| def build_results(self) -> dict: | ||
| """Apply drops, compute averages, and return aggregated results and total grade.""" | ||
| final_grades = 0.0 | ||
| rows = [] | ||
| for assignment_type, bucket in self.buckets.items(): | ||
| policy = self.policy_map.get(assignment_type, {}) | ||
| bucket.drop_lowest(policy.get('num_droppable', 0)) | ||
| earned_visible, earned_all = bucket.averages() | ||
| weight = policy.get('weight', 0.0) | ||
| short_label = policy.get('short_label', '') | ||
| row = { | ||
| 'type': assignment_type, | ||
| 'weight': weight, | ||
| 'average_grade': round_away_from_zero(earned_visible, 4), | ||
| 'weighted_grade': round_away_from_zero(earned_visible * weight, 4), | ||
| 'short_label': short_label, | ||
| 'num_droppable': policy.get('num_droppable', 0), | ||
| 'last_grade_publish_date': bucket.last_grade_publish_date, | ||
| 'has_hidden_contribution': bucket.hidden_state(), | ||
| } | ||
| final_grades += earned_all * weight | ||
| rows.append(row) | ||
| rows.sort(key=lambda r: r['weight']) | ||
| return {'results': rows, 'final_grades': round_away_from_zero(final_grades, 4)} | ||
|
|
||
| def run(self) -> dict: | ||
| """Execute full pipeline (collect + aggregate) returning final payload.""" | ||
| self.collect() | ||
| return self.build_results() | ||
|
|
||
|
|
||
| def aggregate_assignment_type_grade_summary( | ||
| course_grade, | ||
| grading_policy: dict, | ||
| has_staff_access: bool = False, | ||
| ) -> dict: | ||
| """ | ||
| Aggregate subsection grades by assignment type and return summary data. | ||
| Args: | ||
| course_grade: CourseGrade object containing chapter and subsection grades. | ||
| grading_policy: Dictionary representing the course's grading policy. | ||
| has_staff_access: Boolean indicating if the user has staff access to view all grades. | ||
| Returns: | ||
| Dictionary with keys: | ||
| results: list of per-assignment-type summary dicts | ||
| final_grades: overall weighted contribution (float, 4 decimal rounding) | ||
| """ | ||
| aggregator = _AssignmentTypeGradeAggregator(course_grade, grading_policy, has_staff_access) | ||
| return aggregator.run() | ||
|
|
||
|
|
||
| def calculate_progress_for_learner_in_course(course_key: CourseKey, user: User) -> dict: | ||
| """ | ||
| Calculate a given learner's progress in the specified course run. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Do we use the terminology "bucket" elsewhere in the grading code? I don't hate the name, but I don't want to add a convention if we already have something else we call this, say on the frontend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the frontend, the calculation logic primarily resides in these two functions:
https://github.com/openedx/frontend-app-learning/pull/1797/files#diff-4d3a6d853ff9fd1354a56024f06c6718c856c281e295de92c5b09ac1e86774bfL6-L92
Initially, I tried to replicate the exact calculation flow from the frontend, but it became difficult to follow due to the complexity of the logic. To improve readability and maintainability, I introduced the
_AssignmentBucketclass to make the code more modular.We can definitely consider renaming it if “bucket” doesn’t feel intuitive, for example, something like
AssignmentDetailsmight be clearer?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine, we can keep the terminology for now. Thank you.