Solves #422 Improve unit handling
Solves #474 by allowing price input per price unit (I think the rounding discrepancy should be in a tolerable range now)
Solves #827 Minimum order quantity for certain articles
Solves #843 Allow to insert an integer / number in orders to set amount of a certain product
Solves #850: Full article versioning (still to implement: apply new version specifically to future/current/closed/settled orders, and/or specific order)
Fixes #1056 Critical bug: Article price changes when closing order
Makes #710 obsolete (unit_quantity == 0 causes 500 when viewing group_order)

We've decided against supporting ordering partial cases, so #645 won't be solved, however if you choose a scalar unit, you can order loose quantities like 2.5 kg.

Sharedlist issues which will probably be affected:
#675 feature: (Un)check articles for article sync
#1030 Synchronize from external database fails with "out of range" error
#1033 Access control for shared suppliers

Note: Implementation of #569 (Work with datafoodconsortium) could be done based on this PR.

to-do: Check issue numbers below 400

@lentschi I ran into an issue with the migration 20240726083740_alter_articles_add_versioning step.
The part with
article_versions[0..-2].each do |obsolete_version| update("UPDATE order_articles SET article_version_id = #{quote latest_version['id']} WHERE article_version_id = #{quote obsolete_version['id']}")
Seems to replace the incorrect article_version_id which leads to duplicates and "disappearing" entries.
This halts the migration on the add_index :order_articles, %i[order_id article_version_id], unique: true step.

I tried this on actual current 'live' data used in v4.9.1 (I can therefore not share it here but I have logging of the migration).

Example: an entry with article_price_id = 718 exists and one with article_price_id=543
After migration this column is changed in article_version_id but there are now two entries with 718. The corresponding order_article.id for the 543 record has become a second 718 entry and 543 has therefore disappeared as original.
The pre-migration article_prices with id 543 has article_id=0. As does the 718 entry. It looks like these records are therefore unusable and could be removed and the corresponding order_article entry with this id could also be removed, thus preventing turning into an, I assume, unintended duplicate?
Was this also the intention?
The special case here is that both belong to the same order_id. But I am unclear why the 543 entry doesn't keep the reference (with a article_id=0) as others do?

I checked a few other with this condition, where the id has been changed, but they don't seem to show this result, I didn't check all of them, though. Either way: those records seem useless as they refer to non existing articles?

Code

I consider a complete code review to be impossible. With such a large scope, the changes would have to be divided into several functionally complete and consecutive commits that can be reviewed one after the other. Subsequent decomposition would take too much time.

Synchronisation (of article lists)

• There is now a ‘Synchronisation source’ field in the settings of a supplier. A hint is needed as to what form of input is expected there (a URL) and what common URLs to be used in this field look like. Is it only intended for the Foodsoft internal share links?
• The use case for the share link feature is unclear to me: Is it about one foodcoop maintaining the items of one supplier, another relying on it and synchronising with the item list of the first? Are there such cases? As far as I understand Sharedlists, it is mainly about synchronising article lists without an intermediate step using the wholesaler's lists (in the sense of a merchandise management system).
• In the installation I am familiar with, Sharedlists mainly uses mail import (e.g. Antakya, Odin) and FTP import in BNN format (e.g. Terra and possibly others such as Kornkraft/ Naturkost Nord). The Terra BNN, XLS or CSV files, for example, are also available via HTTPS, but a login is required first. The synchronisation feature is therefore unlikely to be usable in its current form for many existing food coops. This does not affect the manual import of files, provided Foodsoft supports the format.
• I suggest implementing the previous FTP-BNN and mail functionality directly in Foodsoft (without Sharedlists) after consultation with existing foodcoops or wholesalers. (If we agree on this, we should open an issue.)
• A note is required in the release notes or a comparable document that the existing links to the external sharedlists suppliers will be lost.

Units

• The migration 20240726083743_create_article_units.rb inserts ‘unit codes’ directly into the database. We should consider inserting the data as seed data at any time. This was noticed because the migration doesn't work with a SQLite database (use of NOW()).
• Further questions and comments on the units will follow.

I ran into an issue with the migration 20240726083740_alter_articles_add_versioning step

@RayOei Thanks for reviewing and trying it out with your data!
I can't quite follow what's going wrong. 🤔 Maybe it would be simplest, if you could provide a rails seed file in a branch forked from the current master, after which - when switching to this PR's branch - the migrations would fail. I could then debug it on my machine.
(Or alternatively - if a seed file is too much effort - you could just provide the original primary and foreign keys in a tabular layout, so it would be easier to manually reproduce.)

I consider a complete code review to be impossible. With such a large scope, the changes would have to be divided into several functionally complete and consecutive commits that can be reviewed one after the other.

Yes, I agree. Like I said elsewhere: The reason this PR has gone out of bounds, is that we started with an issue (article units) that was blocked by several other issues (namely article versioning and dependencies to sharedlists).

So in theory it could have been done in three separate PRs, which would have been easier to review (article versioning -> sharedlists replacement -> article units) - but unfortunately it would mean a lot of effort dividing the changes into functional units.

@RayOei And about your questions about synchronization - it tried to explain the general idea here, but maybe it wasn't very clear.

Since - as was pointed out - this PR is so big, I think discussing things like this as global comments here will quickly lead to chaos, so I suggest either we need to find another way. I'll message you about it and we get back to GitHub as soon as we figured out how to best communicate here.

I'll message you about it and we get back to GitHub as soon as we figured out how to best communicate here.

See here: #1087 (comment)