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

[16.0][ADD] shopinvader_api_sale_loyalty: FastAPI services to apply coupons #1430

Conversation

marielejeune
Copy link
Contributor

@marielejeune marielejeune commented Oct 18, 2023

FastAPI services for applying coupons to cart.

Depends on

TODO

  • /apply_coupon service must be on cart router, not loyalty router
  • Add a service /apply_reward to apply a claimable reward (automatic program, without code)
  • When updating cart items, we must try to apply automatic rewards if possible (a unique program, no reward choice, no product choice). The CartResponse must return all claimable rewards, so that before validating the cart, if some rewards are still claimable, they can be applied (by calling the /apply_reward service, chosing the right reward / free product).
  • The shopinvader_cart_user_group must have the shopinvader_loyalty_user_group as it uses all loyalties concepts.
  • Keep the distinction between reward_amount and reward_amount_tax_included.

@marielejeune marielejeune force-pushed the 16.0-shopinvader_api_sale_loyalty-mle branch from 3c6d596 to 09f925a Compare October 18, 2023 07:23
@marielejeune marielejeune changed the title [WIP][ADD] shopinvader_api_sale_loyalty: FastAPI services to apply coupons [16.0][WIP][ADD] shopinvader_api_sale_loyalty: FastAPI services to apply coupons Oct 18, 2023
@marielejeune marielejeune force-pushed the 16.0-shopinvader_api_sale_loyalty-mle branch 4 times, most recently from 94ab4b8 to 560b584 Compare October 20, 2023 09:22
@marielejeune
Copy link
Contributor Author

This module will maybe need some schemas adaptations when #1421 will be finished but it's ready for first reviews.

@marielejeune marielejeune marked this pull request as ready for review October 20, 2023 09:25
@marielejeune marielejeune changed the title [16.0][WIP][ADD] shopinvader_api_sale_loyalty: FastAPI services to apply coupons [16.0][ADD] shopinvader_api_sale_loyalty: FastAPI services to apply coupons Oct 20, 2023
Copy link
Collaborator

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

@marielejeune The code is clear and easy to read! A few comments to make it even better.

shopinvader_api_sale_loyalty/routers/cart.py Outdated Show resolved Hide resolved
shopinvader_api_sale_loyalty/routers/cart.py Outdated Show resolved Hide resolved
shopinvader_api_sale_loyalty/models/sale_order.py Outdated Show resolved Hide resolved
shopinvader_api_sale_loyalty/routers/loyalty.py Outdated Show resolved Hide resolved
shopinvader_api_sale_loyalty/routers/cart.py Outdated Show resolved Hide resolved
shopinvader_api_sale_loyalty/routers/cart.py Outdated Show resolved Hide resolved
@marielejeune marielejeune force-pushed the 16.0-shopinvader_api_sale_loyalty-mle branch from 560b584 to e50c9a0 Compare November 3, 2023 14:30
@marielejeune marielejeune force-pushed the 16.0-shopinvader_api_sale_loyalty-mle branch 10 times, most recently from a1bf7e8 to 81b5e36 Compare November 13, 2023 13:26
@simahawk
Copy link
Contributor

@marielejeune ciao, any plan to finish this?
BTW I opened a PR against your branch to fix perms, see acsone#8
Could you have a look at it?

@marielejeune
Copy link
Contributor Author

Hi @simahawk , this is finished and waiting for reviews :-) Feel free also to have a look at the dependent PR in OCA/sale-workflow

@marielejeune marielejeune force-pushed the 16.0-shopinvader_api_sale_loyalty-mle branch from 81b5e36 to 7288c6f Compare January 16, 2024 14:05
@marielejeune
Copy link
Contributor Author

marielejeune commented Jan 16, 2024

CHANGES (following the logic in #1495)

  • Routes renaming (preserve old routes, with deprecated=True)
  • cart_uuid is now of type UUID, not str anymore
  • Add some doc
  • Fix _sync_cart method: don't apply rewards on a non existing cart

@marielejeune marielejeune force-pushed the 16.0-shopinvader_api_sale_loyalty-mle branch from 7288c6f to 89fb7d2 Compare January 16, 2024 14:10
@marielejeune
Copy link
Contributor Author

@marielejeune ciao, any plan to finish this? BTW I opened a PR against your branch to fix perms, see acsone#8 Could you have a look at it?

I've cherry-picked your commit

@lmignon
Copy link
Collaborator

lmignon commented Jan 17, 2024

@marielejeune All your dev dependencies are now merged... can you cleanup the test-requirements plz?

@marielejeune marielejeune force-pushed the 16.0-shopinvader_api_sale_loyalty-mle branch from 35981a7 to 69caba6 Compare January 17, 2024 15:16
@marielejeune marielejeune force-pushed the 16.0-shopinvader_api_sale_loyalty-mle branch from 69caba6 to 39c4f27 Compare January 18, 2024 09:08
@marielejeune
Copy link
Contributor Author

marielejeune commented Jan 18, 2024

I've also added the current alias on the new cart routes, and sorted them.

@lmignon
Copy link
Collaborator

lmignon commented Feb 1, 2024

/ocabot merge path

@shopinvader-git-bot
Copy link

Hi @lmignon. Your command failed:

Invalid options for command merge: path.

Ocabot commands

  • ocabot merge major|minor|patch|nobump
  • ocabot rebase
  • ocabot migration {MODULE_NAME}

More information

@lmignon
Copy link
Collaborator

lmignon commented Feb 1, 2024

/ocabot merge patch

@shopinvader-git-bot
Copy link

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-1430-by-lmignon-bump-patch, awaiting test results.

shopinvader-git-bot pushed a commit that referenced this pull request Feb 1, 2024
Signed-off-by lmignon
@shopinvader-git-bot
Copy link

It looks like something changed on 16.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 16.0-ocabot-merge-pr-1430-by-lmignon-bump-patch, awaiting test results.

shopinvader-git-bot pushed a commit that referenced this pull request Feb 1, 2024
Signed-off-by lmignon
@shopinvader-git-bot
Copy link

It looks like something changed on 16.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 16.0-ocabot-merge-pr-1430-by-lmignon-bump-patch, awaiting test results.

@shopinvader-git-bot shopinvader-git-bot merged commit 7844c78 into shopinvader:16.0 Feb 1, 2024
3 checks passed
@shopinvader-git-bot
Copy link

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

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.

5 participants