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

Tax page fixes #13162

Merged
merged 5 commits into from Apr 8, 2019
Merged

Tax page fixes #13162

merged 5 commits into from Apr 8, 2019

Conversation

zuk3975
Copy link
Contributor

@zuk3975 zuk3975 commented Apr 3, 2019

Questions Answers
Branch? develop
Description? ../PrestaShopBundle/Resources/config/routing/admin/improve/international/tax.yml was missing _legacy_parameters in admin_taxes_delete route. Added missing TypedRegexConstraint in TaxType
Type? refacto
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? Does not impact user interface

This change is Reviewable

@prestonBot prestonBot added develop Branch Refactoring Type: Refactoring labels Apr 3, 2019
@zuk3975
Copy link
Contributor Author

zuk3975 commented Apr 3, 2019

@matks one more tiny PR. Didint find anything more to fix in taxes

@matks matks added the migration symfony migration project label Apr 3, 2019
@zuk3975 zuk3975 closed this Apr 3, 2019
@zuk3975 zuk3975 reopened this Apr 3, 2019
@zuk3975 zuk3975 changed the title Add missing legacy parameters in taxes routing Tax page fixes Apr 3, 2019
@matks
Copy link
Contributor

matks commented Apr 4, 2019

@zuk3975 It's a bit outside of the scope of this PR, but could you unit test class TypedRegexValidator 😃 ?

mickaelandrieu
mickaelandrieu previously approved these changes Apr 4, 2019
@mickaelandrieu
Copy link
Contributor

It's a bit outside of the scope of this PR, but could you unit test class TypedRegexValidator ?

I had the same feeling 😆

@zuk3975
Copy link
Contributor Author

zuk3975 commented Apr 4, 2019

@zuk3975 It's a bit outside of the scope of this PR, but could you unit test class TypedRegexValidator ?

@matks added some unit tests.
I tried to avoid using dataProviders(as i was suggested for performance) and do simple foreach instead, but I couldnt due to extended ConstraintValidatorTestCase which is keeping violations in context and is restricted to get exactly 1 violation (not sure why though) - tests teared apart once it reached at least 2 violations. So I couldn't find proper way to reset violations and stayed with dataProviders.

@matks
Copy link
Contributor

matks commented Apr 4, 2019

@zuk3975 It's a bit outside of the scope of this PR, but could you unit test class TypedRegexValidator ?

@matks added some unit tests.
I tried to avoid using dataProviders(as i was suggested for performance) and do simple foreach instead, but I couldnt due to extended ConstraintValidatorTestCase which is keeping violations in context and is restricted to get exactly 1 violation (not sure why though) - tests teared apart once it reached at least 2 violations. So I couldn't find proper way to reset violations and stayed with dataProviders.

No worries 😄 great job

matks
matks previously approved these changes Apr 4, 2019
@matks
Copy link
Contributor

matks commented Apr 5, 2019

It needs a rebase as TypedRegexConstraint is now TypedRegex

@matks matks added the Waiting for rebase Status: action required, waiting for rebase label Apr 5, 2019
@zuk3975
Copy link
Contributor Author

zuk3975 commented Apr 8, 2019

It needs a rebase as TypedRegexConstraint is now TypedRegex

@matks done :)

@matks
Copy link
Contributor

matks commented Apr 8, 2019

It needs a rebase as TypedRegexConstraint is now TypedRegex

@matks done :)

😅 unfortunately there are still traces of TypedRegexConstraint according to Travis


> @php -d date.timezone=UTC ./vendor/bin/phpunit -c tests-legacy/phpunit-admin.xml
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.
...............................................................  63 / 133 ( 47%)
.....................
Fatal error: Class 'PrestaShop\PrestaShop\Core\ConstraintValidator\Constraints\TypedRegexConstraint' not found in /home/travis/build/PrestaShop/PrestaShop/src/PrestaShopBundle/Form/Admin/Improve/International/Tax/TaxType.php on line 76

@matks matks added Waiting for author Status: action required, waiting for author feedback and removed Waiting for rebase Status: action required, waiting for rebase labels Apr 8, 2019
@zuk3975
Copy link
Contributor Author

zuk3975 commented Apr 8, 2019

It needs a rebase as TypedRegexConstraint is now TypedRegex

@matks done :)

unfortunately there are still traces of TypedRegexConstraint according to Travis


> @php -d date.timezone=UTC ./vendor/bin/phpunit -c tests-legacy/phpunit-admin.xml
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.
...............................................................  63 / 133 ( 47%)
.....................
Fatal error: Class 'PrestaShop\PrestaShop\Core\ConstraintValidator\Constraints\TypedRegexConstraint' not found in /home/travis/build/PrestaShop/PrestaShop/src/PrestaShopBundle/Form/Admin/Improve/International/Tax/TaxType.php on line 76

@matks Finally travis looks happy :D

@matks matks added this to the 1.7.6.0 milestone Apr 8, 2019
@matks matks removed the Waiting for author Status: action required, waiting for author feedback label Apr 8, 2019
@matks matks merged commit 63ce791 into PrestaShop:develop Apr 8, 2019
@matks
Copy link
Contributor

matks commented Apr 8, 2019

Thank you @zuk3975

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
develop Branch migration symfony migration project Refactoring Type: Refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants