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

[Behat][Promotion] Fixed discount promotion scenarios #4199

Merged

Conversation

Zales0123
Copy link
Member

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets
License MIT
Doc PR

Based on #4170
First portion of new promotion scenarios, for very simple cases - receiving fixed discount without any rules and receiving discount equal to cart items total.

function it_creates_fixed_discount_action_for_promotion(
$actionRepository,
$objectManager,
$sharedStorage,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have the same order of arguments as in let?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@Zales0123 Zales0123 force-pushed the fixed-discount-promotion-scenarios branch 3 times, most recently from 0f79cc8 to 459ce8e Compare February 18, 2016 12:44
$action = $this->testPromotionFactory->createFixedDiscountAction($amount, $currentPromotion);
$this->actionRepository->add($action);

$this->objectManager->flush();
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to flush since repository on add does persist and flush :)

@Zales0123
Copy link
Member Author

Ok, after some brainstorm we have some conclusion, how for now it should be done 😄

  • per-type methods should be implemented on custom action factory in Core (which will be done in separate PR)
  • TestPromotionFactory should create only promotion and have no knowledge about actions
  • PromotionContext should have testPromotionFactory, promotionRepository and actionFactory (implemented in point 1)... and of course in the future custom ruleFactory

We believe this concept is the best approach at this moment, but surely it's really big thing to discuss, how these services should be implemented in the perfect Sylius ;)

@michalmarcinkowski michalmarcinkowski added the Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). label Feb 18, 2016
@Zales0123 Zales0123 force-pushed the fixed-discount-promotion-scenarios branch from 459ce8e to e7a9b26 Compare February 19, 2016 08:07
@Zales0123
Copy link
Member Author

UPDATE: After changing TestPromotionFactory logic slightly, this PR is now dependent on #4223, which should be first reviewed and merged ;)

public function thereIsPromotion($promotionName)
{
$promotion = $this->testPromotionFactory->create($promotionName);
$promotion->addChannel($this->sharedStorage->get('channel'));
Copy link
Contributor

Choose a reason for hiding this comment

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

How about $this->testPromotionFactory->createForChannel($promotionName, $this->sharedStorage->get('channel'));?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@Zales0123 Zales0123 force-pushed the fixed-discount-promotion-scenarios branch from e7a9b26 to 1152df9 Compare February 19, 2016 12:30
pjedrzejewski pushed a commit that referenced this pull request Feb 19, 2016
…arios

[Behat][Promotion] Fixed discount promotion scenarios
@pjedrzejewski pjedrzejewski merged commit bdc5c7a into Sylius:master Feb 19, 2016
@pjedrzejewski
Copy link
Member

Thank you Mateusz! 👍

@Zales0123 Zales0123 deleted the fixed-discount-promotion-scenarios branch October 6, 2016 20:28
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
…n-scenarios

[Behat][Promotion] Fixed discount promotion scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants