Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for limiting number of attempts in assessment mode to Studio editor #81

Merged
merged 4 commits into from
Jul 29, 2016

Conversation

itsjeyd
Copy link
Contributor

@itsjeyd itsjeyd commented Jul 19, 2016

Description

This PR implements the following use case1:

As a course author, I want to be able to limit the number of times a learner may attempt a drag-and-drop problem in assessment mode. More specifically, I want to be able to specify that a learner may try a problem one time, n times, or as many times as they wish (unlimited attempts).

1Note that learner-facing functionality related to limiting the number of available attempts will be covered by future PRs.

UX

For drag-and-drop problems in standard mode, the interactive Studio editor does not show an option for setting the maximum number of attempts.

For drag-and-drop problems in assessment mode, the interactive Studio editor shows a numeric input field that allows course authors to specify the maximum number of attempts. The field is hidden until a course author sets the mode of a drag-and-drop problem to "assessment mode" (as described above).

  • By leaving the maximum number of attempts unset, course authors can indicate that the number of times learners may attempt the problem is unlimited.
  • By setting the maximum number of attempts to a positive number n, course authors can indicate that learners may attempt the problem exactly n times.

The value of the “Maximum attempts” setting defaults to “not set” (unlimited attempts).

Switching modes does not affect the current value of the “Maximum attempts” field.

JIRA tickets: SOL-1941

Discussions: The feature implemented in this PR was discussed with @marcotuts as part of his review of the discovery document for DnDv2 assessment mode.

Dependencies: None

Screenshots:

Standard mode:

edit-view-standard

Assessment mode:

edit-view

Sandbox URL:

Studio: http://studio-dndv2-sandbox2.opencraft.hosting/
LMS: http://dndv2-sandbox2.opencraft.hosting/

The sandbox is currently using version 40037b1 of DnDv2.

Testing instructions:

To test from scratch (using sandbox or local development environment):

  1. Add a DnDv2 block to a unit in Studio.
  2. Click "EDIT". Observe that "Problem mode" is set to "standard" (cf. Problem mode field and edit capabilities #77), and that the "Maximum attempts" setting is not visible.
  3. Change "Problem mode" to "assessment". Observe that the "Maximum attempts" setting becomes visible below the "Problem mode" setting.
  4. (Optionally) Change the value of the "Maximum attempts setting.
  5. Continue to the last pane of the Studio editor, then click "Save".
  6. Click "EDIT" again. Observe that the "Maximum attempts setting is still visible (and set to whatever value it had when you clicked "Save").
  7. Change "Problem mode" to "standard". Observe that the "Maximum attempts" setting is hidden.
  8. Save, click "EDIT" again, and switch "Problem mode" back to "assessment". Observe that the value of the "Maximum attempts" has been preserved from when you last changed it (or is still at the default if you chose not to change it earlier).

To test using existing DnDv2 instances, access

"PR Review Testing" > "Maximum attempts" > "Standard mode"

and/or

"PR Review Testing" > "Maximum attempts" > "Assessment mode"

units on the sandbox (they are part of the "Drag and Drop Demos" course) and perform relevant steps listed above.

Reviewers

Also tagging @pdesjardins as a docs FYI.

@openedx-webhooks
Copy link

Thanks for the pull request, @itsjeyd! 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=81

@cahrens
Copy link

cahrens commented Jul 19, 2016

@itsjeyd Thanks for the heads-up. Please let me know when the initial review by @mtyaka is complete, and then @staubina and I will do a second review.

@mtyaka
Copy link
Contributor

mtyaka commented Jul 20, 2016

@itsjeyd The code works and looks good.

The discovery document says to set the scope of the max_attempts field to Scope.content, but you are setting it to Scope.settings. Is that intentional? I guess Scope.settings is more appropriate.

This PR adds two new strings (field label and help text) that should be added to .po files. Should that be done in this PR or do we have a separate process for extracting translations strings (similar to edx-platform)? Last time I added a new string to this repository I just manually added it to the po files.

@itsjeyd
Copy link
Contributor Author

itsjeyd commented Jul 20, 2016

@mtyaka Thanks for reviewing!

Yes, I'm intentionally setting the scope of max_attempts to Scope.settings: I had a closer look at Scope.settings vs. Scope.content now than I did when I put together the discovery document, and couldn't really find a reason why the scope of max_attempts should be different from the scope of the other settings that can be modified in Studio.

Good catch about the strings! AFAIK we don't have a separate process for extracting translations, so I just updated the .po files manually like you suggested (I also did that for the strings from #77).

Please have a look at the updates (the first commit is just a cosmetic change that doesn't alter any logic).

@mtyaka
Copy link
Contributor

mtyaka commented Jul 20, 2016

@itsjeyd Thanks for the explanation and updates!

👍

  • I followed the testing instructions locally: everything works as advertised
  • I read through the code
  • Includes documentation

@itsjeyd
Copy link
Contributor Author

itsjeyd commented Jul 20, 2016

Thanks @mtyaka!

@cahrens @staubina This is ready for you to have a look. Please see the PR description for sandbox links. Thanks!

@@ -492,6 +511,7 @@ function DragAndDropEditBlock(runtime, element, params) {
var data = {
'display_name': $element.find('#display-name').val(),
'mode': $element.find("#problem-mode").val(),
'max_attempts': $element.find("#max-attempts").val(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend that you use classes instead of ids-- it is a general best practice to avoid ids.

- Wrapp input field for max_attempts in label to be able to use classes instead of ids.
@itsjeyd
Copy link
Contributor Author

itsjeyd commented Jul 22, 2016

@cahrens Thanks for the review! I addressed your comments (40037b1), please have another look.

@cahrens
Copy link

cahrens commented Jul 22, 2016

👍 Will there be end-to-end tests written once the LMS part of the functionality is complete?

@staubina
Copy link
Contributor

👍 Also wondering about @cahrens question.

Although testing the UX is helpful, it would be good to test this with the backend functionality.

@itsjeyd
Copy link
Contributor Author

itsjeyd commented Jul 22, 2016

Thanks @cahrens and @staubina! Yes, we'll add integration tests that verify behavior for problems with limited/unlimited attempts in upcoming tickets (when implementing functionality for submitting answers in assessment mode).

@cptvitamin Would you like to have a quick look at this PR as well?

@itsjeyd
Copy link
Contributor Author

itsjeyd commented Jul 25, 2016

@edx/edx-accessibility This PR is ready for you guys to have a look (repeating my earlier ping based on info about this new tag that was posted on the mailing list).

@sstack22
Copy link

+1 on the studio functionality.

@itsjeyd
Copy link
Contributor Author

itsjeyd commented Jul 26, 2016

Thanks for the review, @sstack22!

@mtyaka mtyaka mentioned this pull request Jul 28, 2016
4 tasks
@bradenmacdonald
Copy link
Contributor

Merging this now. It's a small Studio-editor-only change, so I believe it is unlikely to have a11y concerns, but we now have a ticket, SOL-1978, for a final / follow-up a11y review of any assessment mode functionality that didn't get an a11y review before our deadlines.

@bradenmacdonald bradenmacdonald merged commit cd68989 into master Jul 29, 2016
@bradenmacdonald bradenmacdonald deleted the tim/max-attempts branch July 29, 2016 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants