TerriaJS / terriajs

A library for building rich, web-based geospatial data platforms.
https://terria.io
Apache License 2.0
1.19k stars 364 forks source link

Default value for date field unwarranted and results in failing jobs #7200

Closed uwer closed 4 months ago

uwer commented 5 months ago

In the recent updates teh date-time fields assume the current time, for the WPS under oa-gis.csiro.au/wps these are optional and the total extent is assumed (on most often expected). Using the current time will have all requests fail as there is no data.

Proposal: if the property is optional don't assign a default value.

          [Added a change](https://github.com/TerriaJS/terriajs/pull/7194/commits/8ce55eef34e96e1eb08dea7af7006e674b3e71f5) to fix the other problem in the OG issue about requesting `status` when it is not supported.

Now the job finishes with a successful response, although i'm not sure if the response is meaningful.

image

Originally posted by @na9da in https://github.com/TerriaJS/terriajs/issues/7194#issuecomment-2174888344

uwer commented 5 months ago

I take this is the result of changes in 8.7.2 and 8.7.3, if there is a way of indicating that no default datetime value should be set - I could fix that at least for nationalmap.

nf-s commented 5 months ago

I think the issue here is that WPS dateTime process inputs should only be given a default value if they are required fields (minOccurs is not 0).

All the dateTime fields across the WPS processes published by oa-gis.csiro.au/wps are marked as optional, so we need to make a change in TerriaJS.

I don't mind giving required dateTime process inputs a default value of the current time in Terria - as the user would have to set the value anyway

na9da commented 4 months ago

Hi @uwer - when you get a chance could you please test this new branch? I have made the change to use default date only when the field is required, now the dates can be left empty.

uwer commented 4 months ago

Great, appears to work as expected!! Thank you!

On Wed, Jul 3, 2024 at 10:08 AM Nanda @.***> wrote:

Hi @uwer https://github.com/uwer - when you get a chance could you please test this new branch http://ci.terria.io/refactor-wps-datetime/#share=s-5bFqE9KlldTl3h1uQK7eC5bxZ64? I have made the change to use default date only when the field is required, now the dates can be left empty.

— Reply to this email directly, view it on GitHub https://github.com/TerriaJS/terriajs/issues/7200#issuecomment-2204748694, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG5MPSUZYWVVD3JLLRDOM3ZKM6GTAVCNFSM6AAAAABJRJE3TGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBUG42DQNRZGQ . You are receiving this because you were mentioned.Message ID: @.***>

ljowen commented 4 months ago

Update 3 Jul 24: PR assigned