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

Promotion eligibility? usage count #3205

Closed
cedum opened this issue May 10, 2019 · 3 comments
Closed

Promotion eligibility? usage count #3205

cedum opened this issue May 10, 2019 · 3 comments
Labels
type:enhancement Proposed or newly added feature

Comments

@cedum
Copy link
Contributor

cedum commented May 10, 2019

When performing the eligibility check for a promotion, the usage limit check should not count the promotable that has been already applied for the promotion.
I suspect the same must be done also when re-computing the amounts in promo actions, otherwise, it sets them to zero.

Here's a test case (I did my best to explain it without letting my brain go into a loop 💥):

  • Given I have an active promotion with 1 usage limit
  • Given I have a completed order with the promotion above applied
  • When I edit and save the promotion' adjustment and the promo has 0 usages left
    • The adjustment is saved as ineligible and amount set to 0
    • The promotion changes to 1 usage left
  • When I edit and save again the promotion' adjustment
    • The adjustment is saved as eligible and amount to the promo discount
    • The promotion changes back to "0 usages left"

Rinse and repeat to switch the adjustment to ineligible and back

Reproduced against solidus versions: 2.9.0.alpha and 2.7.1

@loicginoux
Copy link
Contributor

loicginoux commented May 10, 2019

Just checking but was your order completed ? it should only count promotion usage for completed order normally. If it does it with incomplete order, that's something weird indeed.

#promotion model:

    def usage_count
      Spree::Adjustment.eligible.
        promotion.
        where(source_id: actions.map(&:id)).
        joins(:order).
        merge(Spree::Order.complete).
        distinct.
        count(:order_id)
    end

@cedum
Copy link
Contributor Author

cedum commented May 10, 2019

@loicginoux yes, tested it on a completed order (I've updated the issue description to reflect this)

@kennyadsl
Copy link
Member

I think you are right, this happens because the eligibility check is done when there are no eligible "usage" left. I think we should add that "usage" to the count somehow to make it work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Proposed or newly added feature
Projects
None yet
Development

No branches or pull requests

4 participants