Assessment targeted review

[bradenmacdonald]

This new feature allows content authors to set an optional "review message" on each MCQ/MRQ/Rating question.

The review messages are shown in assessment mode when the student is reviewing their grade before being allowed to try again. The review screen now includes a list of topics to review, based on which questions the student got wrong.

Screenshot:

If the student got all the questions right or has no attempts remaining, the new review section is not shown.

Here's how it looks to course authors in Studio:

[e-kolpakov]

@bradenmacdonald nit: could be lazy - lazies are per-instance, so even if you're in mixin each object of every class will get it's own lazy property.

[bradenmacdonald]

@e-kolpakov Using lazy would be nice, but it would force this to become an attribute rather than a method, and I'd like this to be a method so that block.steps and block.get_steps() follows the same API pattern as block.children / block.get_children() and block.parent / block.get_parent()

[e-kolpakov]

@bradenmacdonald makes sense

[e-kolpakov]

@bradenmacdonald one minor nit: the formatting of the list is different from the design doc. Here's what I see:

And it should look like this:

Note the left margin and line spacing in the list. Also, the list itself should be positioned under the submit, not above.

[e-kolpakov]

@bradenmacdonald one more thing :) It looks like the code does not support more than a single assessment message, but one can add any number of them in Studio. It would be nice to allow single instance only, similar to how it's made for mentoring-level messages.

Also, latest xblock-utils provide a mixin for handling nested blocks in a more "declarative" way - you might want to check it out: StudioContainerWithNestedXBlocksMixin. Examples are in GP v2 utils-update branch (virtually everywhere, but GroupProjectXBlock is probably the richest example so far).

Simply put:

• Parent block need to inherit from the StudioContainerWithNestedXBlocksMixin (which inherits from StudioContainerXBlockMixin), and override allowed_nested_blocks property to contain a list of blocks that we want to add to it via Studio as either:
  ** XBlock class - simple mode, just allows unlimited children of the type
  ** NestedXBlockSpec instance - provides more config options including only allowing single instance.
• Nested Blocks (i.e. in this case Answer, Tip and Message) need to have the follwoing constants set:
  ** CATEGORY - XBlock category as set on add button (and setup.py entrypoint)
  ** STUDIO_LABEL - human-readable name for an XBlock

If any css/js needs to be added you can just override the author_preview_view and author_edit_view. Also, for hiding the studio header in preview - I've took another option to not use any view that studio appends the header to, so XBlockWithPreviewMixin was created.

[bradenmacdonald]

@e-kolpakov I think I've addressed all your comments.

I tried using StudioContainerWithNestedXBlocksMixin and it looks really nice - I just ran into two issues:

1. Does it include any CSS by default? The buttons were the wrong shape:
   
2. I can't use it with pb-message, because the MentoringMessageBlock has many different names in Studio - the STUDIO_LABEL depends on the boilerplate value. If you add some way to override the label when defining a NestedXBlockSpec then it could work.

I'd love to refactor the whole problem builder to use StudioContainerWithNestedXBlocksMixin but I think that's outside the scope of this PR.

[e-kolpakov]

@bradenmacdonald the rules we used for those buttons were xblock-specific. Also, preview and edit views usually need studio view rules applied in order to look more or less like the actual student content. So I decided it would make sense to not include the css into StudioContainerWithNestedXBlocksMixin - instead, inherting class would just build the fragment via single super call and append required css, like this.

On the other hand, it actually might make sense to include basic rules for buttons. If you send a PR against utils I'll be happy to review it, otherwise (if you decide to go with adding css in descendant class) I'll include that next time I get to work on it.

Same thing about NestedXBlock. I believe just adding a kwarg label to override block label would just do the trick.

Also, it might be a good idea to log this feature requests as issues on utils repo - I'll go create them. :)

UPD:

• StudioContainerWithNestedXBlocksMixin - default css for buttons openedx-unsupported/xblock-utils#25
• NestedXBlockSpec - add ability to override label openedx-unsupported/xblock-utils#26

[e-kolpakov]

@bradenmacdonald positioning issue is fixed, but now "If you retake..." message persists when user is retaking the assessment:

Is that intentional?

[bradenmacdonald]

Good catch, thanks. I will fix.

[e-kolpakov]

@bradenmacdonald 👍