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

[Channel] Add not blank validation for channel default currency #5592

Conversation

Arminek
Copy link
Contributor

@Arminek Arminek commented Jul 25, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Related tickets #5565
License MIT

@@ -86,6 +86,14 @@ public function chooseDefaultTaxZone($taxZone)
/**
* {@inheritdoc}
*/
public function chooseDefaultLocale($locale)
{
// $this->getElement('default_locale')->selectOption($locale);
Copy link
Member

Choose a reason for hiding this comment

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

Why did you leave it?

@Arminek Arminek force-pushed the not-nullable-default-currency-on-channel branch 5 times, most recently from 799b87b to f4cedce Compare July 25, 2016 09:33
@Arminek Arminek changed the title [Channel] Add not blank validation for channel default locale [Channel] Add not blank validation for channel default currency Jul 25, 2016
@Arminek Arminek force-pushed the not-nullable-default-currency-on-channel branch from f4cedce to 010c03a Compare July 25, 2016 10:52
@@ -13,7 +13,7 @@ Feature: Channel validation
When I name it "Mobile channel"
But I do not specify its code
And I try to add it
Then I should be notified that code is required
Then I should be notified that "code" is required
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? 👎

@Arminek Arminek force-pushed the not-nullable-default-currency-on-channel branch 3 times, most recently from 5943b0a to 2ccde64 Compare July 26, 2016 07:51
@@ -6,12 +6,14 @@ Feature: Adding a new channel

Background:
Given I am logged in as an administrator
And the store has currency "Euro"
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the store has currency "Euro"
And I am logged in as an administrator

At the beginning we should configure shop and login after everything is setup (we follow this convention everywhere).

@pamil
Copy link
Contributor

pamil commented Jul 26, 2016

👍, the same will need to be done for channel's default locale

@Arminek Arminek force-pushed the not-nullable-default-currency-on-channel branch from 2ccde64 to d740ad6 Compare July 26, 2016 09:58
@michalmarcinkowski michalmarcinkowski merged commit 1de9418 into Sylius:master Jul 26, 2016
@michalmarcinkowski
Copy link
Contributor

Good job! 👍

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.

5 participants