feat: handle optional blocks in verticals

[DanielVZ96]

Note: Not sure how to increase coverage since there's no testing in place for

Description: handles not counting optional children for non-optional verticals

Dependencies:
open-craft/edx-platform#638

Testing instructions:

• Using this branch and feat: optional xblocks edx-platform#34275
• Create a sequence with a single optional unit and a non-optional unit
  • You can set them as optional in the settings for the unit in the content outline in studio
• Finish only the non-optional unit
• Make sure the sequence gets marked as done

Reviewers:

• tag reviewer
• tag reviewer

Merge checklist:

• All reviewers approved
• CI build is green
• Version bumped
• Changelog record added
• Documentation updated (not only docstrings)
• Commits are squashed

Post merge:

• Create a tag
• Check new version is pushed to PyPi after tag-triggered build is
  finished.
• Delete working branch (if not needed anymore)

Private-ref: BB-8586

[openedx-webhooks]

Thanks for the pull request, @DanielVZ96! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

[Agrendalath]

@DanielVZ96, shouldn't this be something like not optional_vertical and not getattr(child, "optional_content", False)? We should add a test for this. We could also use a variable like required_completion = getattr(item, "optional_content", False) to avoid too many negations that make this condition hard to read.

[DanielVZ96]

There are only four combinations:

1. Optional Vertical and Optional Child -> Include Child
2. Optional Vertical and Required Child -> Include Child (shouldn't happen but this is how we can handle it gracefully)
3. Required Vertical and Required Child -> Include Child
4. Required Vertical and Optional Child -> Exclude Child

This means that the only case in which we want to exclude the child
is when the parent is required and the child isn't.

This can be achieved with either of the following logic (nots turns ors into ands)

        a) optional_vertical or required_child  # can be read as "if it's an optional vertical include all, if it's a required vertical only accept required children"
        b) not (required_vertical and optional_child)  # can be read as "exclude the 4th case" 

Your's (not optional_vertical and not getattr(child, "optional_content", False)) can be paraphrased as required_vertical and required_child, and would only return true (thus include) the third case.

I'm refactoring the code to include this explanation and make it easier to test, since the vertical_is_complete is really hard to test without having edx-platform code available.

[Agrendalath]

@DanielVZ96, please convert this PR to draft - since this should be merged with the upstream PR to edx-platform, it will take a while before this is ready to be merged.

[Agrendalath]

Could we expand this docstring a bit to explain the concept of "optional" XBlocks?

[DanielVZ96]

done

[Agrendalath]

Nit: we should use @ddt.data for this.

[DanielVZ96]

done

[pomegranited]

Hi @DanielVZ96 @Agrendalath are you planning to finish this PR?

@pkulkark and I are CCs on this repo, so one of us can help with reviewing and merging.

[DanielVZ96]

@pomegranited It depends on this PR i'm working on open-craft/edx-platform#638 It's been a bit slow because I can't run js browser tests locally in palm.

[pomegranited]

No worries @DanielVZ96 , thanks for the update!

[Agrendalath]

@DanielVZ96, I need help understanding this PR's context. A course consists of the following components:

1. Course
2. Section (chapter)
3. Subsection (sequence)
4. Unit (vertical)
5. Individual XBlocks (potentially with children, in case of a LibraryBlock, ProblemBlock, etc.).

This PR handles counting the optional children of a vertical, which are mentioned in the last point above. We can override XBlock fields through the Django shell, but how can we create such optional XBlocks in Studio?

Also, should we mark the parent as completed in case the optional children are incomplete? In this case, users would get the completion checkmarks for these sections. It would be impossible for them to find the incomplete items if they want to increase their optional completion. I suppose we could introduce some new completion checkmark type (e.g., partial completion), but this will require more investigation and is out of scope here.

[DanielVZ96]

@Agrendalath you are completely right. i may have confused the terminology when working on this. there's no way of setting a single xblock as optional in studio for now.

[openedx-webhooks]

@DanielVZ96 Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.