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.

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

[16.0][IMP] rma_sale: operation required #410

Merged
merged 1 commit into from
Aug 16, 2024
Merged

[16.0][IMP] rma_sale: operation required #410

merged 1 commit into from
Aug 16, 2024

Conversation

sbejaoui
Copy link
Contributor

@sbejaoui sbejaoui commented Jul 22, 2024 •
edited
Loading

The rma operation is required in the portal, and it makes sense for it to be required in the sales wizard as well

fixes: #404

cc/ @pedrobaeza , @jbaudoux , @lmignon , @rousseldenis

@OCA-git-bot
Copy link
Contributor

Hi @chienandalu,
some modules you are maintaining are being modified, check this out!

@sbejaoui sbejaoui marked this pull request as draft July 22, 2024 07:57
@sbejaoui sbejaoui force-pushed the 16.0-rma_operation_required-sbj branch from 026abc2 to 38d0867 Compare July 22, 2024 08:03
@sbejaoui sbejaoui changed the title [IMP] rma_sale: operation_id required [IMP] rma_sale: operation required Jul 22, 2024
@sbejaoui sbejaoui changed the title [IMP] rma_sale: operation required [16.0][IMP] rma_sale: operation required Jul 22, 2024
@sbejaoui sbejaoui force-pushed the 16.0-rma_operation_required-sbj branch 4 times, most recently from cdf4b1a to 4b8f9ab Compare July 22, 2024 12:49
@sbejaoui sbejaoui marked this pull request as ready for review July 22, 2024 12:53
@rousseldenis rousseldenis added this to the 16.0 milestone Jul 29, 2024
rousseldenis
rousseldenis approved these changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@rousseldenis rousseldenis 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

pedrobaeza
pedrobaeza reviewed Jul 29, 2024
View reviewed changes
rma_sale/wizard/sale_order_rma_wizard_views.xml Show resolved Hide resolved
rma/tests/test_rma.py Outdated Show resolved Hide resolved
rma/wizard/stock_picking_return_views.xml Outdated Show resolved Hide resolved
@sbejaoui sbejaoui force-pushed the 16.0-rma_operation_required-sbj branch 3 times, most recently from 90ce4a5 to 252cae6 Compare July 29, 2024 10:26
@sbejaoui sbejaoui requested review from jbaudoux and pedrobaeza July 29, 2024 10:35
jbaudoux
jbaudoux approved these changes Jul 30, 2024
View reviewed changes
Copy link

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

LG, Thanks

jbaudoux
jbaudoux reviewed Aug 2, 2024
View reviewed changes
Copy link

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

Can you perform the same in the stock picking return ? And add the operation in the lines.
You can also create RMAs from there

@sbejaoui sbejaoui mentioned this pull request Aug 15, 2024
Open
@sbejaoui
Copy link
Contributor Author

Can you perform the same in the stock picking return ? And add the operation in the lines. You can also create RMAs from there

done

image

@sbejaoui sbejaoui force-pushed the 16.0-rma_operation_required-sbj branch from 252cae6 to 3f0d9cf Compare August 15, 2024 13:11
@sbejaoui
Copy link
Contributor Author

@pedrobaeza , ready for merge

@sbejaoui sbejaoui mentioned this pull request Aug 15, 2024
Open
@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). 🤖

jbaudoux
jbaudoux requested changes Aug 16, 2024
View reviewed changes
Copy link

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

Picking return wizard to fix

@pedrobaeza
Copy link
Member

Please squash commits together. They don't add value as separated. And remember you can do git commit --amend and git push -f, as GitHub will put the diff of such forced push, so you avoid extra CI builds for nothing.

@sbejaoui
[IMP] rma_sale: operation required
142a3c3 
The rma operation is required in the portal, and it makes sense
for it to be required in the sales wizard as well
@sbejaoui sbejaoui force-pushed the 16.0-rma_operation_required-sbj branch from a93dca8 to 142a3c3 Compare August 16, 2024 08:19
@sbejaoui sbejaoui requested a review from jbaudoux August 16, 2024 08:24
@sbejaoui
Copy link
Contributor Author

@jbaudoux , could you please update your review? We'd like to merge this PR since many others depend on it.

@pedrobaeza
Copy link
Member

/ocabot merge major

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-410-by-pedrobaeza-bump-major, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit e387b0c into OCA:16.0 Aug 16, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 07a7ab1. 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

@pedrobaeza pedrobaeza pedrobaeza approved these changes

@jbaudoux jbaudoux jbaudoux approved these changes

@rousseldenis rousseldenis rousseldenis approved these changes

Assignees
No one assigned
Labels
approved enhancement merged 🎉 needs review
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.

RMA - require operation
5 participants
@sbejaoui @OCA-git-bot @pedrobaeza @jbaudoux @rousseldenis