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

[18.0][MIG] l10n_fr_siret + add glue module l10n_fr_siret_account #584

Merged
merged 102 commits into from
Dec 20, 2024

Conversation

alexis-via
Copy link
Contributor

Since v18, l10n_fr doesn't depend on account (only on base) and there is a glue module l10n_fr_account.
So, we should follow this movement in OCA and have l10n_fr_siret and l10n_fr_siret_account
Fixes #38 which was opened 9 years ago !!!
Small improvements: add siren/siret/nic in list view as optional=hide

This migration PR is an alternative to #578

Comment on lines +11 to +12
siren = fields.Char(tracking=50)
nic = fields.Char(tracking=51)
Copy link
Contributor

@legalsylvain legalsylvain Nov 26, 2024

Choose a reason for hiding this comment

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

I don't get why you redefine this two fields on the glue module. Why not in base module ?
also I'm not very familiar with this new tracking values. could you elaborate ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tracking is a feature of the mail module, not base. l10n_fr_siret_account depend on account which depend on mail.
For example, you will not see tracking=X on res.partner fields defined in the base module... they are in the mail module: https://github.com/odoo/odoo/blob/18.0/addons/mail/models/res_partner.py#L19

@legalsylvain
Copy link
Contributor

Since v18, l10n_fr doesn't depend on account (only on base) and there is a glue module l10n_fr_account.

Very nice modular move.

So, we should follow this movement in OCA and have l10n_fr_siret and l10n_fr_siret_account

+1, indeed.

alexis-via and others added 21 commits December 12, 2024 18:00
Currently translated at 100.0% (23 of 23 strings)

Translation: l10n-france-16.0/l10n-france-16.0-l10n_fr_siret
Translate-URL: https://translation.odoo-community.org/projects/l10n-france-16-0/l10n-france-16-0-l10n_fr_siret/fr/
Currently translated at 100.0% (23 of 23 strings)

Translation: l10n-france-16.0/l10n-france-16.0-l10n_fr_siret
Translate-URL: https://translation.odoo-community.org/projects/l10n-france-16-0/l10n-france-16-0-l10n_fr_siret/es/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: l10n-france-16.0/l10n-france-16.0-l10n_fr_siret
Translate-URL: https://translation.odoo-community.org/projects/l10n-france-16-0/l10n-france-16-0-l10n_fr_siret/
Currently translated at 100.0% (21 of 21 strings)

Translation: l10n-france-16.0/l10n-france-16.0-l10n_fr_siret
Translate-URL: https://translation.odoo-community.org/projects/l10n-france-16-0/l10n-france-16-0-l10n_fr_siret/es/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: l10n-france-17.0/l10n-france-17.0-l10n_fr_siret
Translate-URL: https://translation.odoo-community.org/projects/l10n-france-17-0/l10n-france-17-0-l10n_fr_siret/
Fixes OCA#38  l10n_fr_siret: unrequired dependency on account
This is now possible because l10n_fr doesn't depend on account anymore
and there is a new official module l10n_fr_account that acts as a glue
module between l10n_fr and account.
@alexis-via alexis-via force-pushed the 18-mig-siret_no_dep_accoun branch from 7c715e3 to b19448f Compare December 12, 2024 17:01
siren = fields.Char(
string="SIREN", related="partner_id.siren", store=True, readonly=False
)
nic = fields.Char(
Copy link

Choose a reason for hiding this comment

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

Presuming the nic here seems to be problematic.

The basic problem is that we've only been able to structure the res_partner companies (unique SIREN) by having the partner marked 'company' having the SIREN, but with a null NIC giving a SIRET as *****, adding for each establishment a child partner with it's NIC set correctly.

For the case of the HQ (siège), setting the partner type 'invoice' as well as setting it as the default invoiced partner for the company (for other partners, customer or suppliers, it may not be exactly that way).

What happens here with a blank nic is that it'll probably break the SIRET in a number of reports.
Unfortunately, it is seemingly not possible to non-programatically change the partner_id on the company.

Perhaps what is needed on the res_partner company is a HQ link to the child partner currently registered as HQ (siège), which link could be followed to make res_company happy... or at least so if NIC is null.

BTW, when the company moves [the HQ], it is straight-forward that the company partner is the only place that these details should be managed (creation of establishments, archival of closed establishments, et cetera).

Last but not least, the SIRET has a opening date, and at some point in time a possible closing date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My advice is:

  • on the parent partner: address of the HQ (siège social) with siren + NIC of the HQ
  • one child partner with type = other/delivery/invoice for each secondary establishment with a different NIC value

Copy link

Choose a reason for hiding this comment

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

This really doesn't work stably. It may be ok with a single permanent registered address for a company but not if the company moves or has secondary addresses.
The way I mentioned is the only way we've found to ensure integrity concerning the auxiliary accounts, not to mention references to any other address usage.
The model was much cleaner when there was still a res.partner.address, because SIREN==res.partner and SIRET==res.partner.address

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just like you, I prefered the previous datamodel with res.partner and res.partner.address !!! :)

@alexis-via
Copy link
Contributor Author

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 18.0-ocabot-merge-pr-584-by-alexis-via-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 9a84a8f into OCA:18.0 Dec 20, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at c33cd8f. Thanks a lot for contributing to OCA. ❤️

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.