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

[Attribute] Configuration validation of text and select attributes #8755

Merged
merged 3 commits into from
Oct 10, 2017
Merged

[Attribute] Configuration validation of text and select attributes #8755

merged 3 commits into from
Oct 10, 2017

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented Oct 2, 2017

Q A
Branch? 1.0
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets fixes #8671
License MIT

@GSadee GSadee added the Bug Fix label Oct 2, 2017
@GSadee GSadee added this to the 1.0 milestone Oct 2, 2017
@GSadee GSadee changed the title [Attribute] Configuration validation of a text and select attribute [Attribute] Configuration validation of text and select attributes Oct 2, 2017
@GSadee GSadee force-pushed the attribute-configuration-validation branch from 102a5d2 to 5a8b52b Compare October 2, 2017 11:25
Zales0123
Zales0123 approved these changes Oct 10, 2017
View reviewed changes
src/Sylius/Behat/Context/Ui/Admin/ManagingProductAttributesContext.php Outdated
* @When I specify its min length as :min
* @When I specify its min entries value as :min
*/
public function iSpecifyItsMinValueAs(int $min)
Copy link
Member

Choose a reason for hiding this comment

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

: void R.I.P [*] 😢

@GSadee GSadee force-pushed the attribute-configuration-validation branch from 5a8b52b to 592dfa2 Compare October 10, 2017 06:30
pamil
pamil reviewed Oct 10, 2017
View reviewed changes
Copy link
Contributor

@pamil pamil left a comment

Choose a reason for hiding this comment

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

Let's add finals on constraints and it's good to go! :)

src/Sylius/Bundle/AttributeBundle/Validator/Constraints/ValidSelectAttributeConfiguration.php Outdated

use Symfony\Component\Validator\Constraint;

class ValidSelectAttributeConfiguration extends Constraint
Copy link
Contributor

Choose a reason for hiding this comment

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

final?

src/Sylius/Bundle/AttributeBundle/Validator/Constraints/ValidTextAttributeConfiguration.php Outdated

use Symfony\Component\Validator\Constraint;

class ValidTextAttributeConfiguration extends Constraint
Copy link
Contributor

Choose a reason for hiding this comment

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

final?

@GSadee GSadee force-pushed the attribute-configuration-validation branch from 592dfa2 to 7745533 Compare October 10, 2017 12:59
@pamil
Copy link
Contributor

pamil commented Oct 10, 2017

Since that specs have been made final, we should replace creating mocks of them with using these concrete classes in validator specifications.

@GSadee GSadee force-pushed the attribute-configuration-validation branch from 7745533 to 87a18ca Compare October 10, 2017 13:41
@GSadee
Copy link
Member Author

GSadee commented Oct 10, 2017

@pamil fixed 👍

@pamil pamil merged commit 32464f9 into Sylius:1.0 Oct 10, 2017
@pamil
Copy link
Contributor

pamil commented Oct 10, 2017

Thanks @GSadee! 🎉

@GSadee GSadee deleted the attribute-configuration-validation branch October 10, 2017 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zales0123 Zales0123 Zales0123 approved these changes

@pamil pamil pamil approved these changes

@pjedrzejewski pjedrzejewski pjedrzejewski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@GSadee @pamil @pjedrzejewski @Zales0123