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][MIG] sale cancel reason #3062

Merged
merged 71 commits into from
Apr 15, 2024

Conversation

maciej-wichowski
Copy link

No description provided.

guewen and others added 30 commits April 9, 2024 13:16
(../trunk-generic/ rev 29.1.1)
(../trunk-generic/ rev 32.1.1)
In order to get visibility on https://www.odoo.com/apps the OCA board has
decided to add the OCA as author of all the addons maintained as part of the
association.
* [MIG] sale_cancel_reason Migration to 10.0

* Follow coding standard

* Convert test to unittest2
Updated by Actualizar ficheiros PO com o novo POT (msgmerge) hook in Weblate.
Currently translated at 100.0% (23 of 23 strings)

Translation: sale-workflow-10.0/sale-workflow-10.0-sale_cancel_reason
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-10-0/sale-workflow-10-0-sale_cancel_reason/pt/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-12.0/sale-workflow-12.0-sale_cancel_reason
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_cancel_reason/
Copy link
Sponsor Contributor

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

Could you add a Reason field to this wizard?
image
image

sale order.
</p>
<group>
<field name="reason_id" widget="selection" />
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Please remove the selection widget. With many records, filtering becomes complicated, and not all records are displayed.

Suggested change
<field name="reason_id" widget="selection" />
<field name="reason_id" options="{'no_create': True, 'no_open': True}" />

@maciej-wichowski
Copy link
Author

Hello @celm1990 , thank you for your review! I added possibility to choose cancel reason on mass quotation cancel wizard.
Regarding removing selection widget: I don't think we should make such improvements during module migration. I suggest to create an Issue in this repo to properly discuss your suggestion with the maintainers.

@rousseldenis
Copy link
Sponsor Contributor

/ocabot migration sale_cancel_reason

@@ -5,7 +5,7 @@

{
"name": "Sale Cancel Reason",
"version": "16.0.1.0.1",
"version": "17.0.1.0.1",
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Usually we reset to 17.0.1.0.0

Copy link
Sponsor 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.

Minor remark

Copy link
Sponsor Contributor

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

For consistency, both wizard fields must be the same (either many2one with widget=selection or not)

<field name="arch" type="xml">
<footer position="before">
<group>
<field name="reason_id" />
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

image

Suggested change
<field name="reason_id" />
<field name="reason_id" options="{'no_create': True, 'no_open': True}"/>

Copy link
Author

Choose a reason for hiding this comment

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

Forgot about it, thanks.

@maciej-wichowski maciej-wichowski force-pushed the 17.0-mig-sale_cancel_reason branch 2 times, most recently from 866d216 to 4be9715 Compare April 11, 2024 15:12
Copy link
Sponsor Contributor

@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 think these two commits can be squashed into a single commit
image

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

@maciej-wichowski
Copy link
Author

@rousseldenis , since it's approved, could you merge?

@rousseldenis
Copy link
Sponsor Contributor

I think these two commits can be squashed into a single commit image

@celm1990 FYI, if the commit is reflecting an improvement or is not related to the preceding one (migration), it's common to let them in place. As maybe, it can be easily forward/backported.

@rousseldenis
Copy link
Sponsor Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-3062-by-rousseldenis-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 454272e into OCA:17.0 Apr 15, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

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

@maciej-wichowski maciej-wichowski deleted the 17.0-mig-sale_cancel_reason branch April 16, 2024 10:25
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.

None yet