Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Invoices] Notify if any order cannot be invoiced on bulk invoice sending #12458

Conversation

abdellani
Copy link
Member

@abdellani abdellani commented May 9, 2024

What? Why?

What should we test?

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

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@abdellani abdellani self-assigned this May 9, 2024
@abdellani abdellani force-pushed the 12374-bulkk-sending-invoice-must-show-error-if-abn-require-and-not-set branch 5 times, most recently from 3e86db5 to 3d32f70 Compare May 17, 2024 13:24
@rioug rioug added technical changes only These pull requests do not contain user facing changes and are grouped in release notes feature toggled These pull requests' changes are invisible by default and are grouped in release notes and removed technical changes only These pull requests do not contain user facing changes and are grouped in release notes labels May 20, 2024
def distributors_without_abn(orders)
Enterprise.where(
id: orders.select(:distributor_id),
abn: [nil,""],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty string abnisn't cover by specs, could you update that ?

@abdellani abdellani force-pushed the 12374-bulkk-sending-invoice-must-show-error-if-abn-require-and-not-set branch from 7a05c7f to 9caa9db Compare June 7, 2024 16:58
@abdellani abdellani force-pushed the 12374-bulkk-sending-invoice-must-show-error-if-abn-require-and-not-set branch from 9caa9db to b0862d2 Compare June 7, 2024 16:59
@abdellani abdellani requested a review from rioug June 7, 2024 16:59
Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Good now, Thanks @abdellani !

@filipefurtad0 filipefurtad0 self-assigned this Jun 12, 2024
@filipefurtad0 filipefurtad0 added pr-staged-fr staging.coopcircuits.fr and removed pr-staged-fr staging.coopcircuits.fr labels Jun 12, 2024
@filipefurtad0
Copy link
Contributor

Hey @abdellani ,

Tried both described scenarios, after making ABN mandatory for the instance, and deleting the ABN for the enterprise. On the orders page:

  • attempting to print the invoice

image

  • attempting to send the invoice per email

image

Looks great! Merging :-)

@filipefurtad0 filipefurtad0 merged commit b1f7807 into openfoodfoundation:master Jun 12, 2024
53 of 54 checks passed
@dacook dacook changed the title notify the user if any order cannot be invoiced on bulk invoice sending [Invoices] Notify if any order cannot be invoiced on bulk invoice sending Jun 13, 2024
@RachL RachL added technical changes only These pull requests do not contain user facing changes and are grouped in release notes and removed feature toggled These pull requests' changes are invisible by default and are grouped in release notes labels Jun 13, 2024
@filipefurtad0 filipefurtad0 removed the pr-staged-fr staging.coopcircuits.fr label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical changes only These pull requests do not contain user facing changes and are grouped in release notes
Projects
Status: Done
5 participants