Only display Supplier column in Order Cycle Report email if there is more than one supplier

[cillian]

What? Why?

• Closes Remove the "supplier" column from the order cycle notifications email #11601

Note, the failing test on this branch is one that is failing on the master branch.

What should we test?

1. Create an order cycle with orders containing products from more than one supplier. Click the Notify producers button and check that the Order Cycle Report email contains a 'Supplier' column.
2. Create an order cycle with orders containing products from only one supplier. Click the Notify producers button and check that the Order Cycle Report email does not contain a 'Supplier' column.

Release notes

Changelog Category (reviewers may add a label for the release notes):

• User facing changes
• API changes (V0, V1, DFC or Webhook)
• Technical changes only
• Feature toggled

[mkllnk]

Not related to this PR but I'm wondering why the name is raw here. It looks like it got copied from the text template without knowing that it's introducing a security issue. A supplier could inject HTML in the name.

• 6ecf896#diff-8811f57637800c39d8e1e157c4d1fca015d5d0c9ed4fb8eb277210315396e797

[rioug]

This will be addressed here #11801

[rioug]

Nice one 👍

[filipefurtad0]

Hey @cillian,

Below a summary of my findings:

1. OC with multiple suppliers: the supplier column is removed, which is great, but the indentation for the summary row is a bit off, which compromises readability
   

2. OC with one supplier only: we find the same issue with the summary row indentation, but also, when grouping the orders by customer(* please see side note below), we can see that the supplier column is still there:
   

I'm a bit hesitant to merge this one: it would remove the column in most cases, but perhaps best to fix the summary row indentation before shipping?

Also, would be great if we could remove the Suppliers column from the table which groups orders by customer.

Would it be ok to address these two points, within this PR? The alternative would be to split this and create separate issues, but perhaps it is doable. Please let me know what you think.

Moving back to in dev.

• Side note: after some head-scratching, I've found out how to enable this option: this can be done on the Customer names on reports option, on the dashboard:

[cillian]

@filipefurtad0 Thanks, that's fixed now, I should have spotted that.

Before

After

Before (with customers)

After (with customers)

[drummer83]

Hi @cillian,
ooof, this was a hard one to crack. 😆
First of all a big Thank you to @filipefurtad0 for laying the ground with his testing notes above. 💚

Regarding Filipe's findings

1. Incorrect indentation of summary rows.
2. Hide Suppliers column for the list grouped by customers as well.

Those are both fixed now, as we can see here:

HOWEVER

I think there is another misunderstanding underlying here.
To make the Suppliers column useful I tried to find a test setup with two different suppliers actually being listed in that column. But I couldn't, no matter what... So I looked at the code to understand how this column is filled and I came across distributors_pickup_times. This was a hint towards distributors and I set up an order cycle with two distributors. Looking at the notification email then made me realize what the original idea of this email might have been:

In this email

• the supplier can see where his/her products have been sold (the list of different distributors),
• the supplier can see how many items of each product in total have been sold (8 pieces in the example above),
• BUT the supplier cannot see how many items of each product need to be delivered to which distributor - that would make sense, right?
• The supplier column is always containing just one value, because as @audez mentioned in the issue the email is always sent to just one producer.

Conclusion

• I think this email would make much more sense if instead of the suppliers the distributors were listed as a column instead.
• It still makes sense to only display this column, if there are more than one distributors involved.

I may be totally wrong, but this is my conclusion for today.

I think we can safely merge this because it is an improvement.
We might consider replacing the supplier column with the distributor column.

What do you think, Cillian? Does this make sense?
@openfoodfoundation/train-drivers-product-owners Any comments about the history of this email maybe?

I will add the feedback needed label for now until we know how to move on.

Thanks again! 🙏

[cillian]

Hi @drummer83 Nice catch, yes I think displaying the distributor makes more sense.

One other thing I noticed, I think the same product can be sold for multiple distributors (not 100% sure about this e.g. maybe my local data not setup correctly). So I was wondering if there is multiple distributors do we need to show a list for each distributor like in this following example? This would show people the total amount of each product for each distributor, which could be for multiple customers. Note, the potatoes quantity is 2 because it was sold at multiple distributors.

And here is an example with just one distributor:

[RachL]

I think the same product can be sold for multiple distributor

Yes! In FR, distributors are often just a pickup places so some managers can have OC with over 10 distributors 🙈

Any comments about the history of this email maybe?

No idea! maybe @kirstenalarsen has some hints? But I concur that it would make sense to display distributors when there are more than one. I think this could be open as a new papercut straight in main repo 👍

[drummer83]

Thanks, @cillian!
Your examples look great to me! Makes perfect sense.

I will merge this one and open a new issue to remove the supplier column and add the tables per distributor in the way you showed in your comment! ➡️ #11916

Thanks - amazing work! 🙏