Natixar / natixar-frontend

The static front end of the Natixar SaaS platform
0 stars 5 forks source link

Front End Makes Bad Requests to Back-End: Unaligned Dates #40

Closed lepeuvedic-natixar closed 3 months ago

lepeuvedic-natixar commented 5 months ago

Problem: The back-end is specified to reject requests if the dates are not well aligned considering the associated time step. The time step can be specified as an integer value followed by a letter: m for minutes, h for hours, d for days, w for weeks, M for months, Q for quarters, y for years. The only currently supported integer value is 1. Omitting it is equivalent to specifying 1.

The front end currently always requests "scale=1m" and the alignment constraint means that the dates should always be YYYY-MM-01T00:00+00:00

Steps to Reproduce:

  1. Set the proxy in debug mode and capture all the requests (won't be necessary when the back-end actually starts rejecting such requests, since the default proxy mode captures all failures.
  2. Check "Month" is selected on both Total Emissions charts.
  3. Observe that any date range selected is passed verbatim without alignment as a request.
  4. Observe that hour, minutes, secondes and time zone are missing, when only seconds are optional (and must be 00 when present).

Desired Behavior: The goal is to make it impossible to generate a chart using partial data. If the database contain data up to May 3rd, 2024, it shall not be possible to display monthly data for the month of May 2024, because the absolute emissions would be small not because of any progress made, but because only partial data has been recorded for the month of May.

The data range selector should query the back-end to read the available date range in the database for the currently displayed scenario(s) (using the /scenario/{scenarioId} endpoint with the client's default scenario URI). In a scenario compare situation (currently not possible in the front-end), the smallest of all the compared scenarios date ranges should be the limit. While this comparison is not yet possible, the relevant function interface in the front-end should allow for lists of scenarios, not just one scenario, even if they are currently only used with a list containing only one scenario.

The user set dates MUST NOT change when the user clicks on time step selection buttons. This is especially important since the two "Total Emissions" charts can have different time steps.

An inappropriate situation can result from either a choice of start date which is too close to end date, a choice of end date that is too close to start date or a choice of time scale that is too large on any of the two charts. Accordingly, guards and input validation must be in place for all four inputs:

All the ISO8601 timestamps can be used in the /data/ranges request. The yyyyWww (week number) ISO8601 format is accepted to simplify week-aligned requests.

With the guards in place, the front-end shall properly align the start and end dates it puts on requests to the back-end. In order to simplify processing and to maintain compatibility with ISO8601 timestamp standard, a date without time is invalid in a request to the backend as specified in API Specification - Data Endpoint.

Relevant part of the specification:

Note: The precision in italics is not present in the actual specification, but the full ISO8601 W notation is supported.

lepeuvedic commented 4 months ago

The problem with unaligned start/end timestamps comes from here: https://github.com/Natixar/natixar-frontend/blob/d713c04a472807427aae8816cc2cd8afb1b80b3c/src/data/store/features/emissions/ranges/EmissionRangesSlice.ts#L257

const selectTimeRangeReducer: CaseReducer<
  EmissionRangeState,
  PayloadAction<TimeRange>
> = (state, action) => {
  state.dataRetrievalParameters.timeRangeOfInterest = action.payload
}

The selectProtocolReducer function must include logic to align on natural boundaries the state.dataRetrievalParameters.timeRangeOfInterest following the specification, based on the smallest currently chosen time step. Note that the backend supports requesting several time ranges at once, especially in order to avoid having to carry out date-aligned aggregation of data in the front end, but the state vector defined in EmissionRangesSlice.ts does not have the capability to store more than one set in allPoints[]. On top of that, the timeStepInSecondsPattern is stored in the definition of overallTimeWindow (the front end still lacks multiscale points storage capability), but the state of the time scale buttons on the two time-dependent charts is not stored, and the data is not refreshed from the back-end when these buttons are used. All this is somewhat problematic because it is not possible to select more than one-week of data at minute scale, and

https://github.com/Natixar/natixar-frontend/blob/d713c04a472807427aae8816cc2cd8afb1b80b3c/src/data/store/features/emissions/ranges/EmissionRangesSlice.ts#L49

In a nutshell the API specification says two things:

For example these rules imply that, if the start date (or the absolute beginning of recorded data) is a Wednesday, week-scale display can only occur starting on the next Monday. The front should enforce that requested data is always included in the date range set on the date range picker.

ChrisNatixar commented 3 months ago

Fixed with PR #73