Corgam / SS23_ADSP_TCF

An official repository for the "Tangible Climate Futures" project from the ADSP (SS23) course at TU Berlin.
1 stars 2 forks source link

Tcf 113 upload workflow #136

Closed flori950 closed 1 year ago

flori950 commented 1 year ago

Pls review asap. Merged tcf and main in my Branch and needed to fix a lot of errors.

thdrbrkw commented 1 year ago

I found following issues testing:

thdrbrkw commented 1 year ago

Also, you have created seperate components with redundant code. I am not sure how I feel about it. Maybe three components: no-data, files (json/csv/txt), and datasets (simra, netcdf)? @salbani what do you think?

flori950 commented 1 year ago

Are you sure? Because It's much easier for routing then because we route over components, and we cannot change the URL then. I would let it like that, otherwise we need to change the whole routing.

Corgam commented 1 year ago

I have deleted unnecessary doubled TS interfaces inside common folder. Also few notes:

thdrbrkw commented 1 year ago

Are you sure? Because It's much easier for routing then because we route over components, and we cannot change the URL then. I would let it like that, otherwise we need to change the whole routing.

Yes we could use seperate URLs: In the app.routes different routes link to the same component and this component retrieves the url-segments and therefore knows, if it is a CSV/ JSON/ etc. But I'm not sure if the rebuild is worth it.

  • When clicking no file, the user should be also allowed to upload not referenced datatypes (currently only referenced are available). Just check how it was done by Theo before the current upload changes.

Oh sorry, that was my instruction. I just assumed uploading pure text would not really happen (since we have the txt upload) and therefore said non referenced data will be fine.

Corgam commented 1 year ago

Looks good, two final notes: