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

[CatalogPromotion][API] Extend the possibility of editing catalog promotion #13013

Merged
merged 3 commits into from
Aug 27, 2021

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented Aug 27, 2021

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets
License MIT

@GSadee GSadee added Feature New feature proposals. API APIs related issues and PRs. labels Aug 27, 2021
@GSadee GSadee requested a review from a team as a code owner August 27, 2021 06:53
Comment on lines +272 to +274
$response = $this->client->show($catalogPromotion->getCode());

Assert::true($this->responseChecker->hasTranslation($response, $localeCode, $field, $value));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$response = $this->client->show($catalogPromotion->getCode());
Assert::true($this->responseChecker->hasTranslation($response, $localeCode, $field, $value));
Assert::true($this->responseChecker->hasTranslation(
$this->client->show($catalogPromotion->getCode()),
$localeCode,
$field,
$value)
);

wdyt? :D

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even in one line all of the params.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would be for leaving as it is now, for me, it looks clearer

And I specify its label as "Christmas -50%" in "English (United States)"
And I describe it as "This promotion gives a 50% discount on all products" in "English (United States)"
And I save my changes
Then this catalog promotion label in "English (United States)" locale should be "Christmas -50%"
Copy link
Member

Choose a reason for hiding this comment

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

It's, of course, correct, but I feel we can go further with the naturalness of Behat steps

Then this catalog promotion should be labelled as "Christmas -50%" in "English (United States)

something like this? But still, it's good to go :D

Given the catalog promotion "Christmas sale" is available in "United States"
When I want to modify a catalog promotion "Christmas sale"
And I make it available in channel "Europe"
And I make it unavailable in channel "United States"
Copy link
Member

Choose a reason for hiding this comment

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

Btw, should we already take it into account in catalog promotion application? To not apply it on variants that are not available in some channel? 🤔

@Zales0123 Zales0123 merged commit 3022cba into Sylius:master Aug 27, 2021
@Zales0123
Copy link
Member

Thanks, Grzegorz! 🎉

@GSadee GSadee deleted the api-cp-update branch August 27, 2021 10:55
Zales0123 added a commit that referenced this pull request Aug 27, 2021
… promotions scenarios (GSadee)

This PR was merged into the 1.11-dev branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | master
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | resolves comment #13013 (comment)
| License         | MIT

<!--
 - Bug fixes must be submitted against the 1.9 or 1.10 branch (the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

845df97 [CatalogPromotion][Behat] Remove duplicated feature file
5abf3de [CatalogPromotion][Behat] Minor improvements for catalog promotions scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs. Feature New feature proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants