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

Fix address validation having a country w/o states #3763

Merged
merged 1 commit into from
Sep 24, 2020

Conversation

cedum
Copy link
Contributor

@cedum cedum commented Sep 18, 2020

This is an extract from #3129 and addresses the concern having a separate commit fixing #3098 and enabling the stores to backport the fix more easily without being forced to upgrade.

This mainly fixes the address' state validation bug when switching from a country with states (regions) to one without.
Before validating that the associated state belongs to the associated country, ensure the country does have states; if it doesn't, nullify the state_id since we don't need one here.

This is quite a tricky bug that has been partially introduced when the validate_state_matches_country validation method has been merged.
Note: both state validation methods are being deprecated in #3129 and partially refactored into a configurable validation class allowing to override the default behavior and handling any region-based edge case around the address' state normalization and validation logic.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

This fixes the address' state validation when switching from a country
having states to one without.
Before validating that the associated state belongs to the associated
country, ensure the country does have states; if it doesn't, just
nullify the state.
@cedum cedum force-pushed the fix-address-country-no-states branch from 12a9bf6 to 23402ea Compare September 18, 2020 16:27
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

👍 thanks Dumitru!

@kennyadsl kennyadsl merged commit 00fe22d into solidusio:master Sep 24, 2020
@kennyadsl kennyadsl deleted the fix-address-country-no-states branch September 24, 2020 09:17
@kennyadsl
Copy link
Member

@cedum if I understand correctly we can partially revert this PR now? I mean, at least the JS and HTML parts that look like hacks to fix the same issue client side.

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.

3 participants