New receive screen (ready)

[wvengen]

When the received amounts differ from those ordered, redistribution of changed quantities over members is manual. This pull request adds a "receive" screen in the orders section, where amounts received can be entered, after which products are redistributed over members using the existing algorithm.

Includes the addition of a units_received field in OrderArticle. An additional units_billed field is created as well although it has no user-interface yet (but this is very much similar to this screen).

Where OrderArticle#units_to_order was used, you'll probably want to use OrderArticle#units, which takes the last entered amount (ordered, billed, received - in that order). Where the amount of units of closed orders is shown, there now is a title attribute showing the history (e.g: "2 ordered, 1 billed, 1 received").

A new convention is introduced: whenever a number is the amount of wholesale units (full boxes, unit_quantities, lot, whatever), a watermark image of a box is shown next to it. I hope this brings clarity to whether numbers are member units or wholesale units.

See also foodcoop-adam#2 and foodcoop-adam#3.

• Get feedback on the concept
• Clarify balancing exclamation mark for received units in balancing (see comment below)
• Make update of amount of units in balancing work when units_received is set
• Use ajax publish/subscribe design pattern
• Find a way to minimise the risk of receive overwriting manual updates done in balancing
• Edit button for the order article
• Hide/disable article amount fields in edit article for receive
• Warning sign when unlock button was pressed
• Tests for when received units field is disabled or not
• Integration test for ordering and changing received units
• Review right delta (+1/-1/v) mark
• Finish i18n
• update German translation
• workaround Label pluralization heartcombo/simple_form#974 or revert pluralisation of activerecord attributes
• only show package icon when unit_quantity>1 (edit box and delta)
• make delta work when unit_quantity was changed
• fix notice after receive (always shows "No new articles to receive")

Former tasks which where reorganized or deleted:

• Vertical align of cells -> Center table rows vertically #247
• Redistribute when unlock was pressed, even if amount was unchanged (?)

After this, next steps would be

• allow to select a different product for an ordered article (and move the ordergroup orders with it); possibly also support a replacement in a different order for a different supplier
• add a similar screen to enter products on the invoice (perhaps somewhere in the "new invoice" screen),
• add a similar screen when closing the order, so that adjustments can be made before sending it to the supplier, with foodsoft knowing about them

[JuliusR]

From my point of view, this is (in principle) the way how the user interface should be designed: For each task in the real world, there should be a specific clear form.

However, the workflow in our foodcoop seems to be a bit different from yours. At that day when the products arrive, a small team of people (2-4) is responsible for distributing the products to the boxes of the ordergroups and the rest to the stockit. If there are deviations from the ordered amounts, the updated values are currently entered in the balancing form in order to also change the billing for the ordergroups. In the new form suggested here there is no possibility to change the distribution to the ordergroups, so how is this meant to be done?

Anyway, some more comments:

• Can you explain the checkboxes below? The second one is always disabled for me and the purpose of the first one is not absolutely clear.

      Surplus to member tolerance, and stock
  

• How about some default values in the input fields? This way, it would be easier to change the amounts for some products only. Currently, an exception is thrown if inputs are empty.

• Why do you disable the edit link for orders in process? By URL manipulation you still get there. This is the only way to add Articles to the order because the select list in the form only shows those articles which are already included in the order.

• What do you mean by

  Find a way to minimise the risk of receive overwriting manual updates done in balancing
  

• In the balancing screen: Did you by purpose change the conditions for the exclamation mark to show up? Previously, I think it was meant to indicate that the totally delivered amount is not distributed properly to all ordergroups. Currently, even those quantities which are sent to the stockit should be assigned to some artificial ordergroup, e.g., stockit orderer, so that in total the sum is correct. With this change, the exclamation mark shows up if the amount ordered differs from the amount received. From my point of view, this is an info (question mark) rather than an alert (exclamation mark).

[wvengen]

Thanks for your extensive reply! What you describe is similar to what we do, only we do not track our stock using foodsoft.

What the receive screen does is not only storing the number of received units, but also re-running the distribution algorithm (same as when the order is closed) to update the ordergroup amounts. This is also what the checkboxes at the bottom do: they control where the rest of the articles go after members got their quantities. One option is tolerance (which is what happens when the order is closed), the other is stock (which is not implemented yet, that's why it is disabled for now).

It probably makes a lot of sense to add a notice that ordergroup amounts are being changed as well, especially when they have been manually updated (if we can detect that).

• Surplus means here the rest of the articles after members got their quantities. Could probably use a clearer formulation :)
• I thought it was best not pre-fill the input box, so that it is possible to track which amounts were filled in and which not. I think it would cause errors if the amount received was pre-filled, people could easily press submit without really checking. What do you think? And, oops, exception when an input field is empty? I need to fix that, thanks.
• Previously these orders would be shown with the settled orders, and no edit button was present. Remember that the order is closed already. And yes, it is possible to edit it, but not encouraged. I also plan to implement an ordering screen soonish, where it is possible to modify the amounts to be ordered at the moment that an order is closed, which allows for removing articles from and adding them to the order at that time.
  • I meant to include all supplier's articles in the "add article" box - will change that. (And in the future, add the option to import products from the shared supplier too).
• Imagine that most products are distributed, and that there are some last-minute changes to which ordergroup gets what. Someone uses the balancing screen to update these amounts in foodsoft. Then someone else sees that the number of units received of a product is different than expected and changes that in the receive screen. This will redistribute everything, and all the manual updates to the ordergroup amounts done earlier are lost. This is something I'd like to avoid, or at least clearly warn that these manual changes are being lost.
• Ah, as we don't use stockit I hadn't caught that one! So for stock orders, there should be no receive screen. Still its behaviour shouldn't have changed - the old code shows OrderArticle#units_to_order, and the new code shows that as well (unless units_received or units_billed is set - something that shouldn't be happening for a StockArticle). Perhaps it's best to enforce that in StockArticle.

Does this make sense?

[JuliusR]

Thanks for the explanations. I think I have a better idea of your intention now. I will now try to focus on the fundamental topics here instead of commenting on minor issues.

What the receive screen does is not only storing the number of received units, but also re-running the distribution algorithm (same as when the order is closed) to update the ordergroup amounts.

Yeah, that is in principle a useful feature. Since we as foodcoop members need to check the delivered products anyway, it is a nice idea to combine this step with an update of the order in the foodsoft (instead of drawing ticks and crosses on a sheet of paper).

However, in order to be production ready for us I see the need of possibly tricky improvements:

• add option to edit the prices, units, and unit_quantities of the delivered products (possibly with an option to apply the changes globally)
• allow for an easy replacement of one product by another one; including a smooth transfer of the ordergroup demands and tolerances

Do you feel motivated to implement these things in a team before going for a merge or do you prefer to introduce the feature more quickly? In the latter case it would be important for us that the old-fashioned pen-and-paper method would still work as usual.

In the balancing screen: Did you by purpose change the conditions for the exclamation mark to show up? [...]

Ah, as we don't use stockit I hadn't caught that one! So for stock orders, there should be no receive screen. Still its behaviour shouldn't have changed - the old code shows OrderArticle#units_to_order, and the new code shows that as well (unless units_received or units_billed is set - something that shouldn't be happening for a StockArticle). Perhaps it's best to enforce that in StockArticle.

You are right that stock orders should not have a receive screen. However, I was referring to a different phenomenon. The exclamation mark in the balancing screen now shows up if, e.g., 0 ordered, 1 received. I did not check the code but if my memory is right, this is new. Previously, the exclamation mark had a very special meaning and I would like to keep this assignment. Would you mind using an, e.g., question mark in the balancing screen if the received amount differs from the ordered one?

[wvengen]

Thanks Julius, also for seeing an overall picture of this.

• I'd like the paper-method to remain functional even when this feature is fully complete (so foodcoops can transition whenever they want, not when a software update dictates it).
• I'd love to work on this as a team to get it completely functional for all groups who use foodsoft.
• I lean towards wanting to merge step by step, with each step a small but complete improvement. Keeps us agile. (I have too many branches laying around with big but unfinished changes).
• Good point about the exclamation mark. Will do. This can keep existing behaviour and still make sense when receive is being used:
  • If received is not set and ordered matches ordergroups, show nothing.
  • If received is not set and ordered does not match ordergroups, show exclamation mark.
  • If received is set and matches ordergroups, show nothing.
  • If received is set and does not match ordergroups, show question mark.

[wvengen]

Ok, that's it, I think. Do you think it's ready? If so, let's:

• merge master
• add "(ready, please review)" to pull request title
• wait a couple of days to give others a chance for feedback, and merge

[wvengen]

p.s. Any reason why you put the table footer above the body? Since it can be quite a long list of new articles to add, I'd like to load the table body first.

[JuliusR]

Ok, that's it, I think. Do you think it's ready?

I do not know when I will have time to look at it carefully. Maybe at the weekend. Feel free to merge master already.

p.s. Any reason why you put the table footer above the body?

Yes, seems like I am old. Did not know that is is allowed after the tbody now. In an old version of the foodsoft, I once experienced the tfoot not being displayed at all because it was placed after the tbody.

See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/tfoot

The tbody must appears after any caption, colgroup or thead element. It can be before or after all tbody and tr elements, but not intermixed with them.

HTML 4 The tfoot element cannot be placed after any tbody and tr element. This restriction has been softened in HTML5.

[wvengen]

This restriction has been softened in HTML5.

Ah, didn't realise that - thanks for looking it up!

[JuliusR]

Nice.

From my point of view we can

add "(ready, please review)" to pull request title

Since you might have a reason for not having done so yet, I will leave it up to you.

After some days I will also have a second look on all changes here.

[wvengen]

Great! I've spotted some small bugs (see description), when these are fixed I consider it ready.

[wvengen]

It occured to me that this might be a bit long for the green button. Would "empfang(en)" be an option?

[JuliusR]

It occured to me that this might be a bit long for the green button.

Do you care about aesthetics or the usability on small screens? The other options I can think of are definitively worse although they might be acceptable. I would vote for a change if there is a problem but just for symmetry I would not worsen the "understandability".

However, there might be a term which I just can not think of. Suggestions are welcome.

[wvengen]

The size of the button attracts my attention. And yes, I was thinking of smaller screens too. But I wouldn't want to sacrifice clarity! I'll see if some German person here has any ideas :) Not really important, but just checking.

[JuliusR]

Ah, I see your point about the eye-catching thing. I will keep this in mind if I find a nice short replacement.

[wvengen]

Since it's been a week, I'll start merging this. To be safe, I've pushed the pre-receive version of foodsoft to a foodsoft-archive branch.

[JuliusR]

Sorry, I have not found time yet to review all the stuff.

For me it is fine - go on.

[wvengen]

Ok! Thanks for the note.