Click handlers and backend processing for assessment mode #87

e-kolpakov · 2016-08-02T15:11:50Z

Description: This PR actually implements submitting attempts for Assessment Mode. As of this PR, the minimum viable product implementation of Assessment Mode in Drag and Drop v2 is complete.

Dependencies: #86 (now merged)

JIRA: Part of SOL-1944

Discussions: Discovery document for DnDv2 assessment mode (relevant section: 2.2.3)

Sandbox URLs:

Updated 2016-08-12 to commit 5b837e7

Testing instructions:

Create/edit DnDv2; put it into assessment mode
Set "Max attempts" to N: N > 2
Publish, open in LMS.
Note that "Submit" button is displayed. Make sure problem is in its initial state (reset if needed). Submit button should be disabled. Attempts info should not be displayed.
Put any item to correct zone. "Submit" button should become enabled.
Click "Submit". Expected to happen:
6.1. Submit button shows spinner until AJAX request is sent
6.2. When AJAX is completed, incorrect items are returned to item bank, other items stay where they were.
6.3. Feedback message is updated to contain info about correctly placed items, misplaced items and items that should be put, but was not. If there were any misplaced items, feedback should also contain note about them being returned to item bank. If "initial" overall feedback is specified, it should be shown as well.
6.4. Attempts info should increase the number of used attempts.
Use N-1 attempts to test various combinations of correct, misplaced and missing items (note pluralization should be appropriate). Same things as in (6) should happen.
Submit final attempt. Expected to happen:
8.1. Same as 6.1. - spinner
8.2. All items stay where they were (misplaced are not returned to bank, as opposed to 6.2)
8.3. All of 6.3 + note about using final attempt and final grade are shown.
8.4. Attempts info should read "Used N of N attempts".
8.5. Submit and Reset buttons are permanently disabled.

Reviewers

@mtyaka
TNL: @cahrens and/or @staubina
a11y: @cptvitamin
Product: @sstack22

openedx-webhooks · 2016-08-02T15:11:52Z

Thanks for the pull request, @e-kolpakov! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: https://openedx-webhooks.herokuapp.com/github/process_pr?repo=edx-solutions%2Fxblock-drag-and-drop-v2&number=87

mtyaka · 2016-08-03T04:16:36Z

drag_and_drop_v2/drag_and_drop_v2.py

@@ -412,12 +382,120 @@ def default_background_image_url(self):
        """ The URL to the default background image, shown when no custom background is used """
        return self.runtime.local_resource_url(self, "public/img/triangle.png")

+    @property
+    def attemps_remain(self):
+        return self.max_attempts is None or self.num_attempts < self.max_attempts


We might want to allow both None (empty) and 0 values of max_attempts to mean "unlimited attempts". It would be consistent with some other problem types where 0 means no limit.

mtyaka · 2016-08-03T09:56:47Z

drag_and_drop_v2/drag_and_drop_v2.py

            feedback_msgs.append(_('Misplaced items were returned to item bank.'))
-        else:
+
+        if not misplaced_ids and not missing_ids:


I was about to suggest to replace this with if self._is_finished() to make it clearer, but then I realized that _is_finished() is currently a bit broken in assessment mode, because it does not check decoy items. _is_finished() should only return true if all non-decoy items are in their correct positions and all decoy items are in the bank.

@mtyaka the problem is slightly deeper, so I've submitted https://tasks.opencraft.com/browse/OC-1829. Won't fix this as part of this PR due to very tight deadline on this one.

mtyaka · 2016-08-03T10:06:42Z

@e-kolpakov I noticed a problem when refreshing the page after you've dragged some items to the board. The items that are in correct positions are disabled (faded out) - they shouldn't be.

Also I think it would be good if users were required to do some changes before they are allowed to submit the problem again. Currently you can keep clicking the submit button and it will keep submitting the identical solution. I think in some other problem types (such as problem builder MRQs) we require the user to change their answer before they can submit again.
This is probably be out of scope for this task, though.

Edit: I just checked standard CAPA checkbox problem, and it does let you submit the same answer over and over again, so perhaps we don't have to worry about that.

e-kolpakov · 2016-08-03T12:21:56Z

@mtyaka addressed review notes, please review

mtyaka · 2016-08-03T13:38:48Z

drag_and_drop_v2/utils.py

+
+class FeedbackMessages(object):
+    FINAL_ATTEMPT_TPL = _('Final attempt was used, final score is {score}')


@e-kolpakov These strings will get translated too early, see: http://edx.readthedocs.io/projects/edx-developer-guide/en/latest/conventions/internationalization/i18n.html#translating-too-early
One possible way around that would be to change these constants to functions (ie. FeedbackMessages.final_attempt(score), FeedbackMessages.correctly_placed(correct_count), etc).

mtyaka · 2016-08-03T14:07:31Z

@e-kolpakov This problem mentioned above is still present:

I noticed a problem when refreshing the page after you've dragged some items to the board. The items that are in correct positions are disabled (faded out) - they should NOT be disabled.

staubina · 2016-08-12T12:48:37Z

drag_and_drop_v2/drag_and_drop_v2.py

        """
-        All items are at their correct place and a value has been
-        submitted for each item that expects a value.
+        Checks answer correctness:


same as last comment on docstrings

cahrens · 2016-08-12T13:37:30Z

The feedback area looks better-- thank you. At some point, it would be good to sync up with the feedback style being developed for capa (see https://github.com/edx/edx-platform/pull/13149 and https://openedx.atlassian.net/browse/TNL-4881, and take care to look at the most recent screenshots).

I also wonder about focus not going to the feedback area, but see that you have an aria-live region on the feedback section. So I think it's fine to wait and see what the a11y review produces.

I'm going to review the code now.

cahrens · 2016-08-12T13:40:06Z

drag_and_drop_v2/drag_and_drop_v2.py

@@ -127,7 +143,7 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
        default={},
    )

-    num_attempts = Integer(
+    attempts = Integer(


This has never been released previously, correct? Just want to make sure there is no data migration issue.

@cahrens As far as I can tell - no, it has never been released.

Correct - this field was introduced in the previous PR, #86, and has never been released.

e-kolpakov · 2016-08-12T13:43:39Z

@cahrens

I also wonder about focus not going to the feedback area

We've had that discussion already - I've got an impression the idea was to wait on a11y to weigh in on this, so I haven't modified this in this PR (despite that line being marked as changed - I've just rearranged the code around it)

cahrens · 2016-08-12T13:56:08Z

drag_and_drop_v2/drag_and_drop_v2.py

+
+        self._publish_item_dropped_event(item_attempt, is_correct)
+
+        return {}


Why does this method return an empty dict, while _drop_item_standard returns the correct and feedback data? They are both called by drop_item- what is the contract for what that method should return?

@cahrens In standard mode, when item is dropped into a zone, learner get feedback (both per-item and overall) immediately.

In assessment mode, when an item is dropped into a zone, learner get no feedback at all. Instead feedback is displayed after an attempt is submitted, which is handled by do_attempt.

cahrens · 2016-08-12T17:46:19Z

I am done reviewing. I have a couple suggestions above, but nothing that I would consider blocking (for the purposes of getting something out that MIT can start working with).

But in addition to the a11y review that will be done next week, there also needs to be a ticket for UX review and testing/bug-fixing of RTL mode-- please include a link to that ticket.

With those caveats, 👍

e-kolpakov · 2016-08-15T11:20:56Z

@cahrens squashed from 5269326

e-kolpakov · 2016-08-15T11:21:21Z

@sstack22 @mtyaka ^^

e-kolpakov · 2016-08-15T11:25:41Z

@cahrens created https://openedx.atlassian.net/browse/SOL-2030 for UX follow up. I've logged a ticket in Opnecraft internal tracker for RTL fixes, I just want acceptance criteria and wording to be reviewed and/or polished by @bradenmacdonald first.

e-kolpakov · 2016-08-15T11:29:25Z

@sstack22 I believe this PR just waits for your approval now - could you please review it today (no pressure, but we're significantly overdue here) and ping me so I could push it forward? Thank you!

pdesjardins · 2016-08-15T13:39:41Z

@bradenmacdonald, for documentation planning, is it possible that this will merge into production in this week's release (August 15th)?

e-kolpakov · 2016-08-15T14:51:04Z

@pdesjardins Looks like Braden is not available yet, so I'll try to answer to the best of my knowledge.

As far as I know, the plan is to include this changeset into today's release. This could happen if @sstack22 approves the PR today; today = 3-4 hours from now for me, and probably something like 8-10 hours for Braden (obviously, he knows better). If it happens, we'll merge this PR and push a version bump to release candidate branch.

sstack22 · 2016-08-15T15:12:20Z

@e-kolpakov, @bradenmacdonald - 👍 on my end. This is great work! I'll comment on the follow-up UX ticket to iron out those future adjustments.

e-kolpakov · 2016-08-15T15:14:15Z

@sstack22 Thank you - merging!

mtyaka reviewed Aug 3, 2016
View reviewed changes

e-kolpakov force-pushed the ekolpakov/assessment-submit branch from ed8ad0d to 3733581 Compare August 3, 2016 07:17

mtyaka reviewed Aug 3, 2016
View reviewed changes

e-kolpakov force-pushed the ekolpakov/assessment-submit branch from 991c3d5 to 811f5a1 Compare August 3, 2016 10:58

e-kolpakov changed the title ~~(WIP) Click handlers and backend processing for assessment mode~~ Click handlers and backend processing for assessment mode Aug 3, 2016

mtyaka reviewed Aug 3, 2016
View reviewed changes

staubina reviewed Aug 12, 2016
View reviewed changes

cahrens reviewed Aug 12, 2016
View reviewed changes

e-kolpakov force-pushed the ekolpakov/assessment-submit branch from 5269326 to a00309d Compare August 15, 2016 11:23

e-kolpakov force-pushed the ekolpakov/assessment-submit branch from a00309d to a134bc9 Compare August 15, 2016 12:18

[SOL-1944] Submitting answers in assessment mode

a0028a7

e-kolpakov force-pushed the ekolpakov/assessment-submit branch from a134bc9 to a0028a7 Compare August 15, 2016 13:06

e-kolpakov merged commit 90c54f0 into openedx:master Aug 15, 2016

e-kolpakov deleted the ekolpakov/assessment-submit branch August 15, 2016 15:14

arbrandes mentioned this pull request Aug 18, 2016

[SOL-1988] Account for decoy items in score #96

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Click handlers and backend processing for assessment mode #87

Click handlers and backend processing for assessment mode #87

e-kolpakov commented Aug 2, 2016 •

edited

Loading

openedx-webhooks commented Aug 2, 2016

mtyaka Aug 3, 2016

mtyaka Aug 3, 2016

e-kolpakov Aug 3, 2016

mtyaka commented Aug 3, 2016 •

edited

Loading

e-kolpakov commented Aug 3, 2016

mtyaka Aug 3, 2016

mtyaka commented Aug 3, 2016

staubina Aug 12, 2016

cahrens commented Aug 12, 2016

cahrens Aug 12, 2016

e-kolpakov Aug 12, 2016

bradenmacdonald Aug 12, 2016

e-kolpakov commented Aug 12, 2016 •

edited

Loading

cahrens Aug 12, 2016

e-kolpakov Aug 12, 2016

cahrens commented Aug 12, 2016

e-kolpakov commented Aug 15, 2016 •

edited

Loading

e-kolpakov commented Aug 15, 2016

e-kolpakov commented Aug 15, 2016

e-kolpakov commented Aug 15, 2016

pdesjardins commented Aug 15, 2016

e-kolpakov commented Aug 15, 2016

sstack22 commented Aug 15, 2016

e-kolpakov commented Aug 15, 2016


		class FeedbackMessages(object):
		FINAL_ATTEMPT_TPL = _('Final attempt was used, final score is {score}')


		self._publish_item_dropped_event(item_attempt, is_correct)

		return {}

Click handlers and backend processing for assessment mode #87

Click handlers and backend processing for assessment mode #87

Conversation

e-kolpakov commented Aug 2, 2016 • edited Loading

openedx-webhooks commented Aug 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtyaka commented Aug 3, 2016 • edited Loading

e-kolpakov commented Aug 3, 2016

Choose a reason for hiding this comment

mtyaka commented Aug 3, 2016

Choose a reason for hiding this comment

cahrens commented Aug 12, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

e-kolpakov commented Aug 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cahrens commented Aug 12, 2016

e-kolpakov commented Aug 15, 2016 • edited Loading

e-kolpakov commented Aug 15, 2016

e-kolpakov commented Aug 15, 2016

e-kolpakov commented Aug 15, 2016

pdesjardins commented Aug 15, 2016

e-kolpakov commented Aug 15, 2016

sstack22 commented Aug 15, 2016

e-kolpakov commented Aug 15, 2016

e-kolpakov commented Aug 2, 2016 •

edited

Loading

mtyaka commented Aug 3, 2016 •

edited

Loading

e-kolpakov commented Aug 12, 2016 •

edited

Loading

e-kolpakov commented Aug 15, 2016 •

edited

Loading