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

[14.0][IMP]sale_line_refund_to_invoice_qty: credit note default #1748

Merged
merged 1 commit into from
Jul 3, 2024
Merged

[14.0][IMP]sale_line_refund_to_invoice_qty: credit note default #1748

merged 1 commit into from
Jul 3, 2024

Conversation

PicchiSeba
Copy link
Contributor

@PicchiSeba PicchiSeba commented Jun 26, 2024 •
edited
Loading

add setting for default value in credit note wizard

@PicchiSeba PicchiSeba force-pushed the 14.0-imp-sale_line_refund_to_invoice_qty-default branch 3 times, most recently from 60800ba to f240d56 Compare June 27, 2024 12:09
@PicchiSeba PicchiSeba changed the title [14.0][IMP]sale_line_refund_to_invoice_qty: add setting for default value i… [14.0][IMP]sale_line_refund_to_invoice_qty: credit note default Jun 27, 2024
@PicchiSeba PicchiSeba force-pushed the 14.0-imp-sale_line_refund_to_invoice_qty-default branch from f240d56 to ff3962d Compare June 27, 2024 13:21
francesco-ooops
francesco-ooops approved these changes Jun 27, 2024
View reviewed changes
Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Functional ok!

Enabling the "Do not reinvoice credit note by default" flag in the Invoice settings, will
disable the "This credit note will be reinvoiced" flag on Credit Notes by default

ilyasProgrammer
ilyasProgrammer approved these changes Jun 27, 2024
View reviewed changes
Copy link
Member

@ilyasProgrammer ilyasProgrammer left a comment

Choose a reason for hiding this comment

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

LGTM

@francesco-ooops
Copy link
Contributor

@JordiMForgeFlow @LoisRForgeFlow is this addition ok for you? thanks!

@JordiMForgeFlow
Copy link
Contributor

@francesco-ooops I think it is much more understandable to have boolean variables that reflect "enabling" something when they are set to True, as it is more intuitive. I would recommend instead of having no_default_reinvoice_credit_note to have a default_reinvoice_credit_note that is set to True by default.

@francesco-ooops
Copy link
Contributor

@PicchiSeba can you attend comment?

@PicchiSeba PicchiSeba force-pushed the 14.0-imp-sale_line_refund_to_invoice_qty-default branch 3 times, most recently from 1c5163a to b848376 Compare June 28, 2024 11:05
@PicchiSeba
Copy link
Contributor Author

@JordiMForgeFlow I updated this PR with your suggestion

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

@PicchiSeba PicchiSeba force-pushed the 14.0-imp-sale_line_refund_to_invoice_qty-default branch 3 times, most recently from c4e557f to 24f452a Compare July 1, 2024 15:03
JordiMForgeFlow
JordiMForgeFlow reviewed Jul 2, 2024
View reviewed changes
Copy link
Contributor

@JordiMForgeFlow JordiMForgeFlow 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 also attend the failed CI? Thank you :)

sale_line_refund_to_invoice_qty/wizards/account_move_reversal.py Outdated Show resolved Hide resolved
sale_line_refund_to_invoice_qty/wizards/account_move_reversal.py Outdated Show resolved Hide resolved
@PicchiSeba PicchiSeba force-pushed the 14.0-imp-sale_line_refund_to_invoice_qty-default branch 2 times, most recently from 6fdf700 to ca17059 Compare July 3, 2024 10:09
@francesco-ooops
Copy link
Contributor

@JordiMForgeFlow looks good to go :)

@PicchiSeba
[IMP]sale_line_refund_to_invoice_qty: credit note default
3286073 
add setting for default value in credit note wizard
@PicchiSeba PicchiSeba force-pushed the 14.0-imp-sale_line_refund_to_invoice_qty-default branch from ca17059 to 3286073 Compare July 3, 2024 12:47
JordiMForgeFlow
JordiMForgeFlow approved these changes Jul 3, 2024
View reviewed changes
Copy link
Contributor

@JordiMForgeFlow JordiMForgeFlow 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 👍🏼

@PicchiSeba
Copy link
Contributor Author

@JordiMForgeFlow Thanks for your patience!

francesco-ooops
francesco-ooops approved these changes Jul 3, 2024
View reviewed changes
Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Functional is ok as well, thanks again @JordiMForgeFlow

@pedrobaeza can we merge?

@pedrobaeza pedrobaeza added this to the 14.0 milestone Jul 3, 2024
@pedrobaeza
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-1748-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 4bb5117 into OCA:14.0 Jul 3, 2024
9 checks passed
@OCA-git-bot
Copy link
Contributor

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

@PicchiSeba PicchiSeba deleted the 14.0-imp-sale_line_refund_to_invoice_qty-default branch July 3, 2024 15:32
@florian-dacosta florian-dacosta mentioned this pull request Feb 7, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilyasProgrammer ilyasProgrammer ilyasProgrammer approved these changes

@francesco-ooops francesco-ooops francesco-ooops approved these changes

@JordiMForgeFlow JordiMForgeFlow JordiMForgeFlow approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
14.0
Development

Successfully merging this pull request may close these issues.
6 participants
@PicchiSeba @francesco-ooops @JordiMForgeFlow @OCA-git-bot @pedrobaeza @ilyasProgrammer