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

[FW][FIX] pos_restaurant: Grant loyalty points for products only #162268

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Apr 17, 2024

Steps to reproduce:

  • Install POS app.
  • Go to POS > Products > Discount & Loyalty
  • Create a New program with:
    • Program Type: Loyalty Cards
    • Conditional rules:
      • Minimum Quantity: 2
      • Grant: 1 Loyalty Points per unit paid
    • Rewards:
      • Reward Type: Discount
      • Discount: 100 % one Cheapest Product
      • In exchange of: 2 Loyalty Points
  • Start a new POS session
  • Select a Customer
  • Add two different products.
  • Notice the Loyalty Points of +2 shown. This is Correct
  • Click on the Reward button
  • Notice how the Loyalty Points are now +3 which is obviously wrong given we only have two products. Basically it's as if the reward line (100% discount) is taken into consideration as the cheapest product.

Investigation:

  • Inside _updatePrograms, pointsForPrograms() are calculated.
  • we sum the lines quantities regardless of whether it's a reward line or not
    totalProductQty += lineQty;
  • By doing so, the reward lines are taken into consideration and the rule is triggerd by skipping this if clause
    if (totalProductQty < rule.minimum_qty) {
    // Should also count the points from negative quantities.
    // For example, when refunding an ewallet payment. See TicketScreen override in this addon.
    continue;
    }

opw-3855323

Forward-Port-Of: #161503

@robodoo
Copy link
Contributor

robodoo commented Apr 17, 2024

@fw-bot
Copy link
Contributor Author

fw-bot commented Apr 17, 2024

@AH-Yussef @caburj cherrypicking of pull request #161503 failed.

stdout:

Auto-merging addons/pos_loyalty/static/src/js/Loyalty.js
CONFLICT (content): Merge conflict in addons/pos_loyalty/static/src/js/Loyalty.js
Auto-merging addons/pos_loyalty/static/src/tours/PosLoyaltyLoyaltyProgramTour.js
CONFLICT (content): Merge conflict in addons/pos_loyalty/static/src/tours/PosLoyaltyLoyaltyProgramTour.js
Auto-merging addons/pos_loyalty/tests/test_frontend.py

stderr:

17:45:15.149495 git.c:463               trace: built-in: git cherry-pick 8a95b9f169a34259cf981bc73d16a99be0b9ae56
error: could not apply 8a95b9f169a3... [FIX] pos_restaurant: Grant loyalty points for products only
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

⚠️ after resolving this conflict, you will need to merge it via @robodoo.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@robodoo robodoo added forwardport This PR was created by @fw-bot conflict There was an error while creating this forward-port PR labels Apr 17, 2024
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Apr 17, 2024
@AH-Yussef AH-Yussef force-pushed the saas-16.3-16.0-opw-3855323-grant_loyalty_points_for_products_only-alhy-rmVV-fw branch from 8fb0888 to 2de0bc4 Compare April 18, 2024 10:19
@C3POdoo C3POdoo requested review from a team and vlst-odoo and removed request for a team April 18, 2024 10:37
@AH-Yussef AH-Yussef force-pushed the saas-16.3-16.0-opw-3855323-grant_loyalty_points_for_products_only-alhy-rmVV-fw branch 2 times, most recently from ae10b3a to 2aa0900 Compare April 21, 2024 01:45
@AH-Yussef AH-Yussef force-pushed the saas-16.3-16.0-opw-3855323-grant_loyalty_points_for_products_only-alhy-rmVV-fw branch from 2aa0900 to 31d9199 Compare April 24, 2024 01:01
@caburj
Copy link
Contributor

caburj commented Apr 29, 2024

Looks like you are missing a step. A dialog is still displayed and you might need to select the reward.

@AH-Yussef AH-Yussef force-pushed the saas-16.3-16.0-opw-3855323-grant_loyalty_points_for_products_only-alhy-rmVV-fw branch 2 times, most recently from e29e2b2 to bd3edab Compare May 2, 2024 10:02
@AH-Yussef AH-Yussef force-pushed the saas-16.3-16.0-opw-3855323-grant_loyalty_points_for_products_only-alhy-rmVV-fw branch from bd3edab to 371d04e Compare May 16, 2024 08:18
Steps to reproduce:
- Install POS app.
- Go to POS > Products > Discount & Loyalty
- Create a New program with:
    - Program Type: Loyalty Cards
    - Conditional rules:
        - Minimum Quantity: 2
        - Grant: 1 Loyalty Points per unit paid
- Rewards:
    - Reward Type: Discount
    - Discount: 100 % one Cheapest Product
    - In exchange of 2 Loyalty Points
- Start a new POS session
- Select a Customer
- Add two different products.
- Notice the Loyalty Points of +2 shown. This is Correct
- Click on the Reward button
- Notice how the Loyalty Points are now +3 which is obviously wrong given we only have two products. Basically it's as if the reward line (100% discount) is taken into consideration as the cheapest product.

Investigation:
- Inside `_updatePrograms`, `pointsForPrograms()` are calculated.
- we sum the lines quantities regardless of whether it's a reward line or not https://github.com/odoo/odoo/blob/e5c3ba58964f47cfd41d337e39e1bf25eaa25379/addons/pos_loyalty/static/src/js/Loyalty.js#L906
- By doing so, the reward lines are taken into consideration and the rule is triggerd by skipping this if clause https://github.com/odoo/odoo/blob/e5c3ba58964f47cfd41d337e39e1bf25eaa25379/addons/pos_loyalty/static/src/js/Loyalty.js#L917-L921

opw-3855323

X-original-commit: 6336e45
@AH-Yussef AH-Yussef force-pushed the saas-16.3-16.0-opw-3855323-grant_loyalty_points_for_products_only-alhy-rmVV-fw branch from 371d04e to dcfc022 Compare May 16, 2024 09:28
@AH-Yussef
Copy link
Contributor

@robodoo r+

robodoo pushed a commit that referenced this pull request May 16, 2024
Steps to reproduce:
- Install POS app.
- Go to POS > Products > Discount & Loyalty
- Create a New program with:
    - Program Type: Loyalty Cards
    - Conditional rules:
        - Minimum Quantity: 2
        - Grant: 1 Loyalty Points per unit paid
- Rewards:
    - Reward Type: Discount
    - Discount: 100 % one Cheapest Product
    - In exchange of 2 Loyalty Points
- Start a new POS session
- Select a Customer
- Add two different products.
- Notice the Loyalty Points of +2 shown. This is Correct
- Click on the Reward button
- Notice how the Loyalty Points are now +3 which is obviously wrong given we only have two products. Basically it's as if the reward line (100% discount) is taken into consideration as the cheapest product.

Investigation:
- Inside `_updatePrograms`, `pointsForPrograms()` are calculated.
- we sum the lines quantities regardless of whether it's a reward line or not https://github.com/odoo/odoo/blob/e5c3ba58964f47cfd41d337e39e1bf25eaa25379/addons/pos_loyalty/static/src/js/Loyalty.js#L906
- By doing so, the reward lines are taken into consideration and the rule is triggerd by skipping this if clause https://github.com/odoo/odoo/blob/e5c3ba58964f47cfd41d337e39e1bf25eaa25379/addons/pos_loyalty/static/src/js/Loyalty.js#L917-L921

opw-3855323

closes #162268

X-original-commit: 6336e45
Signed-off-by: Joseph Caburnay (jcb) <jcb@odoo.com>
Signed-off-by: Ali Hassan Youssef (alhy) <alhy@odoo.com>
@robodoo robodoo closed this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants