feat: optional xblocks #638

DanielVZ96 · 2024-02-29T03:14:10Z

Description

This PR implements separating optional progress from normal progress, and also displaying optional chapters and sequences in the outline.

Impacts: Learners and authors

Supporting information

Fontend App Learning PR: feat: optional xblocks frontend-app-learning#9

Testing instructions

Run the MFE from this branch: feat: optional xblocks openedx/frontend-app-learning#1296
Add "done" to the advanced module list
As a non-staff user go to the progress tab and assert only the typical progress donut appears
As a staff user, on the cms open the settings of any sequence or chapter and mark as optional
As a non-staff user, complete some units, and also the optional unit
Navigate to the progress tab again and assert a new optional donut appears with your progress

Screenshots

Private-ref: https://tasks.opencraft.com/browse/BB-8586

cms/djangoapps/contentstore/utils.py

Agrendalath · 2024-03-01T21:00:27Z

cms/static/js/views/modals/course_outline_modals.js

+                optional_completion: this.model.get('optional_completion'),
+                optionalAncestor: this.model.get('ancestor_has_optional_completion')


Nit: we should not mix the snake_case with camelCase.

Agrendalath · 2024-03-01T22:08:23Z

lms/djangoapps/course_api/blocks/transformers/block_completion.py

@@ -14,7 +14,7 @@ class BlockCompletionTransformer(BlockStructureTransformer):
    Keep track of the completion of each block within the block structure.
    """
    READ_VERSION = 1
-    WRITE_VERSION = 1
+    WRITE_VERSION = 2


Actually, this seems to be a backward-compatible change, so we can revert bumping this version.

Agrendalath · 2024-03-01T22:10:46Z

lms/djangoapps/course_home_api/outline/tests/test_view.py

@@ -437,3 +437,18 @@ def test_cannot_enroll_if_full(self):
        self.update_course_and_overview()
        CourseEnrollment.enroll(UserFactory(), self.course.id)  # grr, some rando took our spot!
        self.assert_can_enroll(False)
+
+    def test_optional_completion(self):


Nit: can we use a more verbose name for this test, like "test_optional_completion_off_by_default"?

lms/djangoapps/courseware/courses.py

Agrendalath · 2024-03-01T22:15:23Z

cms/djangoapps/contentstore/views/block.py

+            if xblock_info['optional_completion']:
+                xblock_info['ancestor_has_optional_completion'] = ancestor_has_optional_completion(xblock, parent_xblock)


What if the optional_completion is set to False (default), but the parent XBlock has it set to True? In this use case, the field is still active. We should disable this checkbox when ancestor_has_optional_completion returns True, regardless of the xblock_info['optional_completion'] value.

Agrendalath · 2024-03-01T22:21:53Z

cms/static/js/views/modals/course_outline_modals.js

                    tabs[1].editors = [StaffLockEditor];
                } else if (xblockInfo.isSequential()) {
-                    tabs[0].editors = [ReleaseDateEditor, GradingEditor, DueDateEditor];
+                    tabs[0].editors = [ReleaseDateEditor, GradingEditor, DueDateEditor, OptionalCompletionEditor];


Regarding this comment about hiding the editor when the completion tracking is not enabled, please see the following parts of code that handle a similar scenario for another editor:

edx-platform/cms/templates/base.html

Line 160 in a8535ff

is_custom_relative_dates_active: ${CUSTOM_RELATIVE_DATES.is_enabled(context_course.id) | n, dump_js_escaped_json},

edx-platform/cms/static/js/views/modals/course_outline_modals.js

Lines 1290 to 1292 in 42451ca

if (course.get('self_paced') && course.get('is_custom_relative_dates_active')) {

tabs[0].editors.push(SelfPacedDueDateEditor);

}

I think we shouldn't move this to a follow-up ticket because this feature is not enabled by default in edx-platform.

wow i didn't realize it'd be so straight forward. implementing this

done. I wasn't able to test it manually without hardcoding the value in the base.html. it seems waffle flags are cached somewhere and take quite some time to invalidate the cache.

In case you want to test it and have my same problem you can hard code completion_tracking_enabled to False in cms/templates/base.html line 162.

Agrendalath · 2024-03-01T22:23:45Z

cms/templates/js/optional-completion-editor.underscore

+			<%- gettext('Optional') %>
+          <% } %>
+        <p class="field-message">
+            <% var message = gettext('Optional %(xblockType)ss won\'t count towards course or parent completion. Only has an effect if completion module is enabled.') %>


Suggested change

<% var message = gettext('Optional %(xblockType)ss won\'t count towards course or parent completion. Only has an effect if completion module is enabled.') %>

<% var message = gettext('Optional %(xblockType)ss won\'t count towards course or parent completion.') %>

Once we hide the editor when the completion tracking is not enabled, we can remove this part.

cms/djangoapps/contentstore/utils.py

Agrendalath · 2024-03-25T19:16:50Z

cms/static/js/spec/views/pages/course_outline_spec.js

        ]);
        appendSetFixtures(mockOutlinePage);
        mockCourseJSON = createMockCourseJSON({}, [
-            createMockSectionJSON({}, [
+            createMockSectionJSON({completion_tracking_enabled: true}, [


@DanielVZ96, the completion tracking should be enabled for the course instead of the section.

Also, you should be able to use something like console.warn(window. course) to verify that it's applied correctly.

…in units

…s, units

DanielVZ96 requested a review from Agrendalath February 29, 2024 03:14

DanielVZ96 mentioned this pull request Feb 29, 2024

feat: optional xblocks open-craft/frontend-app-learning#9

Open

DanielVZ96 force-pushed the dvz/optional-lesson branch 4 times, most recently from 2fee526 to 42451ca Compare February 29, 2024 07:25

Agrendalath reviewed Mar 1, 2024

View reviewed changes

DanielVZ96 requested a review from Agrendalath March 10, 2024 01:46

Agrendalath reviewed Mar 18, 2024

View reviewed changes

cms/djangoapps/contentstore/utils.py Show resolved Hide resolved

DanielVZ96 force-pushed the dvz/optional-lesson branch 2 times, most recently from 487ae2d to a2968d0 Compare March 23, 2024 06:13

Agrendalath reviewed Mar 25, 2024

View reviewed changes

DanielVZ96 force-pushed the dvz/optional-lesson branch 18 times, most recently from 3f0b03c to f7745fd Compare March 31, 2024 05:41

DanielVZ96 force-pushed the dvz/optional-lesson branch 18 times, most recently from 3046507 to 84ad5ee Compare April 2, 2024 11:53

DanielVZ96 requested a review from Agrendalath April 4, 2024 04:46

DanielVZ96 and others added 10 commits April 30, 2024 18:46

feat: optional xblocks

d8c7d47

feat: address PR comments

1a4994b

fix: use completion tracking flag and small docstring changes

96750c6

fix: use correct field for displaying the optional completion editor …

9563633

…in units

feat: display optional completion information in sections, subsection…

33d374b

…s, units

fix: rely on the inheritance mechanism for optional completion

63d04fc

test: add tests for outline modals

8f020f5

test: fix JS tests

aa841e7

test: add sets to null unittest

40d5a33

test: remove redundant changes

e57c355

Agrendalath force-pushed the dvz/optional-lesson branch from d1618cb to e57c355 Compare April 30, 2024 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: optional xblocks #638

feat: optional xblocks #638

DanielVZ96 commented Feb 29, 2024 •

edited

Agrendalath Mar 1, 2024

DanielVZ96 Mar 10, 2024

Agrendalath Mar 1, 2024

DanielVZ96 Mar 10, 2024

Agrendalath Mar 1, 2024

DanielVZ96 Mar 10, 2024

Agrendalath Mar 1, 2024

DanielVZ96 Mar 9, 2024

Agrendalath Mar 1, 2024

DanielVZ96 Mar 9, 2024

DanielVZ96 Mar 9, 2024

Agrendalath Mar 1, 2024

DanielVZ96 Mar 9, 2024

Agrendalath Mar 25, 2024

		optional_completion: this.model.get('optional_completion'),
		optionalAncestor: this.model.get('ancestor_has_optional_completion')

		if xblock_info['optional_completion']:
		xblock_info['ancestor_has_optional_completion'] = ancestor_has_optional_completion(xblock, parent_xblock)

	if (course.get('self_paced') && course.get('is_custom_relative_dates_active')) {
	tabs[0].editors.push(SelfPacedDueDateEditor);
	}

	<% var message = gettext('Optional %(xblockType)ss won\'t count towards course or parent completion. Only has an effect if completion module is enabled.') %>
	<% var message = gettext('Optional %(xblockType)ss won\'t count towards course or parent completion.') %>

feat: optional xblocks #638

Are you sure you want to change the base?

feat: optional xblocks #638

Conversation

DanielVZ96 commented Feb 29, 2024 • edited

Description

Supporting information

Testing instructions

Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielVZ96 commented Feb 29, 2024 •

edited