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

[17.0][OU-ADD] account: Migration to 17.0 #4668

Merged
merged 5 commits into from
Feb 3, 2025
Merged

Conversation

hbrunn
Copy link
Member

@hbrunn hbrunn commented Nov 18, 2024

continuation of #4662

I tested this with my own DB, a few customers', and the demo database and couldn't find issues any more, so from my side I think it's done.

@hbrunn
Copy link
Member Author

hbrunn commented Nov 18, 2024

/ocabot migration account

@hbrunn
Copy link
Member Author

hbrunn commented Nov 18, 2024

tests are fixed in #4669, which also actually makes the files from the test folder run

@hbrunn
Copy link
Member Author

hbrunn commented Nov 18, 2024

/ocabot rebase

@hbrunn hbrunn force-pushed the 17.0-account branch 3 times, most recently from d16d0b3 to 2e69b80 Compare November 18, 2024 22:27
Copy link
Member

@zamberjo zamberjo left a comment

Choose a reason for hiding this comment

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

Tested and so far seems OK

Copy link

@celm1990 celm1990 left a comment

Choose a reason for hiding this comment

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

I tested it, and it's working fine. This PR is blocking several others. Could it be prioritized? @pedrobaeza

Comment on lines +282 to +291
def convert_from_company_dependent(
env,
model_name,
origin_field_name,
destination_field_name,
origin_id_column_name,
model_table_name=None,
):
"""
Move a company-dependent field back to the model table.

Choose a reason for hiding this comment

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

I think it could be useful to move this function to openupgradelib to improve reusability. It could serve as a base utility for scenarios requiring the conversion of property fields to JSON during migrations to version 18.

Copy link
Member Author

Choose a reason for hiding this comment

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

that's a very different logic, you'll find the v18 function here

By now I've seen most of the migrations for v17 and didn't see another module where the case we have in the account module happens.

@OCA-git-bot
Copy link
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). 🤖

@hbrunn
Copy link
Member Author

hbrunn commented Feb 3, 2025

thanks @MiquelRForgeFlow for the review

Copy link
Contributor

@MiquelRForgeFlow MiquelRForgeFlow left a comment

Choose a reason for hiding this comment

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

Amazing, thanks. Would you like to make some commit squash before merge?

@hbrunn
Copy link
Member Author

hbrunn commented Feb 3, 2025

I squashed what I could without co-opting other people's work

@MiquelRForgeFlow
Copy link
Contributor

Test failed :S

@hbrunn
Copy link
Member Author

hbrunn commented Feb 3, 2025

rerunning them helped

@MiquelRForgeFlow MiquelRForgeFlow merged commit ba794c9 into 17.0 Feb 3, 2025
5 checks passed
@OCA-git-bot OCA-git-bot deleted the 17.0-account branch February 3, 2025 17:14
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.

8 participants