OLMIS-4944 Add support for timezones in date filters

Activity

FEOLMIS-3734 32

Keyboard shortcuts  
  • Summarize the review outcomes (optional)
     
    #permalink

    Details

    Warning: no files are visible, they have all been filtered.
    Participant Role Time Spent Comments Latest Comment
    Author 1h 2m 18 Done.
    Reviewer - Complete 37m 14 Something like this: https://github.com/OpenLMIS/openlmis...
    Reviewer - 92% reviewed 4m    
    Total   1h 43m 32  
    #permalink

    Objectives

    • Review change in dev-ui to add moment and moment-timezone for build and test.
    • Review change in dev-ui's Dockerfile from debian jessie to stretch. Jessie seems to no longer have support.
    • Review UI components new locale service to get locale settings from reference data.
    • Review new date filters, which get date format and time zone settings from stored locale settings first, then default settings from startup config if those are not found.
    • Review new locale cache in reference data UI that saves locale settings to cache on login.
    • Review change in CCE UI that uses date filter.

    Branches in review

    #permalink

    Issues Raised From Comments

    Key Summary State Assignee
    #permalink

    General Comments

    Nikodem Graczewski

    I think we should also update the OpenlmiDatePicker directive.

    I think we should also update the OpenlmiDatePicker directive.

    Chongsun Ahn

    Update it how? It will already use dateFormat and timeZoneId from the locale ...

    Update it how? It will already use dateFormat and timeZoneId from the locale settings if they are set, and there is no locale setting for a datepicker format from reference data.

    Nikodem Graczewski

    So it will use different date format then?

    So it will use different date format then?

    Chongsun Ahn

    Basically the changes here are just to get the filters to use settings from r...

    Basically the changes here are just to get the filters to use settings from reference data, and then the defaults if those aren't found (and to support timezone in the filters). Since datepicker isn't a filter and it doesn't have a corresponding reference data setting, it doesn't seem to apply to the work here. It looks like datepicker format uses a different date format already, even before this ticket.

    Nikodem Graczewski

    I see. Isn't the purpose of this ticket to make all components use the locale...

    I see. Isn't the purpose of this ticket to make all components use the locales from the reference data?

    Chongsun Ahn

    Yes but the datepicker format is a separate setting, and not in the reference...

    Yes but the datepicker format is a separate setting, and not in the reference data. I suppose we could add it, but it wasn't done in the other tickets, so this ticket just followed what was done.

    Nikodem Graczewski

    I agree that it uses separate format, but what I'm concerned about is that th...

    I agree that it uses separate format, but what I'm concerned about is that the ticket might not pass the QA because of the second point of the acceptance criteria as the date picker will show different date format (most likely one without timezone).

    Chongsun Ahn

    It may be a non-issue, as it looks like anything that would use datepicker wo...

    It may be a non-issue, as it looks like anything that would use datepicker would probably use business dates, which does not represent an instant in time, and so timezone would not be important. Additionally, the datepicker directive does use the openlmis date filter.

    Chongsun Ahn

    The whole datepicker format is confusing to me as well. Does it need to be di...

    The whole datepicker format is confusing to me as well. Does it need to be different from the date format? And where is the documentation about its meaning, vs. the date formats?

    Chongsun Ahn

    If we want to do this work, let's put it in another ticket, so this ticket do...

    If we want to do this work, let's put it in another ticket, so this ticket doesn't drag on and not finish by the end of this sprint.

    Chongsun Ahn

    https://openlmis.atlassian.net/browse/OLMIS-6170

    https://openlmis.atlassian.net/browse/OLMIS-6170

    Nikodem Graczewski

    Great, thanks!

    Great, thanks!

    Chongsun Ahn

    Also changed the currency filter in ui-components to use the new localeService.

    Also changed the currency filter in ui-components to use the new localeService.

    /tasks/javascript.js Changed
    Open in IDE #permalink
    /tasks/karma.js Changed 1
    /Dockerfile Changed
    Open in IDE #permalink
    /src/cce-inventory-list/cce-inventory-list.html Changed
    Open in IDE #permalink
    /yarn.lock Changed
    Open in IDE #permalink
    /src/openlmis-locale-cache/openlmis-locale-cache.module.js Added
    Open in IDE #permalink
    /src/openlmis-locale-cache/openlmis-locale-cache.run.js Added
    Open in IDE #permalink
    /src/openlmis-locale-cache/openlmis-locale-cache.run.spec.js Added
    /yarn.lock Changed
    /src/openlmis-currency/openlmis-currency.filter.js Changed
    Open in IDE #permalink
    /src/openlmis-currency/openlmis-currency.filter.spec.js Changed
    /src/openlmis-currency/openlmis-currency.module.js Changed
    Open in IDE #permalink
    /src/openlmis-date/openlmis-date.filter.js Changed 7
    Open in IDE #permalink
    /src/openlmis-date/openlmis-date.filter.spec.js Changed
    /src/openlmis-date/openlmis-date.module.js Changed
    Open in IDE #permalink
    /src/openlmis-date/openlmis-datetime.filter.js Changed 2
    Open in IDE #permalink
    /src/openlmis-date/openlmis-datetime.filter.spec.js Changed
    /src/openlmis-date/period.filter.spec.js Changed 5
    /src/openlmis-locale/openlmis-locale.module.js Changed
    Open in IDE #permalink
    /src/openlmis-locale/openlmis-locale.service.js Added 4
    Open in IDE #permalink
    /src/openlmis-locale/openlmis-locale.service.spec.js Added
    /CHANGELOG.md Changed
    /package-yarn.json Changed
    /yarn.lock Changed

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time against