Add option to ignore delivered amounts in order group distribution

[lentschi]

Current behavior

When receiving an order (using OrdersController#receive) and supplying less than the full amount of articles ordered, the distribution among order groups currently works as follows:
The order groups that first placed their group orders will receive (-> GroupOrderArticle#result_computed) full amounts. Those who ordered later will receive only what is left (if anything). So it could be described as a "first order first serve" rule.

However in our (and I think in many other) foodcoops this isn't a viable solution. (The sequence of ordering and fetching stuff is almost always different from each other.) So I suggest the following change:

Suggested change

Add a checkbox to the others config page: "Ignore received amounts when charging order groups"
When this is checked, the order groups are always assigned (and subsequently charged) the full amounts of what they ordered.
(Amounts can still be modified at a later time through the existing interfaces or - once implemented - through #766.)

When the new checkbox remains unchecked (default), the behavior stays the same as it is now.

[wvengen]

I can imagine that this works differently in some foodcoops.
The receive screen was originally added to redistribute on receiving, but I can imagine that just registering the received amount can be useful aswell.

Would the checkbox here belong in the receive screen, in the supplier details, or be a foodcoop-wide option: what do you think?

[lentschi]

Would the checkbox here belong in the receive screen, in the supplier details, or be a foodcoop-wide option: what do you think?

The way I started implementing it (see 8c2ba9e), it would be a foodcoop-wide option. Are you suggesting we should allow different ways of calculation per supplier?

Maybe - since each foodcoop might have different requirements - the calculate_result method could be adapted to be easy to override (e.g. by defining a plugin API for hooking into calculation)...?

[wvengen]

I think it would be useful to have an adaptable calculate_result method. I know of other foodcoops who were pondering if the tolerance would not be distributed on a first-come-first-serve-basis, but spread over all GroupOrderArticle tolerances.

Some parts could be split off from calculate_result into separate methods, which can be overridden in plugins. I'm not sure right now what parts to splitt off, suggestions welcome.

[lentschi]

I think it would be useful to have an adaptable calculate_result method.

@paroga and I just talked about this in depth and concluded that making this adaptable (by plugins etc.) would be quite complicated and too much effort for now. Instead we would suggest to turn my new settings checkbox (ignore_received_units_on_redistribute) into a select with the following three options:

• no automatic distribution (equals checking the already existing checkbox charge_members_manually)
• first order first serve (current calculation)
• distribute as ordered - ignoring actually received units (my new calculation; only makes sense once Add a page allowing to modify the received article amounts for "normal" users #766 has been implemented)

What do you think? (We could later still add plugin support of some kind as a fourth option, but that would be another issue and require some brainstorming.)
If you agree, but have better suggestions for the select options' labels, don't hesitate to share them 😉

[wvengen]

Quick response: that sounds good, thanks for talking this through!

Note that charge_members_manually has nothing to do with distribution, but with creating financial transactions (here).

[lentschi]

Quick response: that sounds good, thanks for talking this through!

👍

Note that charge_members_manually has nothing to do with distribution, but with creating financial transactions

Hm... Yes, but the side effect of this is, that there is no distribution, right? (Or to put it differently: Transactions are just caused by distribution.) But maybe I missed something here. @paroga You suggested this merging of settings - are you sure it would work in all cases or did I misunderstand something?

[paroga]

Hm... Yes, but the side effect of this is, that there is no distribution, right?

The distributions happens after receiving an order and has noting to do with the balancing. Even if you charge your members manually it can make sense to have the correct/different distribution on the group order pdf.

You suggested this merging of settings

no, not really, since those two settings are independent. sorry if i didn't communicated that clearly.

[lentschi]

You suggested this merging of settings

no, not really, since those two settings are independent. sorry if i didn't communicated that clear

Oh - I guess I misunderstood this then. But we did have a discussion in which we agreed on those three options - right?:
#764 (comment)

So, if the first option of this new config select ("no automatic distribution") does not equal checking charge_members_manually then what should it signify?

[paroga]

oh, is see where this misunderstanding comes from. my idea of no automatic distribution was that nothing happens. this means that if i order 2, but only 1 gets delivered foodsoft will not change my received amount to 1. so the received amount of all members needs to be changed manually

[lentschi]

Sorry, I'm still confused 😉

my idea of "no automatic distribution" was that nothing happens. this means that if i order 2, but only 1 gets delivered foodsoft will not change my received amount to 1. so the received amount of all members needs to be changed manually

Ah okay - so but that describes exactly how foodsoft would now behave, once the new checkbox I've already implemented in my PR for this issue has been checked. Then there wouldn't be any need for replacing that with a select as you originally suggested and the charge_members_manually would remain untouched... Or do you still see a need for a select? (In that case please elaborate what the meaning of each of the select's options would be!)

[paroga]

sorry of not being precise again.

i imagine an select box "distribution strategy" (or any better name and independent of :charge_members_manually) for the setting with three items:

• first order first serve (current calculation)
• no automatic distribution (equal to your PR)
• fair distribution (if two ordergroups each order 2 items (4 total), but only 2 get delivered, then each ordergroup gets 1 item)

so that's more of a UI change to your existing PR. you don't need to implement the third option yourself, but I'd like to have the UI more extendable. I think that selecting a "distribution strategy" might be more self explanatory than "Ignore received amounts when charging order groups".

[lentschi]

Aaah, I must have missed the fair distribution option - that wasn't on my radar at all. Thanks for explaining it again!

you don't need to implement the third option yourself

Okay, but do you still want it implemented before the issue can be merged or should we proceed with a select with just the first two options for now? (Personally, I think implementing fair distribution would call for refactoring and unit testing the calculate logic first, so I'd try and postpone that for now.)

[paroga]

you can just implement the two options, no need for the third now. i just want to avoid changing the UI if we get a third (or even more) option

[lentschi]

Done -> I removed 'Draft' status from #765.

The English, German, Spanish and French translations should be fine. @wvengen Can you maybe check the Dutch translations (I used Google Translate for that.)

[paroga]

you can just commit the changes with the languages you know. missing translations will be added before a release. it's easier to make one translation round before release than having to get the translations for every minimal change