noi-techpark / it.bz.opendatahub.databrowser

Explore and navigate through Open Data you need to build your next service.
https://databrowser.opendatahub.com
GNU Affero General Public License v3.0
9 stars 7 forks source link

Feat/file upload modal #431

Closed a-crea closed 1 year ago

a-crea commented 1 year ago

This pull request includes the new file upload language settings modal.

gappc commented 1 year ago

@a-crea Thank you for this PR

Unfortunately it doesn't satisfy all checks (see https://github.com/noi-techpark/it.bz.opendatahub.databrowser/actions/runs/6563367797/job/17827429525?pr=431) and can therefor not be merged.

I think the problems are related to missing REUSE license headers in new files. Please take a look at other existing files and see how such headers look like. Then, please add the header to all related files.

gappc commented 1 year ago

@a-crea I saw in another PR (#432) that the branch was not up-to-date with the current development branch (see https://github.com/noi-techpark/it.bz.opendatahub.databrowser/pull/432#issuecomment-1771279476). The same problem seems to appear in this PR.

Please bring your branch up-to-date with the current development branch doing a rebase (or merge if you prefer). It simplifies the code review very much

thx

gappc commented 1 year ago

@a-crea thank you for this PR

There still seems to be a bug, where I'm not able to add languages (see screenshot)

image

Steps to reproduce:

=> the popup appears, but there is no way to add languages

In my opinion, all languages should be shown such that a user can select languages that were not selected before


There seems to be another (related?) bug, that happens when more than one documents get uploaded. Ifone wants to change the language info without reloading the page, no language is shown at all in the popup

Steps to reproduce:

=> the popup appears, but there is no language shown at all

Please take a look at these issues, thx

MatteoBiasi commented 1 year ago

@gappc this has been separated in another issue: https://github.com/noi-techpark/it.bz.opendatahub.databrowser/issues/438. For this PR this behaviour it's fine.

gappc commented 1 year ago

@MatteoBiasi I had the impression that it should be possible to add languages and #438 was there to discuss if the language assignment should be modified (e.g. provide delete / add buttons). The main problem with this implementation is, that it is impossible to add languages after an upload was saved.

@sseppi if the behavior implemented in this PR is fine, we can merge it

sseppi commented 1 year ago

@MatteoBiasi I had the impression that it should be possible to add languages and #438 was there to discuss if the language assignment should be modified (e.g. provide delete / add buttons). The main problem with this implementation is, that it is impossible to add languages after an upload was saved.

@sseppi if the behavior implemented in this PR is fine, we can merge it

@gappc @MatteoBiasi I agree that the behaviour isn't perfet at the moment, but I need to go online as soon as possible with this new feature. So I would suggest to do compromise, go online with this PR and implement as soon as possibel the changes included in the issue #438

gappc commented 1 year ago

@sseppi @MatteoBiasi @a-crea merge is done :+1: