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

[FIX] FakeModelLoader: avoid loading module again on restore_registry() for version < 15 #28

Merged
merged 2 commits into from
Oct 17, 2023

Conversation

nilshamerlinck
Copy link
Contributor

@nilshamerlinck nilshamerlinck commented Oct 10, 2023

  • FakeModelLoader currently loads the current module on restore_registry()
  • Comment says "# reload is need to reset correctly the field on the record": based on some tests, this was indeed needed for Odoo up to 14.0
  • However, it creates trouble in post_install tests, see cases:
  • So this PR:
    • changes the behavior of reload_registry() to avoid loading the module again for Odoo 15.0+
    • introduces a test case for odoo 15.0+ to make expectations explicit here
  • This will allow to remove the hacky workarounds from the PRs above
  • While not breaking anything for older versions

@nilshamerlinck nilshamerlinck marked this pull request as draft October 10, 2023 06:47
@nilshamerlinck nilshamerlinck changed the title [TMP] please ignore [FIX] FakeModelLoader: avoid loading module again on restore_registry() Oct 10, 2023
@nilshamerlinck nilshamerlinck force-pushed the fix_loading branch 7 times, most recently from f6ea239 to 674d13a Compare October 12, 2023 09:43
@nilshamerlinck nilshamerlinck marked this pull request as ready for review October 13, 2023 06:26
@simahawk simahawk changed the title [FIX] FakeModelLoader: avoid loading module again on restore_registry() [FIX] FakeModelLoader: avoid loading module again on restore_registry() for version < 15 Oct 13, 2023
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

Seems legit.

@sebastienbeau @lmignon could you have a look when you have time? 🙏

@OCA-git-bot
Copy link
Collaborator

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). 🤖

@simahawk
Copy link
Contributor

@nilshamerlinck can you rebase? I'm going to merge a prepare a release

@nilshamerlinck
Copy link
Contributor Author

Hi @simahawk nice, done

@simahawk simahawk merged commit 34c6c5f into OCA:master Oct 17, 2023
5 checks passed
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.

4 participants