search

nens / lizard-catalogue

Lizard Catalogue
https://demo.lizard.net/catalogue/
0 stars 0 forks source link

Add options for validation fields in timeseries export #211

Closed lexvand closed 3 years ago

lexvand commented 4 years ago

Expand time series export functionality in time series export modal with functionality to optionally export the fields "flag", "validation_code", "comment", "last_modified" and "detection_limit"

Design: https://xd.adobe.com/view/42a1c54f-2241-4240-b933-fb5ea4aaa18a-17f2/ image

NOTE: The 'values' and 'timestamps' are required, so the checkboxes can't be unchecked

UPDATE: The new timeseries export endpoint already included all the export options by default. So this option modal is no longer necessary. For how to use the export endpoint, go to: https://nxt3.staging.lizard.net/api/v4/timeseries/export/

lexvand commented 3 years ago

Use /api/v4/timeseries/{uuid}/events/export/

hoanphungt commented 3 years ago

@lexvand In case of export of multiple timeseries at the same time then how can I add multiple uuid to the request?

hoanphungt commented 3 years ago

@lexvand @caspervdw Hi Lex and Casper,

I have 2 questions regarding the export options for timeseries:

Kr, Hoan

lexvand commented 3 years ago

@hoanphungt I've created a new backend issue to support this.

hoanphungt commented 3 years ago

@lexvand Hi Lex,

I encounter a new issue with this timeseries export. The new API v4 timeseries export endpoint: /api/v4/timeseries/export/ return a task instance in API v4. However, this task instance is not registered in the inbox endpoint (probably because the inbox endpoint is only available in API v3). So, although the export task is done successfully, the inbox does not show any task to download.

Furthermore, I guess the export options modal is not necessary anymore since all the fields will be included in the export file by default. Is it correct? If this is the case, then the implementation of the timeseries export will remain the same as before.

Example:

lexvand commented 3 years ago

@byrman Can the task result be made available in the inbox? Or is that something we decide to no longer do in API v4. That is in that case something to discuss.

@hoanphungt True, For Carsten it was easier to include the extra fields by default. And for the user it should make no difference. Do you need any input from Lirry concerning the workflow/design to make it work without the options?

byrman commented 3 years ago

Can the task result be made available in the inbox?

This is configurable: specify ?notify_user=True in the request. At least, that was the intention (it should be tested though.)

hoanphungt commented 3 years ago

@byrman @lexvand Tested and it worked. I can see the task in the inbox now.

hoanphungt commented 3 years ago

@Derryrover Hi Tom,

Could you review this PR: https://github.com/nens/lizard-catalogue/issues/245? To test the timeseries export, it's best to use the Large N&S sample or KNMI All on staging. I needed to add the export task to Redux as well since it is related to the inbox. The implementation is similar to the Raster export.

Kr, Hoan

Derryrover commented 3 years ago

@hoanphungt

Okey looks good to merge.

I found the whole mechanism of keeping track of all the tasks a bit difficult to understand, but it was mostly already in the code before this ticket.

As I understand it timeseries exports first become part of the TimeseriesExport store, But once they are received by the inbox store, the inbox component removes them from the timeseriesExport store. Is my understanding correct?

kr Tom

hoanphungt commented 3 years ago

@Derryrover Yes, you understand it correctly. This is because once the task reached the inbox, it means that the export task has been finished and we don't keep track of finished tasks.

The whole mechanism of keeping track of both the inbox and tasks and keeping them in sync with each other is quite shaky. We do have plan to eventually remove the inbox but use the task endpoint to show task progress. I think it will solve this issue. Anyway, for now it's still good enough.