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.

Sign up for GitHub

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

Jump to bottom

List of channel currencies endpoint #13287

Merged
merged 1 commit into from
Nov 17, 2021
Merged

List of channel currencies endpoint #13287

merged 1 commit into from
Nov 17, 2021

Conversation

paullla
Copy link
Contributor

@paullla paullla commented Nov 11, 2021

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

@paullla paullla requested a review from a team as a code owner November 11, 2021 15:46
@probot-autolabeler probot-autolabeler bot added the API APIs related issues and PRs. label Nov 11, 2021
@paullla paullla force-pushed the shop-currencies-list-api branch from 541aa45 to 292a698 Compare November 12, 2021 09:08
@paullla paullla removed the request for review from a team November 12, 2021 09:09
Zales0123
Zales0123 reviewed Nov 12, 2021
View reviewed changes
features/currency/switching_current_currency.feature Outdated
@api
Scenario: Showing available currencies
When I browse currencies
Then I should see 2 currencies in the list
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be also worth to check what currencies are they?

src/Sylius/Bundle/ApiBundle/DataProvider/CurrencyCollectionDataProvider.php Show resolved Hide resolved
features/currency/switching_current_currency.feature Outdated
Scenario: Showing available currencies
When I browse that channel
Then I should be able to shop using the "USD" currency

@api
Copy link
Member

Choose a reason for hiding this comment

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

Wrong indent

@paullla paullla force-pushed the shop-currencies-list-api branch from 92283fa to f3824d6 Compare November 16, 2021 07:48
@lchrusciel lchrusciel added the Feature New feature proposals. label Nov 17, 2021
GSadee
GSadee approved these changes Nov 17, 2021
View reviewed changes
src/Sylius/Behat/Context/Api/Shop/CurrencyContext.php
* @Then I should see :firstCurrency and :secondCurrency in the list
* @Then I should see :firstCurrency, :secondCurrency and :thirdCurrency in the list
*/
public function iShouldSeeCurrenciesInTheList(... $currencies): void
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public function iShouldSeeCurrenciesInTheList(... $currencies): void
public function iShouldSeeCurrenciesInTheList(string ...$currenciesCodes): void

@GSadee GSadee merged commit 32f2c6e into Sylius:master Nov 17, 2021
@GSadee
Copy link
Member

GSadee commented Nov 17, 2021

Thank you, Paula! 🥇

@lchrusciel lchrusciel mentioned this pull request Nov 18, 2021
Merged
Zales0123 added a commit that referenced this pull request Nov 26, 2021
@Zales0123
minor #13306 [Minor][Behat] Style and type refactor (lchrusciel)
851b3de 
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 | #13287 (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
-------

1001bb1 [Minor][Behat] Style and type refactor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Roshyo Roshyo Roshyo requested changes

@lchrusciel lchrusciel lchrusciel approved these changes

@GSadee GSadee GSadee approved these changes

@Zales0123 Zales0123 Awaiting requested review from Zales0123

Assignees
No one assigned
Labels
API APIs related issues and PRs. Feature New feature proposals.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@paullla @GSadee @Zales0123 @lchrusciel @Roshyo