Skip to content

[17.0] [MIG] stock_picking_tier_validation: migration to v17 #1962

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

Open
wants to merge 10 commits into
base: 17.0
Choose a base branch
from

Conversation

rrebollo
Copy link
Contributor

@rrebollo rrebollo commented May 27, 2025

Standard migration from v14 to v17.

Key Points

The tests were failing due to certain methods in base_tier_validation using raw SQL queries. Since test transactions are not committed to the database, these raw queries were unable to retrieve the expected data, resulting in failed assertions.

To resolve this, we refactored the tests by moving the scenario setup logic to the setUpClass method, which runs in the main transaction and ensures the data is "committed". The actual test methods now run in sub-transactions, allowing raw SQL queries to access the prepared data correctly.

With this approach, the tests now pass as expected.

#1421
@BinhexTeam

Lots of credit to #1875

@rrebollo rrebollo force-pushed the 17.0-mig-stock_picking_tier_validation branch 3 times, most recently from e12f370 to fa1979b Compare May 27, 2025 04:55
@rrebollo rrebollo force-pushed the 17.0-mig-stock_picking_tier_validation branch from fa1979b to b001590 Compare May 29, 2025 13:48
@rrebollo rrebollo marked this pull request as ready for review May 29, 2025 13:53
@rrebollo
Copy link
Contributor Author

ping @Christian-RB

Copy link

@Christian-RB Christian-RB left a comment

Choose a reason for hiding this comment

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

Code and functional review Ok!

Copy link

@edescalona edescalona left a comment

Choose a reason for hiding this comment

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

LGTM

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

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.

6 participants