search

hydroserver2 / hydroserver

Documentation and issue tracker for HydroServer and related applications
BSD 3-Clause "New" or "Revised" License
5 stars 0 forks source link

Time aggregation unit improvement #182

Closed daniel-slaugh closed 3 months ago

daniel-slaugh commented 4 months ago

On the datastream creation form, it's confusing how the intended time spacing units dropdown is presented as a choice of seconds, minutes, hours, days, while the time aggregation units are shown as the list of template time units and any units the user has created where the type is 'time'. Currently, the backend will reject requests where the time aggregation unit doesn't belong to the primary user of the datastream, so anyone trying to create a datastream will need to first create a copy of a time unit then select the unit they created. This is a problem because there's a very good chance their names will overlap with the unit templates.

I think the long term solution is to update timeAggregationUnitId to be like intendedTimeSpacingUnit where it's just a list of strings the user can select.

@horsburgh, Ken and I want to check with you first to make sure this is OK, but in the meantime, I'm going to update the frontend to only show the user's time units in the time aggregation dropdown on the datastream form so Braedon can get started on loading his datastreams.

horsburgh commented 4 months ago

Since we've already settled on this for one, it doesn't make sense for them to be inconsistent. I am OK with your solution.

kjlippold commented 4 months ago

We currently have 'days', 'hours', 'minutes', and 'seconds'. I might capitalize each of these strings so they look cleaner on the frontend. I can also add 'Milliseconds', 'Months', and 'Years' if we want to have some lesser used units covered.

daniel-slaugh commented 4 months ago

@kjlippold I can handle the capitalization on the frontend. I think it makes sense to keep the variables lower case on the backend to stay consistent with naming conventions.

My vote for the lesser used units is to keep what we have in order to stay simple and add more only if we find a convincing reason to. I can't think of a use case where someone would want their datastream to aggregate on the millisecond or month scale. And if they did want to do that, they could just set the time aggregation interval for multiple days or fractions of seconds.

kjlippold commented 4 months ago

@daniel-slaugh Ok, if you want to capitalize them on the frontend I can leave them lowercase on the backend. I'll leave the existing units for now, but I can add more if the need ever comes up.