Skip to content

[18.0][MIG] sale_substate: Migration to 18.0 #3581

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.

Sign up for GitHub

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

Jump to bottom
Merged
OCA-git-bot merged 27 commits into from
Jul 23, 2025
Merged

[18.0][MIG] sale_substate: Migration to 18.0 #3581

OCA-git-bot merged 27 commits into from
Jul 23, 2025

Conversation

@Alexgars73
Copy link
Contributor

@Alexgars73 Alexgars73 commented Feb 13, 2025 •
edited by dreispt
Loading

Migration to 18.0

Depends on:

@Alexgars73 Alexgars73 changed the title [18.0][MIG] sale_substate [18.0][MIG] sale_substate: Migration to 18.0 Feb 13, 2025
@Alexgars73 Alexgars73 force-pushed the 18.0-mig-sale_substate branch 2 times, most recently from c44a5bf to 1f6b22e Compare February 25, 2025 10:47
@Alexgars73 Alexgars73 force-pushed the 18.0-mig-sale_substate branch from 1f6b22e to 2c47da0 Compare March 27, 2025 15:29
@Alexgars73 Alexgars73 force-pushed the 18.0-mig-sale_substate branch from 2c47da0 to 7eebc4b Compare April 7, 2025 11:04
@Alexgars73 Alexgars73 force-pushed the 18.0-mig-sale_substate branch 2 times, most recently from 30c8aac to 301c7c8 Compare April 16, 2025 06:55
@dreispt
Copy link
Member

dreispt commented Apr 24, 2025

@Alexgars73 tests are failing, can you check please?
Also, you could help reviewing the dependency #1050

@Alexgars73 Alexgars73 force-pushed the 18.0-mig-sale_substate branch 2 times, most recently from 81796cf to 30bc5b8 Compare April 24, 2025 11:38
ellbristow
ellbristow reviewed May 16, 2025
View reviewed changes
sale_substate/views/sale_views.xml
Comment on lines 11 to 16
<field
name="substate_id"
widget="statusbar"
options="{'clickable': '1',}"
/>
Copy link

@ellbristow ellbristow May 16, 2025

Choose a reason for hiding this comment

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

Suggested change
<field
name="substate_id"
widget="statusbar"
options="{'clickable': '1',}"
/>
<field
name="substate_id"
widget="statusbar"
options="{'clickable': '1',}"
domain="[('model', '=', 'sale.order'),('target_state_value_id.target_state_value', '=', state)]"
/>

Filter options down to only valid substates.
Saves clutter and alleviates the need to show an error when the user clicks an invalid substate

Copy link
Contributor Author

@Alexgars73 Alexgars73 Jun 5, 2025

Choose a reason for hiding this comment

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

@ellbristow Done!.

@Alexgars73 Alexgars73 force-pushed the 18.0-mig-sale_substate branch from 30bc5b8 to 085a6eb Compare June 5, 2025 10:52
ellbristow
ellbristow approved these changes Jun 5, 2025
View reviewed changes
Copy link

@ellbristow ellbristow left a comment

Choose a reason for hiding this comment

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

Functionally tested. LGTM

theangryangel
theangryangel approved these changes Jun 16, 2025
View reviewed changes
Copy link
Member

@theangryangel theangryangel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Functionally tested, LGTM. No longer blocked by dependency.

However, looks like the commits need a tidy?

@dreispt
Copy link
Member

dreispt commented Jun 16, 2025

Great! Please remove the DON'T MERGE commit.

@dreispt
Copy link
Member

dreispt commented Jun 16, 2025

PS: tests are failing, perhaps a rebase can help?

@ellbristow
Copy link

ellbristow commented Jun 16, 2025

PS: tests are failing, perhaps a rebase can help?

FYI Failed tests are "Unreleased Dependencies"... Namely base_substate, which was merged 2 days ago

@Alexgars73 Alexgars73 force-pushed the 18.0-mig-sale_substate branch 2 times, most recently from 565adee to 9e4f278 Compare June 17, 2025 06:13
@Alexgars73
Copy link
Contributor Author

Alexgars73 commented Jun 17, 2025

@ellbristow Tests are failing, and I'm not sure why. I rebased, but that didn't fix it. Could you help me? Thanks!

@ellbristow
Copy link

ellbristow commented Jun 17, 2025

@Alexgars73 the failing test is not in this module. I don't believe this is related to anything you've done.
I will try to check the other module to see what is causing it to fail

@dreispt dreispt mentioned this pull request Jun 17, 2025
Merged
alexeirivera87 and others added 11 commits July 8, 2025 12:46
@Alexgars73 Alexgars73 force-pushed the 18.0-mig-sale_substate branch from 9e4f278 to c829ceb Compare July 8, 2025 10:47
@Alexgars73
Copy link
Contributor Author

Alexgars73 commented Jul 8, 2025

@dreispt @ellbristow test resolved!

@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented Jul 8, 2025

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

@Alexgars73
Copy link
Contributor Author

Alexgars73 commented Jul 16, 2025

@pedrobaeza could you merge?

@pedrobaeza
Copy link
Member

pedrobaeza commented Jul 16, 2025

Any review from authors, maintainers or PSC is needed.

/ocabot migration sale_substate

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Jul 16, 2025
@OCA-git-bot OCA-git-bot mentioned this pull request Jul 16, 2025
Open
@ellbristow
Copy link

ellbristow commented Jul 16, 2025

@chafique-delli can you review?
There is no maintainer declared. I'm happy to volunteer as maintainer. I am a paid member

chafique-delli
chafique-delli approved these changes Jul 17, 2025
View reviewed changes
Copy link
Contributor

@chafique-delli chafique-delli left a comment

Choose a reason for hiding this comment

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

Code review, LGTM.

@Alexgars73
Copy link
Contributor Author

Alexgars73 commented Jul 17, 2025

@pedrobaeza

@chafique-delli can you review? There is no maintainer declared. I'm happy to volunteer as maintainer. I am a paid member

@dreispt
Copy link
Member

dreispt commented Jul 23, 2025

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented Jul 23, 2025

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-3581-by-dreispt-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 35aa341 into OCA:18.0 Jul 23, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented Jul 23, 2025

Congratulations, your PR was merged at 10e9dd1. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dreispt dreispt dreispt approved these changes

+3 more reviewers

@theangryangel theangryangel theangryangel approved these changes

@ellbristow ellbristow ellbristow approved these changes

@chafique-delli chafique-delli chafique-delli approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved merged 🎉 ready to merge

Projects

None yet

Milestone

18.0

Development

Successfully merging this pull request may close these issues.