Add columns to "pay your supplier" report

[chahmedejaz]

⚠️ Please use clockify code #12476 Flower Farms

What? Why?

• Closes Add columns to "pay your supplier" report #13013
• This PR adds the following columns as per the issue:
  • Producer charges GST?
  • Total tax on product

What should we test?

• As mentioned in the issue

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

[rioug]

Looks good 👍 Note for next time, it's better to add the specs in the same commit as the changes you make. It makes reviewing easier as we can look at the changes and specs on the same page.

[rioug]

Given the specs are passing, I assume the boolean are auto-magically translated to Yes/No ?

[dacook]

I was curious too, so I found out the answer here: https://github.com/openfoodfoundation/openfoodnetwork/blob/master/lib/reporting/report_row_builder.rb#L120-L122

I've been interested to try this gem for this sort of thing: https://github.com/RST-J/human_attribute_values
It would allow translators to choose a more specific description, which might be better in some circumstances.

But I think handling it this way is quite sufficient and much simpler, until someone says we need a better solution.

[dacook]

It seems strange that the report needs to check if charges_sales_tax. I thought that the application logic should be able to work this out. I see that Spree::TaxRate checks for this in match, but I don't think that helps here.

Oh well, it turns out it's quite common to refer to this directly in other reports, all good 👍

[dacook]

Nice to see some of the logic ends up simpler! 😎

[dacook]

I was curious too, so I found out the answer here: https://github.com/openfoodfoundation/openfoodnetwork/blob/master/lib/reporting/report_row_builder.rb#L120-L122

I've been interested to try this gem for this sort of thing: https://github.com/RST-J/human_attribute_values
It would allow translators to choose a more specific description, which might be better in some circumstances.

But I think handling it this way is quite sufficient and much simpler, until someone says we need a better solution.

[chahmedejaz]

Note for next time, it's better to add the specs in the same commit as the changes you make. It makes reviewing easier as we can look at the changes and specs on the same page.

Sure, @rioug will take care of it from now onwards. Thanks 🙂

[chahmedejaz]

I've been interested to try this gem for this sort of thing: https://github.com/RST-J/human_attribute_values
It would allow translators to choose a more specific description, which might be better in some circumstances.

Thanks for introducing this gem, David. Yes, it seems more flexible indeed. Maybe in future we may find a usecase to add this 😬

[filipefurtad0]

Hey @chahmedejaz ,

Thanks for this work and I was able to verify that:

• the two new columns are now included, when generating the report 🟢
• these are de-selected, by default 🟢

However, it seems the TOTAL for the Total tax on Product ($) column is not appearing. I'd think this is necessary requirement - following the logic from other columns. I'll move this issue to "In Progress" for now, so the Total can be added, accordingly.

[RachL]

@filipefurtad0 thanks! When this is back in testing, FYI Theresa has an error in production I'm not able to reproduce yet. If you have a magical idea what it can be linked to^^ #12879 (comment)