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

feat: selectable students waiting step #2025

Merged

Conversation

johnvente
Copy link
Contributor

@johnvente johnvente commented Aug 9, 2023

Description

This PR adds a toggable feature that allows instructors to automatically search a student after selecting it from the ORA waiting list table:

ora-update-demo

Here's what you see in the video explained:

  • There's a student in the ORA waiting list after not receiving an assessment from their peers
  • The instructor selects the Open Responses tab from the instructor dashboard
  • The table shows the selectable column
  • The instructor selects the number of students waiting for an assessment
  • The instructor selects the student from the table
  • A Search button appears
  • The instructor presses the search button
  • The information on the student's submission is shown
    Now 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

  • Create at least two students.
  • Go to your course -> Create a new unit -> Create an open response component -> Peer Assessment.
  • Then edit the open response -> Edit -> Assessment Steps.
    -> Select Step Peer Assessment option.
    Must Grade: 1
    Grade By: 1
  • As students upload the open responses:
  • Have student 1 fill out the open response and submit.
  • Have student 2 fill out the open response, submit, and check student 1's open response.
  • As an instructor, go to the Instructor panel ->Open Responses-> Waiting column.
  • As an instructor, you can see the list of students. Select the one that you want to review and click on the Search Learner button.

DETAILED INSTRUCTIONS, WITH CONFIGURATIONS

You must enable these two features

FEATURES["ENABLE_XBLOCK_VIEW_ENDPOINT"] = True
FEATURES["ENABLE_ORA_SELECTABLE_LEARNER_WAITING_REVIEW"] = True

Here an example to add them in tutor:

from tutor import hooks

hooks.Filters.ENV_PATCHES.add_items(
    [
        (
            "openedx-lms-development-settings",
            """
FEATURES["ENABLE_XBLOCK_VIEW_ENDPOINT"] = True
FEATURES["ENABLE_ORA_SELECTABLE_LEARNER_WAITING_REVIEW"] = True

"""
        ),
    ]
)

Developer Checklist

Reviewer Checklist

Collectively, these should be completed by reviewers of this PR:

  • I've done a visual code review
  • I've tested the new functionality

FYI: @openedx/content-aurora

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 9, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Aug 9, 2023

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:

  • 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.

Please let us know once your PR is ready for our review and all tests are green.

Copy link
Member

@mariajgrimaldi mariajgrimaldi left a 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

@johnvente johnvente changed the title feat: initial waiting approche feat: selectable students waiting step Aug 10, 2023
@johnvente
Copy link
Contributor Author

johnvente commented Aug 10, 2023

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

@johnvente
Copy link
Contributor Author

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

@itsjeyd itsjeyd added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Feb 16, 2024
@itsjeyd
Copy link

itsjeyd commented Feb 22, 2024

Hey @pomegranited, just checking in to see when you'd be able to complete another review pass here?

Copy link
Contributor

@pomegranited pomegranited left a 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

@pomegranited pomegranited added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Feb 28, 2024
@johnvente johnvente force-pushed the feat-waiting-learners-selectables branch from 67c1470 to 88f1ce2 Compare February 29, 2024 13:16
@johnvente johnvente force-pushed the feat-waiting-learners-selectables branch from 88f1ce2 to 95892cb Compare February 29, 2024 13:26
@johnvente johnvente force-pushed the feat-waiting-learners-selectables branch 2 times, most recently from 78853d9 to 8d8f657 Compare February 29, 2024 14:03
@johnvente
Copy link
Contributor Author

👍 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.

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

Copy link
Contributor

@pomegranited pomegranited left a 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
Copy link
Contributor

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:

Suggested change
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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pomegranited pomegranited merged commit 0d510a1 into openedx:master Mar 1, 2024
9 checks passed
@openedx-webhooks
Copy link

@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.

@itsjeyd itsjeyd removed waiting for eng review PR is ready for review. Review and merge it, or suggest changes. waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U product review PR requires product review before merging
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.