-
Notifications
You must be signed in to change notification settings - Fork 196
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
feat: selectable students waiting step #2025
feat: selectable students waiting step #2025
Conversation
Thanks for the pull request, @johnvente! 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:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
openassessment/xblock/static/js/spec/lms/components/WaitingStepList.spec.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/spec/lms/components/WaitingStepList.spec.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/spec/lms/components/WaitingStepList.spec.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/spec/lms/components/WaitingStepList.spec.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepContent.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepContent.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepList.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/containers/WaitingStepDetailsContainer.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/containers/WaitingStepDetailsContainer.jsx
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepContent.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepContent.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments for you to consider! Thanks
Hey @mariajgrimaldi! I've made the changes. Any additional suggestions I will be attentive to review them |
openassessment/xblock/static/js/src/lms/components/WaitingStepContent.jsx
Outdated
Show resolved
Hide resolved
openassessment/xblock/static/js/src/lms/components/WaitingStepContent.jsx
Show resolved
Hide resolved
Hi @pomegranited! I've added the doc for the feature flag and updated the version. Please let me know if there is any changed needed it. Thanks a lot |
Hey @pomegranited, just checking in to see when you'd be able to complete another review pass here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 This change works as described, thank you @johnvente :)
Could you merge latest master
, fix the conflicts, and bump the version strings again? I can merge once that's done.
- I tested this on my devstack using the PR test instructions (which were fantastic 💯 )
- I read through the code
- I checked for accessibility issues by using my keyboard to navigate.
- Includes documentation of the added feature flag.
- User-facing strings are extracted for translation
67c1470
to
88f1ce2
Compare
88f1ce2
to
95892cb
Compare
78853d9
to
8d8f657
Compare
Hi @pomegranited Thanks for the review here we really appreciate it. I've solve conflicts and the comment that you mentioned here Now this is ready to be merged if there is any concern please let me know. Thanks a lot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @johnvente !
I found one teeny nit in a test file (sorry I missed this on my first pass!). But once that's fixed, I will merge.
- I tested this on my devstack with the new feature flag undefined in edx-platform, with the flag defined and False, and with the flag set to True.
- I read through the code
- I checked for accessibility issues by using my keyboard to navigate.
- Includes documentation
- User-facing strings are extracted for translation
xblock_arg_path = "//script[contains(@type, 'json/xblock-args')]" | ||
|
||
xblock_args_el = tree.xpath(xblock_arg_path) | ||
json.loads(xblock_args_el[0].text)['CONTEXT']['selectable_learners_enabled'] = esg_flag_input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be another assert here:
json.loads(xblock_args_el[0].text)['CONTEXT']['selectable_learners_enabled'] = esg_flag_input | |
assert json.loads(xblock_args_el[0].text)['CONTEXT']['selectable_learners_enabled'] == esg_flag_input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pomegranited Done!
@johnvente 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Description
This PR adds a toggable feature that allows instructors to automatically search a student after selecting it from the ORA waiting list table:
Here's what you see in the video explained:
Search
button appearsNow the instructor can grade the student.
Why are we proposing these changes? This improvement makes it easier for the instructors to grade ORA submissions without copying and pasting each username for a manual search. As said before, this feature can be turned on and off in case you want the default ORA behavior.
How to test
Peer Assessment
.-> Select Step Peer Assessment option.
Must Grade
: 1Grade By
: 1instructor
, go to the Instructor panel ->Open Responses
->Waiting
column.instructor
, you can see the list of students. Select the one that you want to review and click on theSearch Learner
button.DETAILED INSTRUCTIONS, WITH CONFIGURATIONS
You must enable these two features
Here an example to add them in tutor:
Developer Checklist
Reviewer Checklist
Collectively, these should be completed by reviewers of this PR:
FYI: @openedx/content-aurora