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

Wps default date #7026

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Wps default date #7026

wants to merge 24 commits into from

Conversation

sixlighthouses
Copy link
Contributor

@sixlighthouses sixlighthouses commented Jan 10, 2024

What this PR does

Fixes #6896 & #6895

Refactored the DateTimeParameterEditor to a functional component and added the terria object to the props, this allows the component to load with a default dateTime -- either the current time or if a timeline is loaded it takes the currently loaded time from that

Test me

Browse to
http://ci.terria.io/wps-default-date/#share=s-bRTdoGD1E1hSA95nLCVwIEjNXa9

From the My Data tab select the AWAVEA WPS service and then the Wave: Annual Bivariate Probability Distribution service

Select a point location, manually edit the start and end times. Update the times manually several times to test that the other unedited values do not change.

image

Ensure the debugging console is open on the Network tab, hit Run Analysis

confirm the Payload of the request has the Date Time correctly formatted as YYYY-MM-DDThh:mm

image

Upon loading the Analysis Tools, if the current Terria session has timeline loaded that time in the will become the default time in the dateTime inputs of the WPS process

image

Checklist

  • There are unit tests to verify my changes are correct
  • I've updated CHANGES.md with what I changed.
  • I've provided instructions in the PR description on how to test this PR.

@sixlighthouses sixlighthouses marked this pull request as ready for review January 11, 2024 01:36
@nf-s nf-s removed their assignment Jan 18, 2024
@sppigot
Copy link
Contributor

sppigot commented Mar 8, 2024

@sixlighthouses - Thanks for your work on this!

Testing with no timeline:

  • Default date/time does get set to current date/time in wps parameter form
  • Manually changing dates or times works correctly

However, when if I set dates/times and then click on the map to get a location, the dates and times seem to be reset to the current date/time when the form reappears. Changing them back to what I had before seems to work but when I submit the form the start date/time value is messed up in the request that goes to the WPS:

e.g.
messedupdatetime

Testing with timeline shortly.

@sppigot
Copy link
Contributor

sppigot commented Mar 8, 2024

@sixlighthouses - testing with a timeline - the default date does get set to the currently selected value in the timeline but seems to be off by 1 day for some reason? (Am I missing something obvious here?). Check the following image:

offby1day

Thanks for your work on this @sixlighthouses !

@sixlighthouses
Copy link
Contributor Author

@sppigot thanks for test and the feedback. I have made some updates so when you get a chance please review again

Copy link
Collaborator

@na9da na9da left a comment

Choose a reason for hiding this comment

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

@sixlighthouses I think these changes look great. Thanks for taking time to refactor the component as tsx.

I have just left a couple of comments.

CHANGES.md Outdated
Comment on lines 61 to 62
- Fix WPS date parameter reset bug
- [The next improvement]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these need to go.

Copy link
Collaborator

@na9da na9da left a comment

Choose a reason for hiding this comment

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

Looks good!

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.

WPS: Date or time widget reset after typing in values is unexpected (chrome)
4 participants