Skip to content

[19.0][MIG] mail_restrict_follower_selection: Migration to 19.0#183

Open
CRogos wants to merge 36 commits into
OCA:19.0from
c4a8-odoo:copilot/migrate-mail-restrict-follower-selection
Open

[19.0][MIG] mail_restrict_follower_selection: Migration to 19.0#183
CRogos wants to merge 36 commits into
OCA:19.0from
c4a8-odoo:copilot/migrate-mail-restrict-follower-selection

Conversation

@CRogos
Copy link
Copy Markdown
Contributor

@CRogos CRogos commented Apr 16, 2026

supersede: #150

  • Fix pre-commit C8113: move mail_followers_edit.py from models/ to wizard/ directory
  • Fix test failures round 1: replace env.ref() with set_param() in tests
  • Fix safe_eval() API change: locals_dict= → positional context dict in Odoo 19.0

hbrunn and others added 30 commits April 16, 2026 15:27
Currently translated at 100.0% (3 of 3 strings)

Translation: social-11.0/social-11.0-mail_restrict_follower_selection
Translate-URL: https://translation.odoo-community.org/projects/social-11-0/social-11-0-mail_restrict_follower_selection/fr/
Currently translated at 100.0% (3 of 3 strings)

Translation: social-11.0/social-11.0-mail_restrict_follower_selection
Translate-URL: https://translation.odoo-community.org/projects/social-11-0/social-11-0-mail_restrict_follower_selection/de/
Currently translated at 75.0% (3 of 4 strings)

Translation: social-13.0/social-13.0-mail_restrict_follower_selection
Translate-URL: https://translation.odoo-community.org/projects/social-13-0/social-13-0-mail_restrict_follower_selection/it/
When creating a record from a record from another model, the model is not in the context (`default_res_model` key). For example: creating an invoice from a sale order.
Currently translated at 100.0% (7 of 7 strings)

Translation: social-14.0/social-14.0-mail_restrict_follower_selection
Translate-URL: https://translation.odoo-community.org/projects/social-14-0/social-14-0-mail_restrict_follower_selection/it/
1. Use forecreate=false in mail_restrict_follower_selection/data/ir_config_parameter.xml so that a deleted configuration is not recreated by a module update
2. Change the default domain to something less agressive
3. Add an example restriction restricting following a specific model
Currently translated at 100.0% (4 of 4 strings)

Translation: social-16.0/social-16.0-mail_restrict_follower_selection
Translate-URL: https://translation.odoo-community.org/projects/social-16-0/social-16-0-mail_restrict_follower_selection/it/
With this is change it is now possible to use the `ref` function
in the domain set in a parameter, allowing to include xmlids
in the domain.
@CRogos CRogos force-pushed the copilot/migrate-mail-restrict-follower-selection branch 2 times, most recently from aac986c to 7a76706 Compare April 17, 2026 07:58
@CRogos CRogos marked this pull request as ready for review April 17, 2026 07:58
Copy link
Copy Markdown

@MohamedOsman7 MohamedOsman7 left a comment

Choose a reason for hiding this comment

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

LGTM

@MohamedOsman7
Copy link
Copy Markdown

@trisdoan can you please review this PR?

@MohamedOsman7 MohamedOsman7 mentioned this pull request May 18, 2026
34 tasks
@MohamedOsman7
Copy link
Copy Markdown

@BhaveshHeliconia could you please make a review?

Comment thread mail_restrict_follower_selection/wizard/mail_followers_edit.py Outdated
[("id", "in", wizard.partner_ids.ids)]
+ safe_eval(
domain_str,
{"ref": lambda str_id: _id_get(self.env, str_id)},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had to ask the AI myself:

That lambda injects a ref() helper into safe_eval so domain expressions can resolve XML IDs safely at runtime.

In this code, safe_eval(str(domain), {"ref": lambda str_id: _id_get(self.env, str_id)}) means: when the evaluated domain contains something like ref("module.external_id"), call _id_get(self.env, str_id) and return the numeric database ID of that record. The helper itself is defined in utils.py and simply does env.ref(id_str).id, so it deliberately returns only the ID, not the recordset.

So I assume this enables you to use ref in the domain: e.g. [('industry_id','=',ref('base.res_partner_industry_A'))]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've verified on runboat and it gives some error. Could you please check on the runboat with this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've fixed it and also improved the wizard domain filter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any way to avoid overwriting onClickAddFollowers? If not, then please add a comment explaining why we need overwriting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are totally right. How about this solution, by intercepting the doAction call? (I have to say that I am not an JS expert, but I like this solution from copilot)

Comment thread mail_restrict_follower_selection/tests/test_mail_restrict_follower_selection.py Outdated
Comment thread mail_restrict_follower_selection/tests/test_mail_restrict_follower_selection.py Outdated
Comment thread mail_restrict_follower_selection/__init__.py Outdated
@@ -0,0 +1,7 @@
<?xml version="1.0" ?>
<odoo>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<odoo>
<odoo noupdate="1">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one is still pending.

Copy link
Copy Markdown
Contributor Author

@CRogos CRogos May 21, 2026

Choose a reason for hiding this comment

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

This is only demo data and not a real configuration. Not sure if we should flag this as noupdate.... OK I had a look into other demo data, you are right.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Can you replace setup() with following code snippet?
onClickAddFollowers() {
        const originalDoAction = this.action.doAction.bind(this.action);
        const saved = this.action;
        this.action = Object.assign(Object.create(this.action), {
            // The web client filters the loadViews context down to `lang` and
            // keys ending with `_view_ref`. We smuggle the followed record's
            // model through under a `_view_ref`-suffixed key so the wizard's
            // `get_view` can apply the per-model follower domain.
            doAction: (action, options) =>
                originalDoAction(
                    {
                        ...action,
                        context: {
                            ...action.context,
                            restrict_follower_res_model_view_ref:
                                this.props.thread.model,
                        },
                    },
                    options
                ),
        });
        try {
            return super.onClickAddFollowers();
        } finally {
            this.action = saved;
        }
    },
  • By this, we can narrow scope from setup() to onClickAddFollowers() and also make use of arrow function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've changed it even though I am not sure if this is a better solution. But maybe it is easier to understand because it is better linked to the reason of this change.

@CRogos CRogos force-pushed the copilot/migrate-mail-restrict-follower-selection branch from 7a76706 to 2f3b290 Compare May 20, 2026 16:46
Comment thread mail_restrict_follower_selection/wizard/mail_followers_edit.py Outdated
@CRogos CRogos force-pushed the copilot/migrate-mail-restrict-follower-selection branch 6 times, most recently from 0323a01 to db78d05 Compare May 22, 2026 13:09
@CRogos CRogos force-pushed the copilot/migrate-mail-restrict-follower-selection branch from db78d05 to cc46022 Compare May 26, 2026 07:48
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.

Code and functional review LGTM!

@OCA-git-bot
Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@CRogos
Copy link
Copy Markdown
Contributor Author

CRogos commented May 26, 2026

@pedrobaeza could you merge?

@pedrobaeza
Copy link
Copy Markdown
Member

/ocabot migration mail_restrict_follower_selection
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 19.0 milestone May 26, 2026
@OCA-git-bot
Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 19.0-ocabot-merge-pr-183-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Copy Markdown
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 19.0-ocabot-merge-pr-183-by-pedrobaeza-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

1 similar comment
@OCA-git-bot
Copy link
Copy Markdown
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 19.0-ocabot-merge-pr-183-by-pedrobaeza-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

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.