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

[FIX] l10n_ar: proper compare document numbers in vendor bills #41196

Closed

Conversation

@zaoral
Copy link
Contributor

zaoral commented Dec 2, 2019

Description of the issue/feature this PR addresses:

When creating vendor bills, it was considering this two numbers as different but actually are the same invoice number

  • 01234-00000001
  • 1234-00000001

To avoid this we remove the zero to the left always if the pos number > 4.

Current behavior before PR:

  1. Create a vendor bill for Odoo Inc
  2. Set Document number to 01234-00000001
  3. Validate invoice.
  4. Create a second vendor bill for same Odoo Inc partner.
  5. Set Document number to 1234-00000001
  6. Validate invoice.

It validate both invoices but it should not, because the document number is the same and can not be used twice for the same vendor.

Desired behavior after PR is merged:

With the change we made now 01234-00000001 is saved as 1234-00000001, so now when try to validate an invoice with the same document number will raise the uniqueness error.

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

When creating vendor bills, it was considering this two numbers as different but actually are the same invoice number

* 01234-00000001
+ 1234-00000001

To avoid this we remove the zero to the left always if the pos number > 4.
@robodoo robodoo added the seen 🙂 label Dec 2, 2019
@zaoral

This comment has been minimized.

Copy link
Contributor Author

zaoral commented Dec 2, 2019

Hi @jjscarafia @ren-odoo @jco-odoo

Please could you review this PR and let me know what do you think.

Best Regards!

Copy link
Contributor

jjscarafia left a comment

LGTM

@robodoo robodoo added the CI 🤖 label Dec 2, 2019
@jco-odoo

This comment has been minimized.

Copy link
Contributor

jco-odoo commented Dec 3, 2019

@zaoral What happens when you have 2 0s?

@zaoral

This comment has been minimized.

Copy link
Contributor Author

zaoral commented Dec 3, 2019

@jco-odoo this will happen: 00123-00000001 will transform the number 0123-00000001

It really do not depends on the quantity of 0 to lthe left, will depend of the length of the number before the dash. This number should have 4 o 5 digits long. We only ensure that is the number is 5 digits longs that all the numbers are representative, if not we remove the zero to left in order to represent the number as a 4 digit number.

@jco-odoo

This comment has been minimized.

Copy link
Contributor

jco-odoo commented Dec 3, 2019

robodoo pushed a commit that referenced this pull request Dec 3, 2019
When creating vendor bills, it was considering this two numbers as different but actually are the same invoice number

* 01234-00000001
+ 1234-00000001

To avoid this we remove the zero to the left always if the pos number > 4.

closes #41196

Signed-off-by: Josse Colpaert <jco@openerp.com>
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Dec 3, 2019

Staging failed: ci/runbot (view more at http://runbot.odoo.com/runbot/build/716139)

@jco-odoo

This comment has been minimized.

Copy link
Contributor

jco-odoo commented Dec 3, 2019

@robodoo retry

@robodoo robodoo added CI 🤖 r+ 👌 and removed error 🙅 labels Dec 3, 2019
robodoo pushed a commit that referenced this pull request Dec 3, 2019
When creating vendor bills, it was considering this two numbers as different but actually are the same invoice number

* 01234-00000001
+ 1234-00000001

To avoid this we remove the zero to the left always if the pos number > 4.

closes #41196

Signed-off-by: Josse Colpaert <jco@openerp.com>
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Dec 3, 2019

Staging failed: ci/runbot on e73e8e72df7169e95ed38b9d85cab907f3236805 (view more at http://runbot.odoo.com/runbot/build/716308)

@zaoral

This comment has been minimized.

Copy link
Contributor Author

zaoral commented Dec 5, 2019

Hi @jco-odoo

About this PR. For some reason is not doing the merge.
There is anything I can help? please let me know if you will like me to rebase this PR

@Xavier-Do

This comment has been minimized.

Copy link
Contributor

Xavier-Do commented Dec 5, 2019

robodoo retry

@robodoo robodoo removed the error 🙅 label Dec 5, 2019
@robodoo robodoo added the merging 👷 label Dec 5, 2019
@robodoo robodoo closed this in 15543eb Dec 5, 2019
@robodoo robodoo added merged 🎉 and removed merging 👷 labels Dec 5, 2019
@robodoo robodoo deployed to merge Dec 5, 2019 Active
@zaoral zaoral deleted the adhoc-dev:13.0-l10n_ar-vendor-bill-number-uniqueness branch Dec 5, 2019
@fw-bot

This comment has been minimized.

Copy link
Contributor

fw-bot commented Dec 9, 2019

This pull request has forward-port PRs awaiting action (not merged or closed): #41436

@fw-bot

This comment has been minimized.

Copy link
Contributor

fw-bot commented Dec 9, 2019

This pull request has forward-port PRs awaiting action (not merged or closed): #41432

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.