search

foodcoopsat / foodsoft_hackathon

Other
1 stars 2 forks source link

Find a solution for broken name validation on article synchronization #57

Open lentschi opened 6 months ago

lentschi commented 6 months ago

Should we still support something like https://github.com/foodcoops/foodsoft/pull/287/files#diff-24503fd25ed68e6ebceee4951bc4f9b255b197278d4aa9d86ef9d5afe3f26beaR230? (I think this might be obsolete after migrating from sharedlists)

yksflip commented 3 months ago

I'm not quite sure what this is about. The ref. code is the articles :uniqueness_of_name validation. This one breaks now with the new supplier-share feature? Honestly I don't understand why there was this constrain in the first place, the comment on uniqueness_of_name irritates me even more. Was the uniqueness of the article names a constraint for the article synchronisation with a sharedlist server?! If so I'd agree that we don't need it anymore. Or could there be other reasons for this validation? Maybe we could ask willem ...

lentschi commented 3 months ago

@yksflip Yeah, it's tricky to decide what to do with this, because I too don't know what the reasoning about the sharedlists design decisions were.

I'm not quite sure what this is about.

Well, as far as I can tell, the referenced code is about this: When there's a sharedlists supplier with multiple articles, which have the same name, but different units (which is apparently something that was possible in sharedlists), the linked foodsoft supplier had to adapt validation to allow for that.

Since we try to integrate it all in foodsoft now, where it's generally not allowed to have the same article name twice in a supplier's list, we have to make a decision:

  1. Either stick with foodsoft's default behavior and do not allow for duplicate article names at all.
  2. Or always allow duplicate article names as long as the units differ.
  3. Or like before: Make validation dependent on whether the supplier has been synchronized remotely or not.

Number 3. I've always found kind of quirky (Why should validation depend on where the article has been imported from?) Number 1. would be simplest as it's how it's currently implemented in this fork. Number 2. would probably cover the most use cases: As @twothreenine has pointed out elsewhere, one could for example sell Chickpeas in either glasses or cans. The article name would then always be "Chickpeas" and only the unit would differ. However, since this would be yet another change in a fork already too big, I would vote for 1 (unless users of sharedlists protest). Foodsoft users are used to add this kind of unit information to the name, which is redundant but pragmatic.

If we were to choose 2., we'd have to define which units have to differ to make an article unique (There are so many fields now :wink: )

yksflip commented 3 months ago

for completeness, a probably not very attractive but simple way:

  1. remove this validation and allow duplicate article names

I think 1. would be the easiest and closest to the previous behavior and be in favor for that way.

lentschi commented 3 months ago
  1. remove this validation and allow duplicate article names

You mean like remove all validation preventing duplication? That would seem like a step back. :thinking:

I think 1. would be the easiest and closest to the previous behavior and be in favor for that way.

:+1: Okay, I'll still leave this issue open for discussion, but move it to Post-Merge.