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

Update known PR zip prefixes and allow zone lookup by partial zip if country hides provinces #332

Merged
merged 4 commits into from
Feb 24, 2025

Conversation

rochlefebvre
Copy link
Contributor

@rochlefebvre rochlefebvre commented Feb 24, 2025

What are you trying to accomplish?

We're updating the list of Portugal zip prefixes to be less stale. The updates in this PR are accurate as of Februrary 2023: still stale, but better.

Also, some wallet pay providers only provide the first part of an address postal code and we still need to look up info on their hidden zone metadata. We're updating Region#zone(zip:) to accept an incomplete zip and resolve the first zone that contains this zip, as long as the country hides provinces from addresses.

Checklist

  • I have added a CHANGELOG entry for this change (or determined that it isn't needed)

@rochlefebvre rochlefebvre force-pushed the update-zip-prefix-handling branch from 9020c42 to 496b390 Compare February 24, 2025 14:51
@rochlefebvre rochlefebvre marked this pull request as ready for review February 24, 2025 15:29
@rochlefebvre rochlefebvre requested a review from a team February 24, 2025 15:29
@rochlefebvre rochlefebvre added the #gsd:43663 Deduplicate country metadata label Feb 24, 2025
@@ -494,6 +499,13 @@ def province_optional?
province_optional || !@zones&.any?(&:province?)
end

# Returns true if this country has zones defined, and has postal code prefix data for the zones
def has_zip_prefixes?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am making this method public.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should add this to the changelog

@@ -57,6 +57,56 @@ class RegionDataConsistencyTest < ActiveSupport::TestCase
end
end

test "If a country has zip_prefixes, then every province has at least one prefix" do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two consistency suites were updated with tests that did not make it into the Worldwide gem's initial release.

Copy link
Contributor

@gabypancu gabypancu left a comment

Choose a reason for hiding this comment

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

Looks good!

Also, some wallet pay providers only provide the first part of an address postal code and we still need to look up info on their hidden zone metadata.

Just curious, where did you find this info about wallets?

next if country.zones.blank?
next unless country.zones.any? { |zone| zone.neighbours.present? }

# Fix `neighboring_zones` data in India and Thailand
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a TODO? Are we missing some neighbouring zones?

@@ -494,6 +499,13 @@ def province_optional?
province_optional || !@zones&.any?(&:province?)
end

# Returns true if this country has zones defined, and has postal code prefix data for the zones
def has_zip_prefixes?
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add this to the changelog

@rochlefebvre rochlefebvre force-pushed the update-zip-prefix-handling branch from 496b390 to dce8836 Compare February 24, 2025 16:08
@rochlefebvre rochlefebvre merged commit 05883ab into main Feb 24, 2025
10 checks passed
@rochlefebvre rochlefebvre deleted the update-zip-prefix-handling branch February 24, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
#gsd:43663 Deduplicate country metadata
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants