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

[FIX] sale_loyalty: prevent coupon removal on double apply #153485

Closed
wants to merge 1 commit into from
Closed

[FIX] sale_loyalty: prevent coupon removal on double apply #153485

wants to merge 1 commit into from

Conversation

mathysPaul
Copy link
Contributor

@mathysPaul mathysPaul commented Feb 9, 2024 •
edited

Addresses the issue where reapplying an already applied coupon
in the website shop led to the disappearance of the discount.
With this fix, the discount remains applied, and the system
continues to inform the user that the coupon has already been
used, preventing confusion and maintaining consistency in the
discount application process.

task-3621246

@mathysPaul mathysPaul self-assigned this Feb 9, 2024
@robodoo
Copy link
Contributor

robodoo commented Feb 9, 2024

Pull request status dashboard.

@C3POdoo C3POdoo added the RD research & development, internal work label Feb 9, 2024
@anko-odoo
Copy link
Contributor

anko-odoo commented Feb 19, 2024 •
edited

Hello 🦆 , LGTM, but squash commits.

@mathysPaul mathysPaul force-pushed the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch from 1d8ac95 to a88d48f Compare February 19, 2024 09:21
anko-odoo
anko-odoo approved these changes Feb 19, 2024
View reviewed changes
Copy link
Contributor

@anko-odoo anko-odoo left a comment

Choose a reason for hiding this comment

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

LGTM

@mathysPaul mathysPaul marked this pull request as ready for review February 20, 2024 12:49
@mathysPaul mathysPaul force-pushed the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch from a2188d3 to 1a5d486 Compare March 7, 2024 08:16
@mathysPaul mathysPaul requested a review from Feyensv March 7, 2024 09:47
@Feyensv
Copy link
Contributor

Feyensv commented Mar 11, 2024

The two commits can be squashed 👀
(and could you add a little test too please ? 🙏)

@mathysPaul mathysPaul force-pushed the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch from 3a7bec3 to 2783ef8 Compare March 21, 2024 15:07
Feyensv
Feyensv reviewed Mar 21, 2024
View reviewed changes
addons/sale_loyalty/tests/test_loyalty.py Outdated Show resolved Hide resolved
addons/sale_loyalty/tests/test_loyalty.py Outdated Show resolved Hide resolved
@mathysPaul mathysPaul force-pushed the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch 2 times, most recently from ec3e200 to b6fb86b Compare March 21, 2024 16:20
@mathysPaul mathysPaul requested a review from Feyensv March 22, 2024 07:31
@mathysPaul mathysPaul force-pushed the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch from b6fb86b to eb3dbdc Compare March 22, 2024 13:19
@Feyensv
Copy link
Contributor

Feyensv commented Mar 22, 2024

  • please rebase to have the latest fixes of runbot issues (cf red runbot)
  • please update the commit message, you should mention the modules you're updating, not the ones you're fixing (website_sale is not updated here).
  • the test is green even without your changes, so it's not correctly testing what you fixed 👀

@mathysPaul mathysPaul force-pushed the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch 2 times, most recently from 91ed8ca to 1374e67 Compare March 27, 2024 11:56
@mathysPaul mathysPaul changed the title [FIX] website_sale: prevent coupon removal on double apply [FIX] website_sale_loyalty: prevent coupon removal on double apply Mar 27, 2024
@mathysPaul mathysPaul force-pushed the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch from 1374e67 to 857ead7 Compare March 27, 2024 13:43
@mathysPaul mathysPaul changed the title [FIX] website_sale_loyalty: prevent coupon removal on double apply [FIX] sale_loyalty: prevent coupon removal on double apply Mar 27, 2024
@mathysPaul mathysPaul requested a review from Feyensv March 27, 2024 15:27
@mathysPaul mathysPaul force-pushed the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch 2 times, most recently from c8bf2e0 to 6db4769 Compare March 28, 2024 13:40
@mathysPaul mathysPaul requested a review from Feyensv March 28, 2024 13:57
@mathysPaul @Feyensv
[FIX] sale_loyalty: prevent coupon removal on double apply
ed74552 
Addresses the issue where reapplying an already applied coupon
in the website shop led to the disappearance of the discount.
With this fix, the discount remains applied, and the system
continues to inform the user that the coupon has already been
used, preventing confusion and maintaining consistency in the
discount application process.

task-3621246
@Feyensv Feyensv force-pushed the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch from 6db4769 to ed74552 Compare March 28, 2024 16:04
Feyensv
Feyensv approved these changes Mar 28, 2024
View reviewed changes
Copy link
Contributor

@Feyensv Feyensv left a comment

Choose a reason for hiding this comment

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

Little push to rely on the test class setup data in the test (improve test perf while keeping the fix covered).

@robodoo r+

@robodoo robodoo closed this in 64d1feb Mar 28, 2024
This was referenced Mar 28, 2024
Closed
Closed
Closed
@fw-bot fw-bot deleted the 17.0-sale-using-same-coupon-twice-removes-discount-matp branch April 11, 2024 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Feyensv Feyensv Feyensv approved these changes

@anko-odoo anko-odoo anko-odoo approved these changes

Assignees

@mathysPaul mathysPaul

Labels
RD research & development, internal work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mathysPaul @robodoo @anko-odoo @Feyensv @C3POdoo