Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] convert_from_company_dependent #390

Closed

Conversation

hbrunn
Copy link
Member

@hbrunn hbrunn commented Oct 28, 2024

this is the function I propose in OCA/OpenUpgrade#4457 (comment), will be useful elsewhere too

@pedrobaeza
Copy link
Member

But I think this depends on the business logic behind, as you are converting a multi-valued field (potentially one per company) into only one value, so you need to decide what to do in case of conflict depending on the use case.

@hbrunn
Copy link
Member Author

hbrunn commented Oct 28, 2024

I think the general case will be pretty much the same everywhere:

v(n-1): model has no company_id and property property_whatever
v(n): model has company_id, property_whatever becomes field whatever

Then the user of the function duplicates every row of model for each company where it makes sense (that's the business logic depending part), and calls convert_from_company_dependent(model, property_whatever, whatever), which is invariant wrt business logic

@pedrobaeza
Copy link
Member

But in such duplication, res_id will be another, and thus, this method won't serve for transferring the value of the corresponding property for the duplicated records.

@hbrunn
Copy link
Member Author

hbrunn commented Oct 29, 2024

it will, because the caller sets company_id for the cloned records as it's appropriate.

consider the account migration, there they duplicate tax groups as it makes sense into the different companies, and in the next function I can replace the horrible incomplete sql statements with three calls to the function I propose.

Wherever Odoo decides to go from properties to company aware on model level, we need the exact same thing.

FROM ir_property ip
WHERE ip.fields_id={field_id} --- {origin_field_name}
AND (
ip.res_id = '{model_name}.' || {table_name}.id
Copy link
Member

Choose a reason for hiding this comment

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

I insist that this is the problem. The duplicated record has an ID that is not on ir_property table, as the corresponding one is the previous record ID.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for insisting, see below

@hbrunn
Copy link
Member Author

hbrunn commented Oct 29, 2024

aaah wait, I get it now. This indeed needs the extra field I propose on the PR to link the clones to their origin record. That's a good thing to have in those cases anyways, will do next week

@hbrunn
Copy link
Member Author

hbrunn commented Nov 4, 2024

I've just grepped through the other analysis files and didn't find another place where we need this for the v17 migration, so for the time being I put the function in the v17 account migration, and we move it to openupgradelib when the next similar case comes up

@hbrunn hbrunn closed this Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants