Skip to content

[19.0][IMP] partner_email_duplicate_warn: access-aware duplicate banner#2375

Open
cliffkujala wants to merge 1 commit into
OCA:19.0from
purekarting:19.0-partner_email_duplicate_warn-access-aware
Open

[19.0][IMP] partner_email_duplicate_warn: access-aware duplicate banner#2375
cliffkujala wants to merge 1 commit into
OCA:19.0from
purekarting:19.0-partner_email_duplicate_warn-access-aware

Conversation

@cliffkujala
Copy link
Copy Markdown

Summary

partner_email_duplicate_warn shows an on-change banner listing other partners that share the same email, each rendered as a clickable link. The matching partners are computed with compute_sudo, so the banner could list — and link to — records the current user is not allowed to read. Clicking such a link (or, depending on the view, merely rendering it) raises the standard Odoo AccessError dialog, with no explanation of why the record is off-limits.

This makes the banner access-aware: it links only to duplicates the acting user may actually read, and summarises the rest as a count instead of leaking links to forbidden records.

Changes

  • _compute_same_email_partner_ids now filters the matched partners through the acting user's read access (_filtered_access("read")), so same_email_partner_ids only ever contains readable records.
  • New computed field same_email_inaccessible_count: the number of same-email partners the user cannot read.
  • The banner view shows the readable duplicates as links (unchanged behavior for those) plus, when same_email_inaccessible_count is set, a short message noting that N additional record(s) the user cannot access also use this address.

Behavior notes

  • No AccessError is raised by the banner anymore; unreadable duplicates are surfaced as a count rather than a link.
  • Detection itself is unchanged (still finds all duplicates); only what is disclosed to the user respects record rules / multi-company access.

Tests

Adds test_partner_duplicate_inaccessible: a duplicate hidden from the acting user by a record rule is counted in same_email_inaccessible_count, is not present in same_email_partner_ids, and reading the computed fields raises no AccessError.

The same_email_partner_ids field is computed as superuser (compute_sudo),
so it could include partners the acting user is not allowed to read. The
warning banner renders those records through the x2many_links widget, which
fetches their display_name as the current user, raising a raw AccessError
("you do not have read access") with no explanation when a duplicate is
hidden by record rules.

Only readable duplicates are now exposed as links. Partners with the same
email that the user cannot access are counted in a new
same_email_inaccessible_count field, and the banner shows a dedicated
message inviting the user to contact an administrator, without disclosing
the inaccessible records.
@OCA-git-bot
Copy link
Copy Markdown
Contributor

Hi @alexis-via,
some modules you are maintaining are being modified, check this out!

@OCA-git-bot OCA-git-bot added series:19.0 mod:partner_email_duplicate_warn Module partner_email_duplicate_warn labels May 31, 2026
Copy link
Copy Markdown
Contributor

@BhaveshHeliconia BhaveshHeliconia left a comment

Choose a reason for hiding this comment

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

@cliffkujala Could you guide me no how I can reproduce this issue? I've tried with some ways, but it is not reproducible as of now.

@BhaveshHeliconia
Copy link
Copy Markdown
Contributor

  • While checking this PR [19.0][IMP] partner_email_check: clearer duplicate-email message + multi-company scope option #2374, I reproduced the issue.

  • Test case:

    • Enable companyA
    • Create a contact with company and email set
    • Enable companyB
    • Try to create another contact with company and same email as the previous one
    • Observe 'Access Rights' error
  • This PR addresses this issue.

  • Also, in odoo, same_vat_partner_id and same_company_registry_partner_id fields of res.partner don't have compute_sudo parameter, instead it is handled directly inside compute method(_compute_same_vat_partner_id). So, IMO we should follow the same pattern as odoo follows for these fields.

Copy link
Copy Markdown
Contributor

@BhaveshHeliconia BhaveshHeliconia left a comment

Choose a reason for hiding this comment

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

Functional review LGTM!

@cliffkujala
Copy link
Copy Markdown
Author

Thanks for the review and the approval, @BhaveshHeliconia!

On reproduction: the key trigger is the acting user not having read access to the matching duplicate partner. Multi-company (your companyA/companyB steps) is one clean way to hit it; a record rule that hides the existing partner from the current user (e.g. a salesperson restricted to their own contacts) is another. In either case, before this PR the same-email banner would compute the match as superuser and then render a link to a record the user can't read, raising the raw AccessError dialog with no explanation. With this change the banner links only to readable duplicates and reports the rest as a count.

On the compute_sudo vs. core pattern point: agreed, mirroring how Odoo core handles same_vat_partner_id / same_company_registry_partner_id (no compute_sudo, escalating inside the compute method) is a reasonable and consistent approach. It's a small, behavior-equivalent refactor. Since it touches the privilege model of a maintained module and isn't required for correctness, I'd defer that call to the module maintainer (@alexis-via) — happy to apply it in this PR if preferred, or leave it for a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:partner_email_duplicate_warn Module partner_email_duplicate_warn series:19.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants