[FIX] account: Migrate security groups

[yajo]

In v10:

• group_account_user implied group_account_invoice
• group_account_manager implied group_account_user

In v11:

• group_account_user implied no other groups
• group_account_manager implied group_account_invoice

These one2many changes are not automigrated, so here's the script.

@Tecnativa TT23168

[pedrobaeza]

Aren't this noupdate data?

[pedrobaeza]

If so, please use the usual method through noupdate_records.xml file.

[yajo]

No, it is updatable, just check the links.

[yajo]

This affects also stock_account, should I make a separate PR or can I push that here too?

[pedrobaeza]

OK, then the problem seems to be that no field means no deletion of previous one. That is getting into a common problem. @MiquelRForgeFlow how did you solve this previously? I think your solution was to make a PR to Odoo, isn't it? Maybe we can perform this check also on the noupdate script.

@yajo put it here if related.

[yajo]

Oh it seems the stock_account problem is in v8: #2261

[MiquelRForgeFlow]

how did you solve this previously?

where? which previous case are you thinking?

perform this check also on the noupdate script.

but this case is unrelated to noupdate script because that is updatable data

[pedrobaeza]

@MiquelRForgeFlow I remember some missing context lines in XML data leads to incorrect action windows or similar in 11.0/12.0. Don't you remember?

I know that is not noupdate cases, but we can use the same engine to detect them and prepare noupdate_records.xml files that handle these cases (maybe we can rename then to data_to_update.xml).

[MiquelRForgeFlow]

so, you mean, doing a PR to Odoo like:

    <record id="group_account_user" model="res.groups">
        <...>
+        <field name="implied_ids" eval="[(3, ref('group_account_invoice'))]"/>
    </record>

    <record id="group_account_manager" model="res.groups">
        <...>
-        <field name="implied_ids" eval="[(4, ref('group_account_invoice'))]"/>
+        <field name="implied_ids" eval="[(3, ref('group_account_user')),(4, ref('group_account_invoice'))]"/>
    </record>

then, yeah, it's doable, but I don't know if they will accept.

[yajo]

For v11 maybe... But that wouldn't fix it here because OU is out of sync with Odoo, right?

[MiquelRForgeFlow]

@yajo your fix here makes sense. If done PR to Odoo, then when odoo accepts it, then we can update upstream again and delete your patch here. Don't worry.

[MiquelRForgeFlow]

I think odoo won't accept that kind of PR because they can say "that's updatable data, so if somebody customize those two groups (i.e., by adding group_account_invoice to group_account_user and group_account_user to group_account_manager) and then they do an update of account module (with the PR), then this two groups modifications will be reseted"

[pedrobaeza]

Although I agree with the change, I think we should provide continuity with the expected permissions: if a user in v10 with one permission have access to certain parts, on v11 it should be the same without changing anything. What this means? That you have to complement this with a direct assignation of the previous group to all the users that were in the other. Do you understand me?

[pedrobaeza]

@yajo please check my last comment

[yajo]

I understand, but it's not going to be fast or easy to find out that. I think we should merge this now, as it doesn't conflict with that and it's fixing a bug experienced in a real world scenario, and improve that later when we face it.

[pedrobaeza]

I have just checked and groups in users are preserved although you change the groups inheritance, so it's good to go