Add supported countries and states endpoints for domain registration#1282
Add supported countries and states endpoints for domain registration#1282
Conversation
oguzkocer
changed the title
oguzkocer
changed the title
XCFramework BuildThis PR's XCFramework is available for testing. Add to your .package(url: "https://github.com/automattic/wordpress-rs", branch: "pr-build/1282")Built from b738f36 |
| #[get(url = "/domains/supported-countries", output = Vec<SupportedCountry>)] | ||
| SupportedCountries, | ||
| #[get(url = "/domains/supported-states/<country_code>", output = Vec<SupportedState>)] | ||
| SupportedStates, |
There was a problem hiding this comment.
Both of these could be pretty trivially added to the WP.com e2e test suite for ongoing validation
oguzkocer
force-pushed
the
domain-supported-country-and-states
branch
from
cac7ec7 to
2b498e7
Compare
jkmassel
left a comment
jkmassel
left a comment
There was a problem hiding this comment.
Just one question about the CountryCode that's worth checking before this lands.
| #[derive(Debug, Clone, Serialize, Deserialize, uniffi::Record)] | ||
| pub struct SupportedCountry { | ||
| /// ISO 3166-1 alpha-2 code (e.g. `"US"`). | ||
| pub code: String, |
There was a problem hiding this comment.
Should this be CountryCode?
There was a problem hiding this comment.
This didn't work when we didn't have the enum for the divider, but should be good now. Addressed in 05815c3.
| #[serde(untagged)] | ||
| enum SupportedCountryEntry { | ||
| Country(SupportedCountry), | ||
| Divider { |
There was a problem hiding this comment.
If someone were to ever align the response type to match Country here, this would no longer match properly. We could solve it by assuming an empty code and name are the separator, but it's pretty theoretical.
This note is mostly just in case anyone ever has to come back to it down the line.
| .collect() | ||
| }; | ||
|
|
||
| let divider_pos = entries |
There was a problem hiding this comment.
If more than one separator is found, we just use the position of the first one and subsequent ones are filtered out. This seems fine.
oguzkocer
force-pushed
the
domain-supported-country-and-states
branch
from
5e76a97 to
05815c3
Compare
jkmassel
left a comment
jkmassel
left a comment
There was a problem hiding this comment.
Approved pending merge fixes, though I left a small improvement suggestion for comparison, and I'm wondering if we should reuse CountryCode
| /// Additional country codes whose tax rules apply alongside this one. | ||
| #[serde(default)] | ||
| #[uniffi(default = [])] | ||
| pub tax_country_codes: Vec<String>, |
There was a problem hiding this comment.
Should this be Vec<CountryCode>?
| let us = response | ||
| .featured | ||
| .iter() | ||
| .find(|c| c.code.0 == "US") |
There was a problem hiding this comment.
Should we implement From<&str> for CountryCode? There's a few spots like this that seem to suggest it'd be helpful
| let states = ctx | ||
| .client | ||
| .domains() | ||
| .supported_states(&CountryCode("DE".to_string())) |
There was a problem hiding this comment.
This was the other case that had me wondering about From<&str>
- Introduce `CountryCode` newtype for ISO 3166-1 alpha-2 codes - Add `SupportedCountry` and `SupportedState` types with deserialization - Wire up `GET /domains/supported-countries` and `GET /domains/supported-states/<country_code>` endpoints - Replace segments test fixtures with synthetic data
…ries The WordPress.com supported-countries API returns a flat array with a sentinel divider separating featured countries from the full list. Introduce a SupportedCountries wrapper that deserializes via an internal untagged enum (Country | Divider), splits at the sentinel, and exposes `featured` and `all` vectors. This also lets us tighten vat_supported, tax_needs_city, and tax_needs_subdivision from Option<bool> to bool since the sentinel (the only entry missing those fields) is now filtered out during deserialization.
Change tax_needs_address and tax_needs_organization from Option<bool> to bool, and tax_country_codes from Option<Vec<String>> to Vec<String>, defaulting to false/empty when absent. The three required bool fields (vat_supported, tax_needs_city, tax_needs_subdivision) still lack defaults, which is sufficient for the untagged enum to reject the divider entry.
oguzkocer
force-pushed
the
domain-supported-country-and-states
branch
from
05815c3 to
b738f36
Compare
3 participants
No description provided.