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

fix(period-select): respect system settings for date formats when rendering Daily periods #89

Merged
merged 7 commits into from
Sep 28, 2021

Conversation

mediremi
Copy link
Contributor

return moment(period.displayName, 'YYYY-MM-DD').format(
// moment format tokens are case sensitive
// see https://momentjs.com/docs/#/parsing/string-format/
dateFormat === 'yyyy-mm-dd' ? 'YYYY-MM-DD' : 'DD-MM-YYYY'
Copy link
Contributor Author

@mediremi mediremi Sep 27, 2021

Choose a reason for hiding this comment

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

This assumes that there will only ever be two date formats, which might be brittle in the future.

Should this be extracted to a function?

Otherwise could always just do dateFormat.toUpperCase()

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think your last suggestion would make most sense, i.e. to call toUpperCase.

Zooming out a little bit, I actually expected this functionality to be added to the fixed-periods module. That would involve a bit more work though:

  • getFixedPeriodTypes would need to accept formatYyyyMmDd (and I guess filterFuturePeriods) too.
  • Every function that calls getFixedPeriodTypes internally (i.e. getFixedPeriodsByTypeAndYear) would also need to accept these args.
  • Then you would pass the formatter to getFixedPeriodsByTypeAndYear below

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Sep 27, 2021

🚀 Deployed on https://pr-89--dhis2-data-approval.netlify.app

@mediremi mediremi marked this pull request as draft September 28, 2021 07:56
@mediremi mediremi changed the title fix: respect system settings for date formats when rendering Daily periods fix(period-select): respect system settings for date formats when rendering Daily periods Sep 28, 2021
Copy link
Contributor

@HendrikThePendric HendrikThePendric left a comment

Choose a reason for hiding this comment

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

I would say the suggestion I've made regarding useConfig is a required change (unless you manage to change my mind ;-) )

The other requested changes are more of a suggestion, even though it does seem like a good idea to me.

src/app-context/app-provider.js Outdated Show resolved Hide resolved
src/app-context/app-provider.js Outdated Show resolved Hide resolved
src/app-context/app-provider.js Outdated Show resolved Hide resolved
src/top-bar/period-select/period-menu.js Outdated Show resolved Hide resolved
return moment(period.displayName, 'YYYY-MM-DD').format(
// moment format tokens are case sensitive
// see https://momentjs.com/docs/#/parsing/string-format/
dateFormat === 'yyyy-mm-dd' ? 'YYYY-MM-DD' : 'DD-MM-YYYY'
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think your last suggestion would make most sense, i.e. to call toUpperCase.

Zooming out a little bit, I actually expected this functionality to be added to the fixed-periods module. That would involve a bit more work though:

  • getFixedPeriodTypes would need to accept formatYyyyMmDd (and I guess filterFuturePeriods) too.
  • Every function that calls getFixedPeriodTypes internally (i.e. getFixedPeriodsByTypeAndYear) would also need to accept these args.
  • Then you would pass the formatter to getFixedPeriodsByTypeAndYear below

src/top-bar/period-select/period-menu.js Outdated Show resolved Hide resolved
@mediremi mediremi marked this pull request as ready for review September 28, 2021 11:54
@HendrikThePendric HendrikThePendric merged commit b2f36dc into master Sep 28, 2021
@HendrikThePendric HendrikThePendric deleted the DHIS2-11880 branch September 28, 2021 12:26
dhis2-bot added a commit that referenced this pull request Sep 28, 2021
# [1.14.0](v1.13.1...v1.14.0) (2021-09-28)

### Bug Fixes

* **approval-status-tag:** fix time-ago messages ([#87](#87)) ([adb0d1d](adb0d1d))
* **approval-status-tag:** unset max width of Tag component ([#67](#67)) ([6235aa4](6235aa4))
* **bottom-bar:** update button state before unmounting via refresh ([#66](#66)) ([854d3dc](854d3dc))
* **clear all selections button:** make button "small" (DHIS2-11674) ([2ab2929](2ab2929))
* **context select:** remove top/bottom padding ([009aa2a](009aa2a))
* **data set count label:** adjust font-size / line-height (DHIS2-11680) ([819b93c](819b93c))
* **data set display table:** table should use only needed space (DHIS2-11678) ([68b7048](68b7048))
* **period-select:** display periods in reverse chronological order ([#88](#88)) ([9d8982f](9d8982f))
* **period-select:** respect system settings for date formats when rendering Daily periods ([#89](#89)) ([b2f36dc](b2f36dc))
* **status-tag:** adjust date/time for server-client timezone offset ([#74](#74)) ([ef1cafd](ef1cafd))

### Features

* **data-workspace:** show notification for non-default form types ([#65](#65)) ([a50ba16](a50ba16))
* **status-tag:** show user and date/time if approved ([#62](#62)) ([9f76cf0](9f76cf0))
dhis2-bot added a commit that referenced this pull request Sep 28, 2021
# [1.14.0](v1.13.1...v1.14.0) (2021-09-28)

### Bug Fixes

* **approval-status-tag:** fix time-ago messages ([#87](#87)) ([adb0d1d](adb0d1d))
* **approval-status-tag:** unset max width of Tag component ([#67](#67)) ([6235aa4](6235aa4))
* **bottom-bar:** update button state before unmounting via refresh ([#66](#66)) ([854d3dc](854d3dc))
* **clear all selections button:** make button "small" (DHIS2-11674) ([2ab2929](2ab2929))
* **context select:** remove top/bottom padding ([009aa2a](009aa2a))
* **data set count label:** adjust font-size / line-height (DHIS2-11680) ([819b93c](819b93c))
* **data set display table:** table should use only needed space (DHIS2-11678) ([68b7048](68b7048))
* **noop:** trigger release process ([3b7dece](3b7dece))
* **period-select:** display periods in reverse chronological order ([#88](#88)) ([9d8982f](9d8982f))
* **period-select:** respect system settings for date formats when rendering Daily periods ([#89](#89)) ([b2f36dc](b2f36dc))
* **status-tag:** adjust date/time for server-client timezone offset ([#74](#74)) ([ef1cafd](ef1cafd))

### Features

* **data-workspace:** show notification for non-default form types ([#65](#65)) ([a50ba16](a50ba16))
* **status-tag:** show user and date/time if approved ([#62](#62)) ([9f76cf0](9f76cf0))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 1.14.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

HendrikThePendric added a commit that referenced this pull request Oct 13, 2021
…#117)

* fix(approval-status-tag): fix time-ago messages (#87)

* fix(approval-status-tag): only process approvedAt if received

* fix(approval-status-tag): fix rounding in timezone offset calculation

* chore(release): cut 1.14.0 [skip ci]

# [1.14.0](v1.13.1...v1.14.0) (2021-09-27)

### Bug Fixes

* **approval-status-tag:** fix time-ago messages ([#87](#87)) ([adb0d1d](adb0d1d))
* **approval-status-tag:** unset max width of Tag component ([#67](#67)) ([6235aa4](6235aa4))
* **bottom-bar:** update button state before unmounting via refresh ([#66](#66)) ([854d3dc](854d3dc))
* **clear all selections button:** make button "small" (DHIS2-11674) ([2ab2929](2ab2929))
* **context select:** remove top/bottom padding ([009aa2a](009aa2a))
* **data set count label:** adjust font-size / line-height (DHIS2-11680) ([819b93c](819b93c))
* **data set display table:** table should use only needed space (DHIS2-11678) ([68b7048](68b7048))
* **status-tag:** adjust date/time for server-client timezone offset ([#74](#74)) ([ef1cafd](ef1cafd))

### Features

* **data-workspace:** show notification for non-default form types ([#65](#65)) ([a50ba16](a50ba16))
* **status-tag:** show user and date/time if approved ([#62](#62)) ([9f76cf0](9f76cf0))

* chore(deps): update app-runtime and related deps

* fix(period-select): display periods in reverse chronological order (#88)

* chore(release): cut 1.14.0 [skip ci]

# [1.14.0](v1.13.1...v1.14.0) (2021-09-27)

### Bug Fixes

* **approval-status-tag:** fix time-ago messages ([#87](#87)) ([adb0d1d](adb0d1d))
* **approval-status-tag:** unset max width of Tag component ([#67](#67)) ([6235aa4](6235aa4))
* **bottom-bar:** update button state before unmounting via refresh ([#66](#66)) ([854d3dc](854d3dc))
* **clear all selections button:** make button "small" (DHIS2-11674) ([2ab2929](2ab2929))
* **context select:** remove top/bottom padding ([009aa2a](009aa2a))
* **data set count label:** adjust font-size / line-height (DHIS2-11680) ([819b93c](819b93c))
* **data set display table:** table should use only needed space (DHIS2-11678) ([68b7048](68b7048))
* **period-select:** display periods in reverse chronological order ([#88](#88)) ([9d8982f](9d8982f))
* **status-tag:** adjust date/time for server-client timezone offset ([#74](#74)) ([ef1cafd](ef1cafd))

### Features

* **data-workspace:** show notification for non-default form types ([#65](#65)) ([a50ba16](a50ba16))
* **status-tag:** show user and date/time if approved ([#62](#62)) ([9f76cf0](9f76cf0))

* test(cypress): fix cypress test failures

* chore(release): cut 1.14.0 [skip ci]

# [1.14.0](v1.13.1...v1.14.0) (2021-09-28)

### Bug Fixes

* **approval-status-tag:** fix time-ago messages ([#87](#87)) ([adb0d1d](adb0d1d))
* **approval-status-tag:** unset max width of Tag component ([#67](#67)) ([6235aa4](6235aa4))
* **bottom-bar:** update button state before unmounting via refresh ([#66](#66)) ([854d3dc](854d3dc))
* **clear all selections button:** make button "small" (DHIS2-11674) ([2ab2929](2ab2929))
* **context select:** remove top/bottom padding ([009aa2a](009aa2a))
* **data set count label:** adjust font-size / line-height (DHIS2-11680) ([819b93c](819b93c))
* **data set display table:** table should use only needed space (DHIS2-11678) ([68b7048](68b7048))
* **period-select:** display periods in reverse chronological order ([#88](#88)) ([9d8982f](9d8982f))
* **status-tag:** adjust date/time for server-client timezone offset ([#74](#74)) ([ef1cafd](ef1cafd))

### Features

* **data-workspace:** show notification for non-default form types ([#65](#65)) ([a50ba16](a50ba16))
* **status-tag:** show user and date/time if approved ([#62](#62)) ([9f76cf0](9f76cf0))

* chore(deps): update cypress

* ci(verify): use new release action

* chore(release): cut 1.14.0 [skip ci]

# [1.14.0](v1.13.1...v1.14.0) (2021-09-28)

### Bug Fixes

* **approval-status-tag:** fix time-ago messages ([#87](#87)) ([adb0d1d](adb0d1d))
* **approval-status-tag:** unset max width of Tag component ([#67](#67)) ([6235aa4](6235aa4))
* **bottom-bar:** update button state before unmounting via refresh ([#66](#66)) ([854d3dc](854d3dc))
* **clear all selections button:** make button "small" (DHIS2-11674) ([2ab2929](2ab2929))
* **context select:** remove top/bottom padding ([009aa2a](009aa2a))
* **data set count label:** adjust font-size / line-height (DHIS2-11680) ([819b93c](819b93c))
* **data set display table:** table should use only needed space (DHIS2-11678) ([68b7048](68b7048))
* **period-select:** display periods in reverse chronological order ([#88](#88)) ([9d8982f](9d8982f))
* **status-tag:** adjust date/time for server-client timezone offset ([#74](#74)) ([ef1cafd](ef1cafd))

### Features

* **data-workspace:** show notification for non-default form types ([#65](#65)) ([a50ba16](a50ba16))
* **status-tag:** show user and date/time if approved ([#62](#62)) ([9f76cf0](9f76cf0))

* chore(release): cut 1.14.0 [skip ci]

# [1.14.0](v1.13.1...v1.14.0) (2021-09-28)

### Bug Fixes

* **approval-status-tag:** fix time-ago messages ([#87](#87)) ([adb0d1d](adb0d1d))
* **approval-status-tag:** unset max width of Tag component ([#67](#67)) ([6235aa4](6235aa4))
* **bottom-bar:** update button state before unmounting via refresh ([#66](#66)) ([854d3dc](854d3dc))
* **clear all selections button:** make button "small" (DHIS2-11674) ([2ab2929](2ab2929))
* **context select:** remove top/bottom padding ([009aa2a](009aa2a))
* **data set count label:** adjust font-size / line-height (DHIS2-11680) ([819b93c](819b93c))
* **data set display table:** table should use only needed space (DHIS2-11678) ([68b7048](68b7048))
* **period-select:** display periods in reverse chronological order ([#88](#88)) ([9d8982f](9d8982f))
* **status-tag:** adjust date/time for server-client timezone offset ([#74](#74)) ([ef1cafd](ef1cafd))

### Features

* **data-workspace:** show notification for non-default form types ([#65](#65)) ([a50ba16](a50ba16))
* **status-tag:** show user and date/time if approved ([#62](#62)) ([9f76cf0](9f76cf0))

* fix(period-select): respect system settings for date formats when rendering Daily periods (#89)

* fix(period-select): respect system settings for date formats when rendering Daily periods

* test: update cypress fixtures

* test(period-select): test formatting of Daily periods

* refactor: use useConfig instead of fetching from system/info endpoint

* revert: "test: update cypress fixtures"

This reverts commit 4805618.

* refactor: expose formatYyyyMmDd option in getFixedPeriodsByTypeAndYear

* chore(release): cut 1.14.0 [skip ci]

# [1.14.0](v1.13.1...v1.14.0) (2021-09-28)

### Bug Fixes

* **approval-status-tag:** fix time-ago messages ([#87](#87)) ([adb0d1d](adb0d1d))
* **approval-status-tag:** unset max width of Tag component ([#67](#67)) ([6235aa4](6235aa4))
* **bottom-bar:** update button state before unmounting via refresh ([#66](#66)) ([854d3dc](854d3dc))
* **clear all selections button:** make button "small" (DHIS2-11674) ([2ab2929](2ab2929))
* **context select:** remove top/bottom padding ([009aa2a](009aa2a))
* **data set count label:** adjust font-size / line-height (DHIS2-11680) ([819b93c](819b93c))
* **data set display table:** table should use only needed space (DHIS2-11678) ([68b7048](68b7048))
* **period-select:** display periods in reverse chronological order ([#88](#88)) ([9d8982f](9d8982f))
* **period-select:** respect system settings for date formats when rendering Daily periods ([#89](#89)) ([b2f36dc](b2f36dc))
* **status-tag:** adjust date/time for server-client timezone offset ([#74](#74)) ([ef1cafd](ef1cafd))

### Features

* **data-workspace:** show notification for non-default form types ([#65](#65)) ([a50ba16](a50ba16))
* **status-tag:** show user and date/time if approved ([#62](#62)) ([9f76cf0](9f76cf0))

* fix(noop): trigger release process

* chore(release): cut 1.14.0 [skip ci]

# [1.14.0](v1.13.1...v1.14.0) (2021-09-28)

### Bug Fixes

* **approval-status-tag:** fix time-ago messages ([#87](#87)) ([adb0d1d](adb0d1d))
* **approval-status-tag:** unset max width of Tag component ([#67](#67)) ([6235aa4](6235aa4))
* **bottom-bar:** update button state before unmounting via refresh ([#66](#66)) ([854d3dc](854d3dc))
* **clear all selections button:** make button "small" (DHIS2-11674) ([2ab2929](2ab2929))
* **context select:** remove top/bottom padding ([009aa2a](009aa2a))
* **data set count label:** adjust font-size / line-height (DHIS2-11680) ([819b93c](819b93c))
* **data set display table:** table should use only needed space (DHIS2-11678) ([68b7048](68b7048))
* **noop:** trigger release process ([3b7dece](3b7dece))
* **period-select:** display periods in reverse chronological order ([#88](#88)) ([9d8982f](9d8982f))
* **period-select:** respect system settings for date formats when rendering Daily periods ([#89](#89)) ([b2f36dc](b2f36dc))
* **status-tag:** adjust date/time for server-client timezone offset ([#74](#74)) ([ef1cafd](ef1cafd))

### Features

* **data-workspace:** show notification for non-default form types ([#65](#65)) ([a50ba16](a50ba16))
* **status-tag:** show user and date/time if approved ([#62](#62)) ([9f76cf0](9f76cf0))

* feat(workflow-provider): add retry button (#95)

* chore(release): cut 1.15.0 [skip ci]

# [1.15.0](v1.14.0...v1.15.0) (2021-09-28)

### Features

* **workflow-provider:** add retry button ([#95](#95)) ([39d53b3](39d53b3))

* chore: setup e2e tests for v37 and v38 (#97)

* chore: setup v37 test jobs and generate fixtures

* chore: remove nr from workflow step name

* chore: use underscore and number in workflow step name

* chore: try no separator

* chore: add config and fixtures

* chore: tweak config

* chore: let release depend on v37 e2e step

* feat(data set): persist selectetd data set in query params

* test(data set): cover data set persist

* chore(release): cut 1.16.0 [skip ci]

# [1.16.0](v1.15.0...v1.16.0) (2021-09-29)

### Features

* **data set:** persist selectetd data set in query params ([5ee1054](5ee1054))

* fix(bottom-bar): disable approve button when it is allowed but pointless (#100)

* refactor: store approval statuses in constant

* fix(bottom-bar): disable button when approving is allowed but pointless

* chore: fix typo in property name

* test(bottom-bar): adjust test so it can assert disabled buttons too

* chore(release): cut 1.16.1 [skip ci]

## [1.16.1](v1.16.0...v1.16.1) (2021-09-30)

### Bug Fixes

* **bottom-bar:** disable approve button when it is allowed but pointless ([#100](#100)) ([0a92315](0a92315))

* chore(jest tests): remove ".only"

* fix(data set table): make table use min-required width

* fix(data set table): ensure 480px max-width rule works

* refactor: fix linter issues (DHIS2-11840)

* chore(release): cut 1.16.2 [skip ci]

## [1.16.2](v1.16.1...v1.16.2) (2021-09-30)

### Bug Fixes

* **data set table:** ensure 480px max-width rule works ([6377e6a](6377e6a))
* **data set table:** make table use min-required width ([62432f9](62432f9))

* fix: show message to users with no authority to approve data (#102)

* fix: show message to users with no authority to approve data

* chore: remove unused import

* chore(release): cut 1.16.3 [skip ci]

## [1.16.3](v1.16.2...v1.16.3) (2021-09-30)

### Bug Fixes

* show message to users with no authority to approve data ([#102](#102)) ([b4fae37](b4fae37))

* fix: revert pr#102 because the bug was actually expected behavior (#104)

* chore: revert "show message to users with no authority to approve data"

This reverts commit 1462f6d.

* chore: revert "remove unused import"

This reverts commit ad0ffac.

* chore(release): cut 1.16.4 [skip ci]

## [1.16.4](v1.16.3...v1.16.4) (2021-09-30)

### Bug Fixes

* revert pr[#102](#102) because the bug was actually expected behavior ([#104](#104)) ([6709bc2](6709bc2))

* fix(approval-status-tag): show correct status texts and icons (#105)

* fix(approval-status-tag): show correct status texts and icons

* test(approval-status-tag): adjust test to new implementation

* test(approval-status-tag): adjust cypress tests to new implementation

* chore: update network fixtures

* fix(approval-satus-tag): adjust test to simplified implementation

* chore(release): cut 1.16.5 [skip ci]

## [1.16.5](v1.16.4...v1.16.5) (2021-10-04)

### Bug Fixes

* **approval-status-tag:** show correct status texts and icons ([#105](#105)) ([ba2f9ae](ba2f9ae))

* fix(approval-status-tag): show custom tag text for unauthorized users (#106)

* fix(auth): distinguish between hasAppAccess and hasApprovalAuthorities

* fix(approval-status-tag): show custom tag text for unauthorized users

* chore(release): cut 1.16.6 [skip ci]

## [1.16.6](v1.16.5...v1.16.6) (2021-10-04)

### Bug Fixes

* **approval-status-tag:** show custom tag text for unauthorized users ([#106](#106)) ([bb2a483](bb2a483))

* test(approval-status-tag): add test for new approval state (#107)

* test(approval-status-tag): add test for new

* fix(approval-status-tag): adjust failing test

* fix(data-workspace): remove warning when showing non-default forms (#113)

* fix(approval-status-tag): don't show time-ago for approved-above (#114)

* chore(release): cut 1.16.7 [skip ci]

## [1.16.7](v1.16.6...v1.16.7) (2021-10-07)

### Bug Fixes

* **approval-status-tag:** don't show time-ago for approved-above ([#114](#114)) ([e8e4d67](e8e4d67))
* **data-workspace:** remove warning when showing non-default forms ([#113](#113)) ([bcceb4b](bcceb4b))

* fix(data-workspace): reduce header height (#108)

* fix(data-workspace): reduce header height

* chore: upgrade @dhis2/ui so it contains table header fixes

* fix: update broken imports

* fix: adress failing tests due to new layering mechanism

* chore: dedupe and refresh yarn lock

* fix(data-workspace): clean up CSS for overflowing table header text

* chore(release): cut 1.16.8 [skip ci]

## [1.16.8](v1.16.7...v1.16.8) (2021-10-12)

### Bug Fixes

* **data-workspace:** reduce header height ([#108](#108)) ([eb62886](eb62886))

Co-authored-by: @dhis2-bot <apps@dhis2.org>
Co-authored-by: ismay <ismay@ismaywolff.nl>
Co-authored-by: Viktor Varland <viktor@dhis2.org>
Co-authored-by: Médi-Rémi Hashim <4295266+mediremi@users.noreply.github.com>
Co-authored-by: Jan-Gerke Salomon <jgs.salomon@gmail.com>
Co-authored-by: Jan-Gerke Salomon <Mohammer5@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants