Added date configuration to Schedule & Details settings page by farhaanbukhsh · Pull Request #30314 · openedx/openedx-platform

farhaanbukhsh · 2022-04-25T21:13:04Z

Description

Adds the ability to configure Schedule & Details settings page with the ability to show dd/mm/yyyy format. Default setting maintains existing behavior and doesn't introduce anything new.
Why:
Some areas of the world operate with the day-first: dd/mm/yyyy rather than mm/dd/yyyy.

JIRA tickets: BB-5329

Discussions: Link to any public dicussions about this PR or the design/architecture. Otherwise omit this.

Dependencies: None

Screenshots:

Sandbox URL: TBD - sandbox is being provisioned.

Merge deadline: None

Testing instructions

Initialize lilac devstack to use branch
Look at the studio --> course(Demo Course) --> settings --> Schedule and Details
Go under the Course Schedule section and you will see the date format is MM/DD/YYYY
Now drop in the studio shell make dev.shell.studio
Edit /edx/etc/studio.yml add SCHEDULE_DETAIL_FORMAT: 'DD/MM/YYYY'

SCHEDULE_DETAIL_FORMAT: 'DD/MM/YYYY'

Restart studio make dev.restart-devserver.studio
Now check the page again you will see the date format has changed to DD/MM/YYYY

Author notes and concerns:

Worked above Added date configuration to Schedule & Details settings page #29879
No change to role(s)
There is no before or after, as the change includes default configuration to remain consistent with preexisting behavior.
https://github.com/open-craft/edx-platform/blob/jbcurtin/bb-5429-date-configuration/cms/envs/common.py#L459

openedx-webhooks · 2022-04-25T21:13:08Z

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

Cup0fCoffee · 2022-04-26T17:00:24Z

👍

I tested this
I read through the code
Includes documentation

farhaanbukhsh · 2022-04-27T06:22:08Z

@natabene This is up for review :)

natabene · 2022-04-29T11:53:13Z

@farhaanbukhsh Thank you for your contribution, I will line this up for our review.

This feature help to configure the date formatt in Schedule and Details settings page. Signed-off-by: Farhaan Bukhsh <farhaan@opencraft.com> Co-authored-by: Joseph Curtin <jbcurtin@opencraft.com>

sarina · 2022-06-01T14:29:56Z

A few questions:

Would it make sense to make this also override-able by individual courses? (I'm really not sure)
Would it make sense to put the date format in the description text as well, since this would potentially be a change for already-running courses? Ie, "First day the course begins (DD/MM/YYYY)" (I think the answer to this is a resounding yes. What about people switching between different Open edX instances with different formats?)
For already-running courses, if you make the instance-wide change during the course run do existing dates change format? (num. 2 above ties in to this question)

mariajgrimaldi · 2022-06-16T16:14:50Z

Hello! If it's okay with @sarina, I can help with this one @natabene

sarina · 2022-06-16T19:43:43Z

@mariajgrimaldi I definitely would like answers to my questions before this merges, I don't think that prevents you from beginning reviewing the implementation.

Ping @farhaanbukhsh - please see my above questions.

farhaanbukhsh · 2022-06-17T03:44:07Z

I am really sorry for the late reply @sarina 🙏🏾 , I was a bit caught up.

A few questions:

1. Would it make sense to make this also override-able by individual courses? (I'm really not sure)

No, I don't think it's a good idea to make it configurable on a course level, a system-wide setting makes it more uniform. The user working on a single instance will be aware of the format supported.

2. Would it make sense to put the date format in the description text as well, since this would potentially be a change for already-running courses? Ie, "First day the course begins (DD/MM/YYYY)" (I think the answer to this is a resounding yes. What about people switching between different Open edX instances with different formats?)

I feel it's good to have but it wouldn't make a huge difference since the format information is already conveyed by the placeholder. AFAIK this is a standard UX in a lot of places so the users are educated about this style.

I don't have a very strong opinion about adding the date format to the description. What do you think? 😇

3. For already-running courses, if you make the instance-wide change during the course run do existing dates change format? (num. 2 above ties in to this question)

Yes, it changes the format for the ongoing course as well and now I see your point of adding the date format in the description. It saves the time for the user to figure out which date format is being used. That definitely sounds like an improvement.

I hope I answered your questions and thanks a lot for thinking through this UI/UX in detail.

sarina · 2022-06-17T15:23:53Z

Yes, it changes the format for the ongoing course as well and now I see your point of adding the date format in the description. It saves the time for the user to figure out which date format is being used. That definitely sounds like an improvement.

OK. I would then add that UX then I'm a 👍🏻 . Your other answers make sense.

mariajgrimaldi

I left some comments. I'm eager to know what you think

mariajgrimaldi · 2022-06-23T21:35:02Z

+            date_placeholder_format = configuration_helpers.get_value_for_org(
+                course_module.location.org,
+                'SCHEDULE_DETAIL_FORMAT',
+                settings.SCHEDULE_DETAIL_FORMAT
+            ).upper()


I asked around what we should do about using site configurations since it's set to be deprecated. The initial response is not to add more technical debt given this will need to be removed once site config it's no longer used.

But I see you used the default to Django settings, so using site configurations is not obligatory. Great!

Note: there's a new note in the OEP-51 specifying what to consider before using a soon-to-be a deprecated feature:https://open-edx-proposals.readthedocs.io/en/latest/processes/oep-0021-proc-deprecation.html#accepted

mariajgrimaldi · 2022-06-23T21:39:10Z

+        if (placeholder == 'DD/MM/YYYY') {
+            datefield.datepicker({dateFormat: 'dd/mm/yy'});


Why is this specific case?

mariajgrimaldi · 2022-06-23T21:41:33Z

+# .. setting_name: SCHEDULE_DETAIL_FORMAT
+# .. setting_default: MM/DD/YYYY'
+# .. setting_description: Settings to configure the date format in Schedule & Details page
+SCHEDULE_DETAIL_FORMAT = 'MM/DD/YYYY'


Can we specify that this is configured globally?

mariajgrimaldi · 2022-07-08T15:31:02Z

    #   in the LMS and CMS.
    # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/429'
    'DISABLE_UNENROLLMENT': False,
+


I think this change is not needed?

farhaanbukhsh · 2022-07-23T13:21:37Z

@mariajgrimaldi @sarina Thanks for the review here. Unfortunately, we don't have an approved budget from the client to work further on the changes. I will keep you in the loop when we unlock some budget for this PR. Sorry again and thanks a bunch for all the work.

cc: @natabene

sarina · 2022-07-25T14:35:26Z

@farhaanbukhsh thanks for the heads up - if you're not planning to work further on this, you can close the PR.

farhaanbukhsh · 2022-07-25T14:49:05Z

@farhaanbukhsh thanks for the heads up - if you're not planning to work further on this, you can close the PR.

Sure @sarina thank you again! :)

openedx-webhooks · 2022-07-25T14:49:15Z

@farhaanbukhsh Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Apr 25, 2022

farhaanbukhsh force-pushed the farhaan/bb-5429-frontend-date-format branch from 25e53e6 to 0136653 Compare April 25, 2022 21:14

openedx-webhooks added awaiting prioritization and removed needs triage labels Apr 29, 2022

pkulkark mentioned this pull request May 20, 2022

[BB-6046] Prepare opencraft-release/maple.3 branch for edx-platform open-craft/openedx-platform#471

Merged

feat: Added date configuration to Schedule & Details settings page

64d44a8

This feature help to configure the date formatt in Schedule and Details settings page. Signed-off-by: Farhaan Bukhsh <farhaan@opencraft.com> Co-authored-by: Joseph Curtin <jbcurtin@opencraft.com>

pkulkark force-pushed the farhaan/bb-5429-frontend-date-format branch from ab2a740 to 64d44a8 Compare May 30, 2022 05:32

mariajgrimaldi self-assigned this Jun 21, 2022

mariajgrimaldi requested changes Jun 23, 2022

View reviewed changes

openedx-webhooks removed the awaiting prioritization label Jun 23, 2022

mariajgrimaldi reviewed Jul 8, 2022

View reviewed changes

farhaanbukhsh closed this Jul 25, 2022

0x29a deleted the farhaan/bb-5429-frontend-date-format branch September 21, 2022 14:59

0x29a mentioned this pull request Sep 21, 2022

[BB-6692] Prepare opencraft-release/nutmeg.2 branch for edx-platform open-craft/openedx-platform#490

Merged

0x29a restored the farhaan/bb-5429-frontend-date-format branch September 22, 2022 11:59

0x29a deleted the farhaan/bb-5429-frontend-date-format branch September 22, 2022 12:00

		if (placeholder == 'DD/MM/YYYY') {
		datefield.datepicker({dateFormat: 'dd/mm/yy'});

Uh oh!

Conversation

farhaanbukhsh commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing instructions

Uh oh!

openedx-webhooks commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Cup0fCoffee commented Apr 26, 2022

Uh oh!

farhaanbukhsh commented Apr 27, 2022

Uh oh!

natabene commented Apr 29, 2022

Uh oh!

sarina commented Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mariajgrimaldi commented Jun 16, 2022

Uh oh!

sarina commented Jun 16, 2022

Uh oh!

farhaanbukhsh commented Jun 17, 2022

Uh oh!

sarina commented Jun 17, 2022

Uh oh!

mariajgrimaldi left a comment

Choose a reason for hiding this comment

Uh oh!

mariajgrimaldi Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mariajgrimaldi Jul 22, 2022

Choose a reason for hiding this comment

Uh oh!

mariajgrimaldi Jun 23, 2022

Choose a reason for hiding this comment

Uh oh!

mariajgrimaldi Jun 23, 2022

Choose a reason for hiding this comment

Uh oh!

mariajgrimaldi Jul 8, 2022

Choose a reason for hiding this comment

Uh oh!

farhaanbukhsh commented Jul 23, 2022

Uh oh!

sarina commented Jul 25, 2022

Uh oh!

farhaanbukhsh commented Jul 25, 2022

Uh oh!

openedx-webhooks commented Jul 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

farhaanbukhsh commented Apr 25, 2022 •

edited

Loading

openedx-webhooks commented Apr 25, 2022 •

edited

Loading

sarina commented Jun 1, 2022 •

edited

Loading

mariajgrimaldi Jun 23, 2022 •

edited

Loading