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.

Sign up for GitHub

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

Jump to bottom

feat: Add toggle to allow redirecting to courseware after enrollment. #31715

Merged
merged 1 commit into from
Nov 9, 2023
Merged

feat: Add toggle to allow redirecting to courseware after enrollment. #31715

merged 1 commit into from
Nov 9, 2023

Conversation

xitij2000
Copy link
Contributor

Description

This change adds a new waffle switch to redirect a student to courseware after enrolment instead of the dashboard.

Supporting information

NA

Testing instructions

  • With a new toggle disabled, enrol a student into a course. They will be redirected to the dashboard after enrolment.
  • With a new toggle enabled, enrol a student into a course. They will be redirected to the course after enrolment.

Deadline

"None"

Other information

Private-ref: BB-7071

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

openedx-webhooks commented Feb 6, 2023 •
edited

Thanks for the pull request, @xitij2000! 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.

@xitij2000 xitij2000 force-pushed the kshitij/bb-7051/redirect-to-courseware branch from 7eb073a to 7d1433f Compare February 13, 2023 13:13
kaustavb12
kaustavb12 approved these changes Feb 13, 2023
View reviewed changes
Copy link
Contributor

@kaustavb12 kaustavb12 left a comment

Choose a reason for hiding this comment

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

👍

  • I tested this: Tested waffle switch works as described
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

This was referenced Feb 14, 2023
Closed
Merged
@mphilbrick211
Copy link

Hi @xitij2000 - is this ready for review/merge?

@xitij2000
Copy link
Contributor Author

@mphilbrick211 Yes, this is good to review.

@e0d
Copy link
Contributor

e0d commented Mar 2, 2023 •
edited by jmakowski1123

@jmakowski1123 feels like there should be a product ticket for this, agree?

Yes, I missed this one, thanks for flagging. cc @mphilbrick211

@xitij2000 Since this feature impacts learners directly, we need to some additional information for it to be reviewed by a product manager. Specifically, some clarity on why the current workflow is a pain point, and some screenshots or other visual of what the fix looks like. I've created a Feature Ticket to track product-related communication, could you please add the requisite product information to this ticket? Happy to chat if you have any questions.

@jmakowski1123 jmakowski1123 mentioned this pull request Mar 3, 2023
Open
7 tasks
@jmakowski1123 jmakowski1123 added the product review PR requires product review before merging label Mar 3, 2023
@xitij2000 xitij2000 force-pushed the kshitij/bb-7051/redirect-to-courseware branch from 7d1433f to 00a0ca8 Compare March 30, 2023 11:11
@xitij2000 xitij2000 force-pushed the kshitij/bb-7051/redirect-to-courseware branch from 00a0ca8 to 16c5451 Compare April 21, 2023 05:34
@jmakowski1123 jmakowski1123 mentioned this pull request Apr 24, 2023
Closed
7 tasks
@xitij2000 xitij2000 mentioned this pull request Jun 27, 2023
Merged
@xitij2000 xitij2000 force-pushed the kshitij/bb-7051/redirect-to-courseware branch from 16c5451 to dfb7f43 Compare June 28, 2023 08:36
This was referenced Aug 25, 2023
Open
Open
@mphilbrick211
Copy link

Hi @xitij2000 - bumping this as Product review is complete. Please see Ryan's comment here.

@mphilbrick211 mphilbrick211 removed the product review PR requires product review before merging label Oct 23, 2023
@xitij2000 xitij2000 force-pushed the kshitij/bb-7051/redirect-to-courseware branch from dfb7f43 to 36aee77 Compare October 24, 2023 03:23
@xitij2000
Copy link
Contributor Author

Hi @xitij2000 - bumping this as Product review is complete. Please see Ryan's comment here.

Thanks! I will update this PR in the next sprint and get it ready to merge.
CC: @Agrendalath

@Agrendalath
Copy link
Member

@xitij2000, I can review this next sprint. Please add me to the ticket and move it to the CC review status once the sprint starts.

@xitij2000 xitij2000 force-pushed the kshitij/bb-7051/redirect-to-courseware branch from 36aee77 to 9090485 Compare November 2, 2023 11:26
@open-craft-grove
Copy link

Sandbox update request received. Deployment will start soon.

@open-craft-grove
Copy link

Sandbox deployment started.

@open-craft-grove
Copy link

Sandbox deployment successful.

Sandbox LMS is available at pr-31715-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-31715-139931.staging.do.opencraft.hosting

@xitij2000
feat: Add toggle to allow redirecting to courseware after enrollment.
1dfa0bd 
This change adds a new waffle switch to redirect a student to coursware after
enrolment instead of the dashboard.
@xitij2000 xitij2000 force-pushed the kshitij/bb-7051/redirect-to-courseware branch from 9090485 to 1dfa0bd Compare November 6, 2023 06:25
Agrendalath
Agrendalath approved these changes Nov 9, 2023
View reviewed changes
Copy link
Member

@Agrendalath Agrendalath left a comment

Choose a reason for hiding this comment

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

👍

  • I tested this: the redirect works correctly
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation

@Agrendalath Agrendalath merged commit f494586 into openedx:master Nov 9, 2023
63 checks passed
@Agrendalath Agrendalath deleted the kshitij/bb-7051/redirect-to-courseware branch November 9, 2023 14:38
@openedx-webhooks
Copy link

@xitij2000 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@0x29a 0x29a mentioned this pull request Dec 31, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Agrendalath Agrendalath Agrendalath approved these changes

@kaustavb12 kaustavb12 kaustavb12 approved these changes

Assignees
No one assigned
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Contributions
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@xitij2000 @openedx-webhooks @mphilbrick211 @e0d @Agrendalath @open-craft-grove @edx-pipeline-bot @kaustavb12 @jmakowski1123