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] shopinvader_api_sale_loyalty: do not update rewards when reading a sale #1552

Conversation

sebastienbeau
Copy link
Contributor

After reading the issue and the #1538 and the PR #1543

I do not think it's necessary to update the reward everytime we call the information on sale object. The method is already call when updating the cart : https://github.com/shopinvader/odoo-shopinvader/blob/16.0/shopinvader_api_sale_loyalty/routers/cart.py#L203

I just do that PR to show that the test are still working even if we remove it.

@sebastienbeau
Copy link
Contributor Author

After playing with the "sale_loyalty", I pretty sure we can simply remove the method.

If a "malicious" customer try to use several time the same loyalty program (like passing several cart with a reward that consume point), the action_confirm of the sale order will block the validation, so their is no way to abuse.

@sebastienbeau sebastienbeau added this to the 16.0 milestone Jun 4, 2024
@sebastienbeau sebastienbeau changed the title shopinvader_api_sale_loyalty: do not update rewards when reading a sale [16.0] shopinvader_api_sale_loyalty: do not update rewards when reading a sale Jun 4, 2024
@sebastienbeau
Copy link
Contributor Author

/ocabot merge minor

@sebastienbeau sebastienbeau deleted the 16.0-do-not-update-loyalty-on-get branch September 19, 2024 15:27
@shopinvader-git-bot
Copy link

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-1552-by-sebastienbeau-bump-minor, awaiting test results.

shopinvader-git-bot pushed a commit that referenced this pull request Oct 23, 2024
Signed-off-by sebastienbeau
@shopinvader-git-bot
Copy link

@sebastienbeau your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1552-by-sebastienbeau-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants