(WIP) Graded library content - Do not merge, for internal review - #3
Closed
bradenmacdonald wants to merge 6 commits into
Closed
(WIP) Graded library content - Do not merge, for internal review#3bradenmacdonald wants to merge 6 commits into
bradenmacdonald wants to merge 6 commits into
Conversation
…e (belongs on subsection/sequential)
Allows modules with 'always_recalculate_grades' to use their children's state when computing their grade via grades.grade()
…ldn't even be an enum anymore?
Member
There was a problem hiding this comment.
This feels hackier still than in #1 . Would it make sense to clean this up, ie not having get_item removing the branch ever, and fixing the higher-level code that relies on this instead? I imagine it would likely be a lot of work, but if we can leave the place cleaner after us, it will be appreciated.
Member
Author
There was a problem hiding this comment.
I had to make this change because my earlier code broke the Acid XBlock in studio. I agree it's less than ideal.
About changing it in the modulestore: we would need to discuss that with upstream, probably Don. It's a small change code-wise that could have a lot of big implications throughout the codebase.
Member
Author
There was a problem hiding this comment.
Thinking about this some more: If we're using a new "LibraryLocator" to reference libraries, then we won't have a special branch, so this code may not be needed at all :)
I'll investigate more today.
Member
Author
There was a problem hiding this comment.
@antoviaque Yep, good news: I have developed a prototype of new locators for the Content Libraries, and with that change, this code (and the other remove_branch stuff you were questioning) is no longer needed at all.
Member
There was a problem hiding this comment.
Great : ) And thank you for the other answers, it useful to read the explanations.
Member
Author
|
Superseded by #5. |
smarnach
added a commit
that referenced
this pull request
Oct 20, 2015
Add French translations and hide course number in the page header.
itsjeyd
pushed a commit
that referenced
this pull request
Dec 23, 2015
itsjeyd
pushed a commit
that referenced
this pull request
Dec 23, 2015
…dback-update-3 Hash Update #3: Diagnostic-Feedback
pomegranited
added a commit
that referenced
this pull request
Feb 20, 2018
Fix pip dependencies broken since ginkgo.1, and mongo replicaset read_preference
DanielVZ96
pushed a commit
that referenced
this pull request
Jan 31, 2024
The Getting Started steps had two #3, and one of them was mis-formatted. The links to Getting Started in the named-release page weren't links. Now they are, though they go to the GitHub page rather than the readthedocs page. We should fix that in the future.
navinkarkera
pushed a commit
that referenced
this pull request
Jul 31, 2026
…d APIs (openedx#38773) * feat: apply ADR 0036 to Xblock v1 (?view=minimal) Adds a ``?view=minimal`` query parameter on ``XblockViewSet.retrieve`` that filters the (tree-shaped) xblock response down to a small set of structural fields — id, display_name, category, children, has_children, studio_url — dropping heavy/contextual fields such as data, metadata, fields, student_view_data, edited_on, published. Default response shape is unchanged (full xblock payload) to avoid breaking the existing Studio frontend. The pre-existing ``?fields=...`` query parameter retains its legacy "type of response" semantics (?fields=graderType, ?fields=ancestorInfo, ?fields=customReadToken); ADR 0036's CSV-subset interpretation is deferred to a future API version to avoid breaking those callers. Adds 4 regression tests covering: default response untouched, minimal strips heavy fields, minimal keeps structural fields, minimal is a no-op for non-dict response bodies (legacy ?fields=graderType path). * feat: apply ADR 0036 to CourseHome v3 (?fields= on list action) The v3 ``HomeViewSet.list`` action returns a wide ``StudioHomeSerializer`` payload with ~25 top-level keys (courses, archived_courses, libraries, allowed_organizations, allowed_organizations_for_libraries, plus Studio settings). Adds a ``?fields=`` query parameter so clients can request a subset of those keys explicitly. The flat-list ``courses`` and ``libraries`` actions are out of scope (each returns a single-key dict wrapping a list of small items — no nested or wide structure to filter). Adds a shared ``apply_field_selection`` helper to ``v3/utils.py`` so future v3 viewsets can opt into the same convention without re-implementing it. Adds 2 regression tests: default keeps all keys, ``?fields=courses,libraries`` restricts to exactly those keys. * docs: audit CourseHome v4 against ADR 0036 (out of scope) Adds an ADR 0036 entry to ``HomeCoursesViewSet``'s compliance list explicitly marking this endpoint as out of scope. Rationale: the v4 home endpoint returns a flat paginated list governed by ADR 0032 (DefaultPagination 7-field envelope). ADR 0036 excludes flat lists from its ``?view=`` / ``?depth=`` / minimal-by-default requirements — those apply to tree-shaped responses or wide flat objects with embedded sub-objects. Each course item is a thin 9-field record with no nested children, no embedded sub-objects, and no tree structure to bound. Per-item ``?fields=`` subset filtering remains a possible follow-up (would require a dynamic-fields serializer mixin and per-field schema documentation) but is deferred to keep the v4 contract stable for the existing Studio frontend. * feat: apply ADR 0036 to Enrollment v2 (?view=minimal flattens course_details) Each enrollment record returned by the v2 ``EnrollmentViewSet.list`` and ``EnrollmentRetrieveView.get`` actions embeds a full ``course_details`` sub-object (which itself includes a ``course_modes`` list and other heavy course-overview fields). Server-to-server callers and AI agents that only need to know which courses a user is enrolled in shouldn't have to parse the embedded sub-object on every row. Adds a ``?view=minimal`` query parameter on both actions that collapses the embedded ``course_details`` to a single ``course_id`` string. The enrollment-level fields (``created``, ``mode``, ``is_active``, ``user``) are preserved. Default response shape is unchanged. Adds 2 regression tests (mocked, MongoDB-free): default list keeps the full shape, ``?view=minimal`` collapses each row's course_details to a flat course_id and the heavy fields are dropped. * feat: apply ADR 0036 to Course Detail v3 (?view=minimal + ?fields=) The v3 ``CourseDetailsViewSet.retrieve`` action returns a wide ``CourseDetailsSerializer`` payload with ~40 top-level fields plus a nested ``instructor_info`` sub-object (instructor names, bios, image URLs) and a ``learning_info`` long-form list. The default full payload is preserved; two new opt-in query parameters apply ADR 0036: * ``?view=minimal`` drops heavy fields (overview, syllabus, description, short_description, instructor_info, learning_info, banner_image_name / banner_image_asset_path, video_thumbnail assets, license) — leaving only identification (course_id, org, run, title, subtitle, language), schedule (start_date, end_date, enrollment_start, enrollment_end, certificate_available_date), and flags (self_paced, certificates_display_behavior, has_changes). * ``?fields=a,b,c`` keeps an explicit CSV subset of top-level keys. Composes with ``?view=minimal`` — the preset is applied first, then the explicit subset. Reuses ``apply_field_selection`` from ``v3/utils.py`` (introduced in the CourseHome v3 commit) so the convention is consistent across v3. Adds 3 regression tests (mocked, MongoDB-free): default keeps all fields, ?view=minimal drops the heavy/embedded ones and keeps identification/schedule/flags, ?fields=course_id,title restricts to exactly those keys. * docs: audit AuthorGrading v3 against ADR 0036 (largely out of scope) Adds an ADR 0036 entry to ``AuthoringGradingViewSet``'s compliance list. Rationale: the v3 grading response is a single top-level ``graders`` list of small fixed-shape objects (type, min_count, drop_count, short_label, weight, id) — no tree nesting, no embedded sub-objects, no ``children`` field, no wide flat object. ``?view=minimal`` and ``?fields=`` would have no fields to drop; the only ADR 0036 concern that applies is anti-pattern #3 (unbounded child list). In practice each course has ≤8 graders (Homework, Lab, Exam, etc.) and the update flow is exercised only by course-authoring staff, so the real-world payload is always small. The hard cap is enforced upstream by ``CourseGradingModel.update_from_json``; documented as a note rather than re-implemented at the view layer.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This is a continuation of the prototype found in #2, extended to implement SOL-19 "As a student, when I complete interactive exercises displayed in a LibraryContent block in the LMS, I see a single aggregated grade for the set of exercises in the progress tab."
Testing instructions: Follow same testing steps as #2, except on step 7 when adding content to the library, add problems (of any type). On step 10, be sure to set "Scored" to True. Then, after all the other steps, view the course as a student. Check that the "Progress" tab is correct, before and after submitting answers to the problems. Test the weighting and make sure none of the children appear in the progress view - only the combined score of the LibraryContent block.
Library screenshot:
LMS screenshot: