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

refactor: add enrollment start #38

Merged
merged 1 commit into from
Feb 8, 2023
Merged

Conversation

dyudyunov
Copy link
Contributor

Set the enrollment start date to the same value as the course start date.

This change is needed because in openedx/edx-platform#30954 default enrollment start was added to fix the course listing for an anonymous user on the index page.

Using the demo course with an empty enrollment date for import you'll get the course with the default enrollment date equal to the default course start (1 Jan 2030 for now). This will cause the deployment error when trying to enroll audit and verified users in the demo course.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 4, 2022
@openedx-webhooks
Copy link

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

@itsjeyd
Copy link

itsjeyd commented Nov 3, 2022

@dyudyunov Thank you for your contribution!

Are the changes ready for review?

@dyudyunov
Copy link
Contributor Author

@itsjeyd hi!
yes, it's ready

@itsjeyd
Copy link

itsjeyd commented Nov 15, 2022

Thanks @dyudyunov!

@arbrandes @feanil Would you be able to update the status of this PR on the contributions board (i.e., move it to Ready for Review)? Michelle said to ping you since she doesn't yet have the necessary permissions to take care of that for PRs from this repo yet.

CC @mphilbrick211

@mphilbrick211
Copy link

@itsjeyd @jristau1984 flagging this as this needs to be review/merged in order to unblock openedx/edx-platform#30954 (comment)

@itsjeyd
Copy link

itsjeyd commented Dec 21, 2022

@mphilbrick211 Noted, thanks for the ping. I went ahead and labeled openedx/edx-platform#30954 as blocked-on-other-work, and updated its status on the contributions board.

@jristau1984 Could you take it from here, please? :)

@itsjeyd
Copy link

itsjeyd commented Jan 6, 2023

Hi @jristau1984, just a friendly nudge about taking a look at this PR.

Copy link

@jristau1984 jristau1984 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Sorry for the delay, I was on extended holiday leave. For future items like this, you could also @ teaching-and-learning for more eyes. Thanks!

@jmakowski1123 jmakowski1123 added the product review PR requires product review before merging label Jan 10, 2023
@itsjeyd
Copy link

itsjeyd commented Jan 11, 2023

Thanks @jristau1984, will do.

Looks like we're not quite ready to merge this yet as it also needs product review.

@jmakowski1123 Since this PR is blocking another one (openedx/edx-platform#30954), it might make sense for product reviewers to try and prioritize it (?).

@itsjeyd
Copy link

itsjeyd commented Jan 24, 2023

Hi @jmakowski1123, do you think it would be possible to get this PR lined up for product review some time soon? That would help unblock openedx/edx-platform#30954 which seems to be close to merging.

CC @mphilbrick211

@jmakowski1123
Copy link

@mphilbrick211 I will plan to review this early next week.

@itsjeyd
Copy link

itsjeyd commented Jan 31, 2023

Thanks @jmakowski1123!

@cablaa77
Copy link

Hi @dyudyunov, I am the product manager for TNL (edX Studio). I think this is a good improvement that our course authors will appreciate and it is approved.

@itsjeyd
Copy link

itsjeyd commented Feb 8, 2023

Thank you @cablaa77!

@jristau1984 This was approved by product without requiring further changes, so I'm changing status to Ready to Merge. Could you take it from here and hit the button?

CC @openedx/teaching-and-learning

@connorhaugh connorhaugh merged commit 8975f1f into openedx:master Feb 8, 2023
@openedx-webhooks
Copy link

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

@jmakowski1123
Copy link

@mphilbrick211 Brad already approved this one, it's good to merge!

@dyudyunov dyudyunov deleted the nutmeg-rg branch March 22, 2023 12:59
@dyudyunov dyudyunov restored the nutmeg-rg branch March 22, 2023 12:59
sambapete pushed a commit to EDUlib/openedx-demo-course that referenced this pull request Apr 11, 2023
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.

8 participants