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

[8.0][partner_contact_vat] Module to have and check VAT field for contacts #177

Closed
wants to merge 2 commits into from
Closed

[8.0][partner_contact_vat] Module to have and check VAT field for contacts #177

wants to merge 2 commits into from

Conversation

yajo
Copy link
Member

@yajo yajo commented Oct 1, 2015

Related to OCA/l10n-spain#174.

This module allows to have a VAT for each contact, separate from the company's.

It does not interfere with any other logic with VAT

has already been reported. If you spotted it first, help us smashing it by
providing a detailed and welcomed feedback `here <https://github.com/OCA/
partner-contact/issues/new?body=module:%20
partner_contact_vat%0Aversion:%208.0.3.0.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
Copy link
Member

Choose a reason for hiding this comment

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

Put 8.0

@yajo
Copy link
Member Author

yajo commented Oct 8, 2015

README updated.

@rafaelbn
Copy link
Member

Just a question, It's not need to validate it? Any number is ok?

2015-11-12_3-53-34

@yajo
Copy link
Member Author

yajo commented Nov 18, 2015

Validation is performed by partner_contact_vat_check, which is on hold until this is merged.

@pedrobaeza
Copy link
Member

Why 2 modules for this little task? The validation should be performed also in this PR.

@yajo
Copy link
Member Author

yajo commented Nov 18, 2015

Odoo separates validation of VAT in base_vat module, so I followed their schema. What if you do not want validation?

@pedrobaeza
Copy link
Member

But that it's because base module contains a lot of things, so isolation is logical, but not in this case. You shouldn't allow to enter a vat that can be invalid, and later let that vat to get into the partner.

@yajo
Copy link
Member Author

yajo commented Nov 18, 2015

We are arguing about future code that has not been reviewed, let's keep present comments to present code please. If in the future PR seems so useless to have 2 modules, I'll just merge it into this one, but that's a problem for the future us.

@pedrobaeza
Copy link
Member

No, it's not future problem. It's a feature needed to validate this PR, so please include it.

@yajo
Copy link
Member Author

yajo commented Nov 18, 2015

Done.

@yajo yajo changed the title Port partner_contact_vat to OCA. [8.0][partner_contact_vat][partner_conact_vat_check] Modules to have and check VAT field for contacts Dec 23, 2015
@yajo yajo changed the title [8.0][partner_contact_vat][partner_conact_vat_check] Modules to have and check VAT field for contacts [8.0][partner_contact_vat] Module to have and check VAT field for contacts Jan 5, 2016
@yajo
Copy link
Member Author

yajo commented Jan 5, 2016

Both modules have been simplified and merged. Now only 1 module to have VAT and check it.

@yajo
Copy link
Member Author

yajo commented Jan 14, 2016

Runbot fails. Who can we ping? Seems like it lacks the OCA_RUNBOT env var.

It's also affected by a problem that is fixed in #233.

@yajo
Copy link
Member Author

yajo commented Feb 2, 2016

Travis fails until #245 is merged.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 55.194% when pulling 5edb9d0 on grupoesoc:contact-vat into 6df33c2 on OCA:8.0.

@dreispt dreispt added this to the 8.0 milestone Apr 7, 2016
@rafaelbn
Copy link
Member

rafaelbn commented Jul 1, 2016

Well I think we can review this again and get it merge. Rebuilding

@yajo
Copy link
Member Author

yajo commented Dec 16, 2016

Closing in favor of partner_identification.

@yajo yajo closed this Dec 16, 2016
@yajo yajo deleted the contact-vat branch December 16, 2016 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants