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

VP-1638: Chained Stackable policy discounts #81

Merged
merged 4 commits into from
Mar 18, 2020

Conversation

yecli
Copy link
Contributor

@yecli yecli commented Mar 16, 2020

  • After applying reward that could affect discount on cart total remaining discount conditions are re-evaluated;
  • Shipment rewards are applied after items and subtotal rewards now to allow shipment promotion use discounted cart subtotal in conditions;
  • Added test for applying rewards with the same priority;

- After applying reward that could affect discount on cart total remaining discount conditions are re-evaluated;
- Shipment rewards are applied after items and subtotal rewards now to allow shipment promotion use discounted cart subtotal in conditions;
- Added test for applying rewards with the same priority;
@yecli yecli requested a review from tatarincev March 16, 2020 16:27
- Added `IPromotionRewardEvaluator` abstraction to allow testing call times;
- Fixed problem with one promotion should apply all its different type rewards in one iteration;
- Added tests checking IPromotionRewardEvaluator calls count in different cases;
return result;
}

protected virtual void EvalAndCombineRewardsRecursively(PromotionEvaluationContext context, Func<PromotionEvaluationContext, IEnumerable<PromotionReward>> evalFunc, ICollection<PromotionReward> resultRewards, ICollection<PromotionReward> skippedRewards)
protected virtual void EvalAndCombineRewardsRecursively(PromotionEvaluationContext context, IEnumerable<Promotion> promotions, ICollection<PromotionReward> resultRewards, ICollection<PromotionReward> skippedRewards)
Copy link
Contributor

@vc-ci vc-ci Mar 17, 2020

Choose a reason for hiding this comment

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

CRITICAL Refactor this method to reduce its Cognitive Complexity from 18 to the 15 allowed. rule

- Now in Stackable policy rewards are grouped by promotions priorities so only same priority promotions are applied on one iteration. Thus we have the ability to strictly manage the order of all promotion appliance;
- Added test for sequential priorities appliance;
@vc-ci
Copy link
Contributor

vc-ci commented Mar 17, 2020

SonarQube analysis reported 2 issues

  • CRITICAL 2 critical

Watch the comments in this conversation to review them.

1 extra issue

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. CRITICAL CombineStackablePromotionPolicy.cs#L133: Refactor this method to reduce its Cognitive Complexity from 19 to the 15 allowed. rule

@yecli yecli merged commit 6886033 into dev Mar 18, 2020
@yecli yecli deleted the feature/VP-1638-discounted-total-chained-reward-appliance branch March 18, 2020 09:21
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