[17.0][OU-ADD] base_automation: migration to 17.0

[petrus-v]

attempt to take care some reviews from #4459

• [17.0][OU-ADD] base_automation: migration to 17.0 #4459 (comment) done by 9490f09
• [17.0][OU-ADD] base_automation: migration to 17.0 #4459 (comment) done by 9f394bd
• [17.0][OU-ADD] base_automation: migration to 17.0 #4459 (comment) done by 2dc89db
• [17.0][OU-ADD] base_automation: migration to 17.0 #4459 (comment) not yet : I've only 4 records in my context and not sure what would be the proper sql code setup values as far I understand only create column with Null values should do the trick, any advices welcomes done by 7857148 (thanks Miquel for the help)
• [17.0][OU-ADD] base_automation: migration to 17.0 #4459 (comment) I'm wondering what's wrong moving forwards ? it is to gives more times to sub modules to change code as well and take care new values ? done by 2d3653c

[legalsylvain]

/ocabot migration base_automation

[OCA-git-bot]

The migration issue (#4124) has not been updated to reference the current pull request because a previous pull request (#4459) is not closed.
Perhaps you should check that there is no duplicate work.
CC @daiduongnguyen-odoo

[MiquelRForgeFlow]

I understand only create column with Null values should do the trick, any advices welcomes

_new_columns = [
    ("base.automation", "trg_field_ref", "many2one_reference"),
    ("base.automation", "trg_selection_field_id", "many2one"),
]

openupgrade.add_columns(env, _new_columns)

I'm wondering what's wrong moving forwards ? it is to gives more times to sub modules to change code as well and take care new values ?

I don't know. That's something only @pedrobaeza should answer, as he made that comment.

[MiquelRForgeFlow]

/ocabot migration base_automation

[petrus-v]

@MiquelRForgeFlow thanks for review and advices.

I understand only create column with Null values should do the trick, any advices welcomes

_new_columns = [
    ("base.automation", "trg_field_ref", "many2one_reference"),
    ("base.automation", "trg_selection_field_id", "many2one"),
]

openupgrade.add_columns(env, _new_columns)

Here my question was not how to create those empty fields, but: if I only create those fields leaving them empty is it enough in this specific case ?

As far I understand, generally we do that for performance reason by populate the field using SQL query re-implementing the business python code in SQL. here the python code that compute trg_selection_field_id was not clear to me what it really does:

    @api.depends('trigger', 'trigger_field_ids')
    def _compute_trg_selection_field_id(self):
        to_reset = self.filtered(lambda a: a.trigger not in ['on_priority_set', 'on_state_set'] or len(a.trigger_field_ids) != 1)
        to_reset.trg_selection_field_id = False
        for automation in (self - to_reset):
            # always re-assign to an empty value to make sure we have no discrepencies
            automation.trg_selection_field_id = self.env['ir.model.fields.selection']

as far I understand the value is False or an empty recordset which is also a falsy value or I missed interpret the following instruction ?

>>> self.env['ir.model.fields.selection'].id
False

also when I read the doc

These scripts are about optimization. As a contributor of openupgrade for a module, if you do not have a problem of excessive > duration, you can propose migration scripts without such optimizations, especially when the SQL queries are complex to write.

Another contributor can always propose a PR for performance improvement, if he faces this problem.

And seen only 4 records in my database I was thinking should I take this risks !

[MiquelRForgeFlow]

Here my question was not how to create those empty fields, but: if I only create those fields leaving them empty is it enough in this specific case ?

Yeah, in this specific case, leaving them empty is fine 😊 (it's a new functionality, they are supposed to be filled using the form view)

[MiquelRForgeFlow]

LGTM

[pedrobaeza]

And please reduce the commit history to one commit.

[pedrobaeza]

Well, 2 commits, the other and yours, but don't put so many commits of your authorship.

[petrus-v]

I've squashed commits ! (I kept it in the mean time with the hope to help reviews)