[16.0][ADD] sale_partner_sale_contact[_on_project]: Add sale contact to orders, invoices and projects

[maisim]

New modules: Sale Contact Management

This PR introduces two complementary modules for managing sale contacts across the sales workflow:

sale_partner_sale_contact

• Adds a sale_contact_partner_id field to sales orders and invoices
• Allows selecting a specific contact person from the customer's organization
• Automatically propagates the contact from order to invoice
• Optional display on PDF reports (configurable in settings)
• Auto-switches customer to parent company when a contact person is selected

sale_partner_sale_contact_on_project

• Extends the contact management to projects
• Automatically copies the sale contact when creating projects from sales orders
• Maintains contact information throughout the project lifecycle

Use case

In B2B scenarios, you often deal with specific contacts within a customer organization (procurement managers, project leads, etc.). While the invoice must go to the company, you need to track who you're actually working with for follow-ups, communication, and relationship management.

Design choices

• Uses child contact hierarchy (existing Odoo pattern)
• Auto-switch behavior prevents accidental invoicing to individuals while preserving contact info
• Optional restriction modules available via settings for stricter enforcement
• No modification of core partner/invoice logic - purely additive

Benefits

• Better contact tracking across sales → invoice → project workflow
• Cleaner reports with relevant contact information
• Improve global consistncy and avoid to get stuffs like Analytic Account linked to a person
• Respects commercial entity relationships while maintaining operational contacts
• All features are backward compatible and don't affect existing workflows. 🤞

Related PRs

You can optionally use this module to limit partner_id selection to companies

• account_invoice_partner_id_company_only: Restrict partner to companies [16.0][ADD] account_invoice_partner_company_only: Restrict partner… account-invoicing#2180
• project_partner_id_company_only: Restrict partner to company [16.0][ADD] project_partner_company_only: Restrict partner to company project#1611
• sale_partner_id_company_only: Restrict partner to companies [16.0][ADD] sale_order_partner_company_only: Restrict partner to companies #4042

[rrebollo]

@maisim are you sure we can achieve similar features with built-in?

[maisim]

@maisim are you sure we can achieve similar features with built-in?

I looked, I didn't found anything

[rousseldenis]

@maisim As @rrebollo said, you can use the contact in standard.

Here, I've just add an invoice address if different (tested in runbot):

[alexey-pelykh]

Thanks for this contribution, Simon. Useful B2B use case. I went through both modules in detail and have feedback on a few areas.

Functional overlap question

I see @rousseldenis and @rrebollo already raised this -- standard Odoo 16 does have invoice/delivery/contact addresses on partners, and the SO form shows invoice and delivery address fields. However, I understand this module targets a different concept: a named commercial contact person (procurement manager, project lead) that travels across SO → invoice → project, distinct from the billing/shipping addresses. If that understanding is correct, it would help to explain that distinction explicitly in the module description to avoid future confusion.

Code-level findings

1. Duplicated onchange logic across 3 models

The _onchange_partner_id_clear_sale_contact method is copy-pasted identically in sale.order, account.move, and project.project. This is a maintenance burden -- any bug fix or behavior change would need to be applied in three places.

Consider extracting this into a mixin model (e.g. sale.contact.mixin) that all three inherit. The field definition and onchange would live in one place.

2. Domain uses child_of but onchange checks parent_id

The field domain uses ('id', 'child_of', partner_id) which includes all descendants (children, grandchildren, etc.), but the onchange validation only checks self.sale_contact_partner_id.parent_id != self.partner_id -- this would incorrectly clear a valid grandchild contact. The validation should match the domain, e.g. using commercial_partner_id or walking up the parent chain.

3. Auto-switch onchange may conflict with standard onchange_partner_id

In sale.order, the standard onchange_partner_id already fires when partner_id changes and updates fiscal position, pricelist, payment terms, etc. Your onchange modifies partner_id during an onchange on partner_id, which can cause infinite recursion or unpredictable sequencing depending on the Odoo version. This is fragile -- the auto-switch from contact → company should probably be handled differently, perhaps via an @api.onchange('partner_id') with a guard flag, or by overriding the standard onchange_partner_id instead of adding a second one.

Same concern applies to account.move where the standard _onchange_partner_id is already complex.

4. Report templates use different HTML elements

The sale order report uses a <p> tag while the invoice report uses a <div> tag for the same content block. Should be consistent.

5. module_* fields in res.config.settings reference non-existent modules

The settings form includes checkboxes for module_sale_partner_id_company_only, module_account_invoice_partner_id_company_only, and module_project_partner_id_company_only. These reference modules from other PRs that aren't merged yet (and project_partner_id_company_only is in a completely different repo). Including auto-install triggers for modules that don't exist in this repo's dependency tree is risky -- if a user toggles the checkbox, Odoo will try to install a module that may not be available, resulting in an error. These should either be removed from this PR or the related modules should be merged first.

6. Missing copy=False consideration

When duplicating a sale order or invoice, sale_contact_partner_id will be copied by default. This might be desired, but it's worth being explicit about the choice -- add copy=True (to document intent) or copy=False if the contact should not carry over to duplicates.

7. account.move view uses attrs for readonly instead of states

The invoice view uses attrs="{'readonly': [('state', '!=', 'draft')]}" but the sale order view does not have any readonly condition. Both should be consistent. For the SO view, you'd typically want readonly in locked/done states too.

Test coverage

The tests are solid for the happy path. A couple of gaps:

• No test for the auto-switch behavior (selecting a contact person as partner → auto-switch to company)
• No test for the _prepare_invoice propagation when sale_contact_partner_id is False (the if guard -- the test test_06 covers this indirectly but doesn't assert the dict key is absent)
• The codecov/patch check is failing, so some lines aren't covered

OCA conventions

• Missing i18n/*.pot export via oca-gen-addon-readme (the .pot files look hand-crafted rather than exported)
• The copyright header says "Odoo Community Association (OCA)" but the author is "OpenStudio SAS" -- for OCA contributions, copyright typically belongs to the contributing company. The copyright line in source files should match the author in manifest.

Overall a useful feature, but the duplicated onchange logic and the domain/validation mismatch should be addressed before merge.

[JordiBForgeFlow]

@maisim Can you change the module to LGPL? Being a quite low level feature can be benefitial to both CE and EE users.

[maisim]

@maisim Can you change the module to LGPL? Being a quite low level feature can be benefitial to both CE and EE users.

No worries, I'll do it right away

[maisim]

Hi @alexey-pelykh,
Thank you so much for your feedback; I've started making the changes accordingly. I'll commit each point individually, which will make reviewing easier. If necessary, I'll then merge them into a single commit per module, as is currently the case.

[MiquelRForgeFlow]

By default, this already has copy=True.

[maisim]

Hi @MiquelRForgeFlow,
It's a request from @alexey-pelykh be be explicit on the behavior

[MiquelRForgeFlow]

Well, you can put it in the mixin instead :S

[maisim]

Oh yes sorry

[MiquelRForgeFlow]

same as previous comment

[maisim]

Same as above

[MiquelRForgeFlow]

By the way, the point of the mixing is avoid declaring this field everywhere, but just to customize this help, you are redeclaring it again anyway, so the mixin loses part of its purpose.

[MiquelRForgeFlow]

The help in the mixin could be: Contact person. Only child contacts of the customer can be selected., this way you can avoid customizing for each model.

[maisim]

So it is better to just remove the field here?
I readd it here because I had a db error, the orm try to recreate the field, but I think I change the way I call the mixin between, let me do some tests to double check

[maisim]

Seems to be ok, let me do some functional tests from the ui

[maisim]

The last changes broke the "auto switch" feature from the UI.
I first tried to re-add @api.onchange decorator but it seems to brake the MRO.
I had to use a new method name (instead of override _onchange_partner_id) , then remove the super()
Both onchange are now called on partner_id change.
It works.
Do you think this is the good way to do that?

[maisim]

5. module_* fields in res.config.settings reference non-existent modules

Any change to get concerned module merged instead of remove the refs?

[MiquelRForgeFlow]

Repo was greenified in #4230 so please rebase. Also, don't add last commit, thus modules are not dependencies. Also, I think it would be better to squash commits, there are too many.

[maisim]

Repo was greenified in #4230 so please rebase. Also, don't add last commit, thus modules are not dependencies.

It was to ease functionnal test of modules installation from the settings. I will remove it

Also, I think it would be better to squash commits, there are too many.

I did it it seprate commits to get one per asked modification. I will squash them in one commit per addon

[MiquelRForgeFlow]

I think this have to be removed, this should be added when merging the corresponding PRs.

[maisim]

There, it's done.

I took a quick look and don't see any other references to the additional modules in the commits.

I think it's not the right approach anyway. It's more of a configuration issue and doesn't really make sense, as @rrebollo kindly pointed out to me. OCA/account-invoicing#2180

Either we do without it here and let everyone override their views to limit the display to companies, or I'll look into integrating it later as a configuration option in this module.
What do you think?