openedx / openedx-demo-course

A demonstration course that can be imported into an Open edX instance
GNU Affero General Public License v3.0
46 stars 68 forks source link

refactor: add enrollment start #38

Closed dyudyunov closed 1 year ago

dyudyunov commented 2 years ago

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

This change is needed because in https://github.com/openedx/edx-platform/pull/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 commented 2 years ago

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:

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 commented 2 years ago

@dyudyunov Thank you for your contribution!

Are the changes ready for review?

dyudyunov commented 1 year ago

@itsjeyd hi! yes, it's ready

itsjeyd commented 1 year ago

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 commented 1 year ago

@itsjeyd @jristau1984 flagging this as this needs to be review/merged in order to unblock https://github.com/openedx/edx-platform/pull/30954#issuecomment-1357851904

itsjeyd commented 1 year ago

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

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

itsjeyd commented 1 year ago

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

itsjeyd commented 1 year ago

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 (https://github.com/openedx/edx-platform/pull/30954), it might make sense for product reviewers to try and prioritize it (?).

itsjeyd commented 1 year ago

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 https://github.com/openedx/edx-platform/pull/30954 which seems to be close to merging.

CC @mphilbrick211

jmakowski1123 commented 1 year ago

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

itsjeyd commented 1 year ago

Thanks @jmakowski1123!

cablaa77 commented 1 year ago

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 commented 1 year ago

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

openedx-webhooks commented 1 year ago

@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 commented 1 year ago

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