-
Notifications
You must be signed in to change notification settings - Fork 28.8k
[REM] base: company_type is dead for good #211043
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
base: master
Are you sure you want to change the base?
[REM] base: company_type is dead for good #211043
Conversation
f35a33f
to
4d2465e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @yagp-odoo 👋 ,
Left some comments
But it feels strange that we sometimes use vat or parent_id in place of is_company(earlier it was easy to distinguish)
It might cause undesirable behaviour to the many flows
Can't we rely on parent_id only?
Thanks
@@ -184,7 +184,7 @@ def _compute_vies_valid(self): | |||
partner.vies_valid = partner.parent_id.vies_valid | |||
continue | |||
try: | |||
vies_valid = check_vies(partner.vat, timeout=10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we remove the timeout here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this was giving me unexpected argument 'timeout' on runbot,
Well Fixed now, the timeout was not present in test's setup class where it was patched. 🙃
addons/crm/models/crm_lead.py
Outdated
@@ -2006,8 +1948,7 @@ def _prepare_customer_values(self, partner_name, is_company=False, parent_id=Fal | |||
'website': self.website, | |||
# company / hierarchy | |||
'parent_id': parent_id, | |||
'is_company': is_company, | |||
'company_name': not is_company and not parent_id and self.partner_name, | |||
'company_name': not parent_id and self.partner_name or self.partner_id.company_name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do have to remove the company_name part as per "new request" in the pad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the specs says remove this feature where we store a company name with a create btn, instead we should always create the company
, so it's just asking me to remove the feature of creating company on button click...
by doing what I did above, we always have company_name, so that create_company
won't be needed 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should completely get rid of company_name
as it will no longer used
what do you think ?
@@ -67,9 +67,6 @@ def res_partner_enrich_and_update_company(self, partner_id): | |||
if not partner: | |||
return {'error': _("This partner does not exist")} | |||
|
|||
if not partner.is_company: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we check parent_id here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was to check contact must be a company, but aren't we trying to get rid of that distinction?
Also, Company 1 > can have a Company 1.1 in child. So it will throw error there, right?
Maybe checking like Contact Must Have a VAT
should be ok?
4d2465e
to
ab1ae35
Compare
Hello @mrsr-odoo 👋
Agreed it might cause unexpected outcome, but we mainly wants to check for VAT, as any registered company must have a VAT.
Yes, but from what I understood, we're now wants to check its subject to VAT or not, not by distinguish its person or company.
In some cases YES, we can like for checking its root contact,..etc. A contact with False parent_id doesn't necessary have to be a Company, so checking VAT in most cases should do I think. Though this can still be change when l10n modules come into play, Accounting Team/s may wants something different 🙃 |
bdf5c9c
to
8c6b21a
Compare
@qdp-odoo I bet somehow this should interest you at some point. Draft and not finished in any way but hey. |
b85b531
to
a813bc9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @yagp-odoo 👋 ,
Left some comments
Some conflicts to resolves
Thanks
@@ -22,6 +22,7 @@ registry.category("web_tour.tours").add("shop_checkout_address_ec", { | |||
], | |||
}); | |||
|
|||
// TODO: Duplicate: Remove it in Master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they forget remove this file ?
@@ -307,7 +304,7 @@ def _find_existing_company(self, email): | |||
if partner_iap: | |||
return partner_iap.partner_id.sudo(False) | |||
|
|||
return request.env["res.partner"].search([("is_company", "=", True), ("email_normalized", "=ilike", "%" + search)], limit=1) | |||
return request.env["res.partner"].search([('parent_id', '=', False), ("vat", "!=", False), ("email_normalized", "=ilike", "%" + search)], limit=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return request.env["res.partner"].search([('parent_id', '=', False), ("vat", "!=", False), ("email_normalized", "=ilike", "%" + search)], limit=1) | |
return request.env["res.partner"].search([("parent_id", "=", False), ("vat", "!=", False), ("email_normalized", "=ilike", "%" + search)], limit=1) |
Should we rely on both parent_id
and vat
?
I think vat
part is enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will be some cases where VAT is set but there will also be Parent ID.. as we're finding company here having parent_id False will ensure it's a company(in most cases) without a parent and has VAT.
if values.get('parent_name') and not partner.parent_id: | ||
# Create parent company if we got 'parent_name' | ||
parent_values = dict(name=values.get('parent_name'), vat=partner.vat) | ||
parent_values.update(self._convert_fields_to_values(self._address_fields())) | ||
parent_company = self.create(parent_values) | ||
# Set new company as parent | ||
partner.write({ | ||
'parent_id': parent_company.id, | ||
'child_ids': [Command.update(partner_id, dict(parent_id=parent_company.id)) for partner_id in partner.child_ids.ids] | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parent_name
is related to parent_id (non store field and readonly=True)
we should not use to create parent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made it stored and readonly=False field.
It will act in replacement of company_name
. it will be not editable anywhere, but in cases like website form submit, we have a company input field, if user fill it, a company will be created and set as its parent.
Normally it wont affect related relation AFAIK.
PS: It's also used in crm
2a54909
to
ba2b5a5
Compare
3446344
to
b3fc3cb
Compare
c136e89
to
239bb89
Compare
Adapt the util tests. COM PR: odoo/odoo#211043 ENT PR: odoo/enterprise#86089 Upgrade: odoo/upgrade#7724
239bb89
to
f304cbb
Compare
635d680
to
9f5b2fd
Compare
9f5b2fd
to
9539902
Compare
Adapt the util tests. COM PR: odoo/odoo#211043 ENT PR: odoo/enterprise#86089 Upgrade: odoo/upgrade#7724
7800681
to
84b09bc
Compare
Introduced a computed boolean field 'l10n_ar_is_company' to determine if a partner is a legal entity in Argentina, based on the AFIP identification type and CUIT prefix. This replaces reliance on the deprecated 'is_company' field, in line with contact model simplification. Logic: - AFIP code = '80' (CUIT) - CUIT prefix in ('30', '33', '34', '51', '55') → considered a company Task-4714892
We'll now depend on 'l10n_sa_edi_additional_identification_scheme' field to distinguish between a person and a company instead of 'is_company'. Task-4714892
Turkish electronic invoicing now uses VAT length to determine the schemeID: - 10-digit VAT → company (VKN) - 11-digit VAT → individual (TCKN) This replaces previous reliance on 'is_company', which has been removed as part of the contact model simplification. Task-4714892
Introduced '_l10n_es_is_legal_entity' to determine if a Spanish VAT number represents a legal entity based on CIF structure: - Format: 1 letter + 8 digits (e.g., A12345678) This replaces the use of 'is_company, aligning with the contact model simplification. Task-4714892
Introduced a computed field 'l10n_hu_is_company' to determine if a partner is a company in Hungary, based on VAT structure. Logic considers a partner a company if: - VAT starts with 'HU' and is 10 digits, or - Matches the Hungarian EU format: 8 digits - [2/4/5] - 2 digits Task-4714892
Updates demo data and tests across various l10n_* modules to reflect the removal of the 'is_company' field from the contact model. Task-4714892
This commit removes the 'company_type' field, which was previously used to distinguish between 'Person' and 'Company' contacts. The goal is to simplify the contact form and model by eliminating this distinction, as part of an effort to streamline and simplify the contact information structure. This change is in line with the broader initiative to remove legacy fields and make the model more straightforward as possible. It had a good run — well, not really — but it's dead for good now. Removed fields: base : is_company, company_type, commercial_company_name, company_name crm : commercial_partner_id survey : certifications_company_count website_slides : slide_channel_company_count Task-4714892
84b09bc
to
b89a968
Compare
This PR removes the
company_type
field, which was previously used to distinguishbetween 'Person' and 'Company' contacts.
The goal is to simplify the contact form and model by eliminating this distinction, as part
of an effort to streamline and simplify the contact information structure.
Changes in
l10n*
modules:l10n_sa*
: We'll now depend on 'l10n_sa_edi_additional_identification_scheme' field todistinguish between a person and a company instead of 'is_company'.
l10n_tr*
: Turkish electronic invoicing now uses VAT length to determine the schemeID:l10n_es*
: determine if a Spanish VAT number represents a legal entity based onCIF structure:
1 letter + 8 digits (e.g., A12345678)
→ legal entityl10n_ar*
: determine a partner is a legal entity based on the AFIP identification typeand CUIT prefix.
l10n_hu*
: determine if a partner is a company based on VAT structure:l10n_*
: general : removedis_company
/company_type
from demo/testsThis change is in line with the broader initiative to remove legacy fields and make the model
more straightforward as possible.
It had a good run — well, not really — but it's dead for good now.
Removed fields:
base :
is_company
,company_type
,commercial_company_name
,company_name
crm :
commercial_partner_id
survey :
certifications_company_count
website_slides :
slide_channel_company_count
Task-4714892