Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add several values for the same property #168

Open
CharlesNepote opened this issue Mar 22, 2024 · 3 comments
Open

Add several values for the same property #168

CharlesNepote opened this issue Mar 22, 2024 · 3 comments
Labels
⭐ top issue Top issue.

Comments

@CharlesNepote
Copy link
Member

CharlesNepote commented Mar 22, 2024

Reported by Alizarine on Slack (2024-03-24).

image

Indeed, the example provides a simple use case: a product could have multiple producer_data_issue.

Option 1: do not change the API, manage it with the UI

With the UI it would be possible to enter a property several times.
In the database, the different values would be stored in a unique property/value, separated by a given sign (a ; for example).
The UI would have to manage the following cases:

  • allowing to enter a property more than one time, and saving the value along with the existing values (it should not save a value if it is already existing)
  • taking care of multiple values in the list of all the possible values for a given property: eg. https://world.openfoodfacts.org/property/producer_data_issue
  • taking care of multiple values in the search as you type feature: when entering a value, the UI is listing all possible values depending on the property

Option 2: change the API, do not change the UI

There would be probably no change at all in the UI.

@CharlesNepote CharlesNepote added the ✨ enhancement New feature or request label Mar 22, 2024
@alexgarel
Copy link
Member

@cquest do you have an opinion on that (or can you share OSM experience on that ?)

@alexgarel
Copy link
Member

On my side, I think option 1 is best for now.

Option 2 would:

  • either require a huge rethinking of the code, the API and the database… so I don't see it as a good option.
  • either require an upper layer of abstraction that encode the multiple value into one value, but would require you to reprovide all values, etc. which, I fear, would feel alien to current API.

For option 1, it should be well documented what the UI does. I think going for "," as a separator is better than ";" because it's more standard (for example it's permitted to represent parameters as list in OpenAPI).

Then you have either to deal with values containing "," or refuse them. If you deal with them, I strongly encourage using csv style (handling quote) as there are a lot of libs to deal with it and avoid silly bugs.

In my opinion, the UI should clearly handle lists (one input box per value) instead of trying to magically guess, which have the risk to lead to complicated cases (explicit is better than implicit).

Also don't try to handle more than lists, as dict, for example must be encoded in property name. (one could even argue that list could be encoded as my::value::1="value 1", my::value::2="value 2", etc., but it might be overcomplicated !)

We could even argue that list could be handled by property name: `

@alexgarel
Copy link
Member

BTW, this also relates to #151

@github-actions github-actions bot added the ⭐ top issue Top issue. label Sep 16, 2024
@teolemon teolemon removed the ✨ enhancement New feature or request label Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⭐ top issue Top issue.
Projects
Status: No status
Development

No branches or pull requests

3 participants