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

[NUT-201] Refactor used_by? method for one_use_per_user rule #10554

Merged

Conversation

KacperMekarski
Copy link
Contributor

This branch is created because of https://spark-solutions.atlassian.net/browse/NUT-201 task.
User can use one_use_per_user promotion many times.
This was tested, but in production user somehow can use the promotion many times.

Actual Behavior

So far condition was checked by looking for same PromotionAction through adjustments:

def used_by?(user, excluded_orders = [])

Although in tests PromotionActions are associated with adjustments as source, in production it occurs that it does not always happen:
irb(main):004:0> user.orders.complete.first.all_adjustments.where(source_type: "Spree::PromotionAction").pluck(:source_id) => [nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil] irb(main):005:0> user.orders.complete.second.all_adjustments.where(source_type: "Spree::PromotionAction").pluck(:source_id) => [nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil] irb(main):006:0> user.orders.complete.third.all_adjustments.where(source_type: "Spree::PromotionAction").pluck(:source_id) => []
However, order is always associated:
irb(main):009:0> user.orders.complete.first.promotions.pluck(:id) => [33, 33, 33, 33, 33, 33, 33] irb(main):010:0> user.orders.complete.second.promotions.pluck(:id) => [33] irb(main):011:0> user.orders.complete.third.promotions.pluck(:id) => [33, 33, 33, 33]

Expected behavior

User can use promo code only once

Possible fix

Use promotion to check if it was already used (simpler and based on production data it works).

@squash-labs
Copy link

squash-labs bot commented Oct 23, 2020

Manage this branch in Squash

Test this branch here: https://spark-solutionsone-use-per-use-tm48z.squash.io

@KacperMekarski
Copy link
Contributor Author

@przemosk
@piotrpanko
@aplegatt
@szymoniwacz

Happy to hear Your thoughts about that!

@KacperMekarski KacperMekarski changed the title Refactor used_by? method for one_use_per_user rule [NUT-200] Refactor used_by? method for one_use_per_user rule Oct 27, 2020
@KacperMekarski KacperMekarski changed the title [NUT-200] Refactor used_by? method for one_use_per_user rule [NUT-201] Refactor used_by? method for one_use_per_user rule Oct 27, 2020
@damianlegawiec damianlegawiec merged commit d002bbb into spree:master Oct 29, 2020
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.

None yet

5 participants