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

UIFR-219: Client time zone saved as a user property for UiUtils' availability. #66

Merged
merged 51 commits into from
Apr 12, 2021

Conversation

icrc-jfigueiredo
Copy link
Contributor

@icrc-jfigueiredo icrc-jfigueiredo commented Mar 23, 2021

Ticket:

https://issues.openmrs.org/browse/UIFR-219

What I have done:

UiUtils to expose utility methods to ... [TBC]

@mks-d mks-d changed the title Html 755-2 Using a User Propriety to save their timezone HTML-755: Client time zone saved as a user property for UiUtils' availability. Mar 25, 2021
Copy link
Member

@mks-d mks-d left a comment

Choose a reason for hiding this comment

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

I'm overall ok with it, except that we still need to nail the naming of this GP...

(@icrc-jfigueiredo also there's formatting issues, did you mvn package before pushing?)

JFFigueiredo and others added 11 commits March 25, 2021 01:45
…module-uiframework into HTML-755-2

� Conflicts:
�	api/src/main/java/org/openmrs/ui/framework/FormatterImpl.java
�	api/src/main/java/org/openmrs/ui/framework/UiFrameworkConstants.java
�	api/src/main/java/org/openmrs/ui/framework/UiUtils.java
�	api/src/main/java/org/openmrs/ui/framework/converter/StringToDateConverter.java
�	api/src/test/java/org/openmrs/ui/framework/FormatterImplTest.java
Add new GP handling timezones.
Add new functions on uiutils about timezone and date formats from UiFramework
Add new date parses to be able to:
    Parse from string with format ISO8601 to date
    Parse Date format to string with RFC3339 format
Change function to format date to use fromISO8601
@icrc-jfigueiredo icrc-jfigueiredo marked this pull request as ready for review April 7, 2021 08:09
@dkayiwa
Copy link
Member

dkayiwa commented Apr 7, 2021

@icrc-jfigueiredo it greatly helps the reviewer to separate formatting changes into their own pull request.

*/
public static String toTimezone(Date date, String format, String timezone) {
if (StringUtils.isEmpty(timezone)) {
timezone = "UTC";
Copy link
Member

Choose a reason for hiding this comment

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

Do you wanna define a constant for UTC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think is necessary? I only use it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I replaces that "UTC" by import static org.joda.time.DateTimeZone.UTC

@dkayiwa
Copy link
Member

dkayiwa commented Apr 9, 2021

@icrc-jfigueiredo do we have a UIFramework JIRA ticket for this pull request?

@icrc-jfigueiredo
Copy link
Contributor Author

I am using this one: https://issues.openmrs.org/browse/HTML-755 but i can create a new one, if you think its necessary.

@dkayiwa
Copy link
Member

dkayiwa commented Apr 9, 2021

It would be great to have them separate.

@icrc-jfigueiredo
Copy link
Contributor Author

So, one ticket for each PR ?

@dkayiwa
Copy link
Member

dkayiwa commented Apr 9, 2021

The htmlformentry module JIRA ticket for changes in the htmlformentry module. A UIFramework JIRA ticket for changes in the uiframework module. But if no changes were made in the htmlformentry module as part of this ticket, then can just move this ticket to the UIFramework JIRA project without creating a new one.

@mks-d mks-d changed the title HTML-755: Client time zone saved as a user property for UiUtils' availability. UIFR-219: Client time zone saved as a user property for UiUtils' availability. Apr 12, 2021
@dkayiwa dkayiwa merged commit d9f37fd into openmrs:master Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants