Skip to content

Extend SubscriberServiceRemote #837

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

Merged
merged 17 commits into from
May 13, 2025
Merged

Conversation

kean
Copy link
Contributor

@kean kean commented May 5, 2025

@kean kean changed the base branch from trunk to wpios-edition May 5, 2025 14:17
@kean kean force-pushed the task/add-search-subscribers branch from 050994c to 00c78cb Compare May 5, 2025 14:29
@dangermattic
Copy link
Collaborator

dangermattic commented May 5, 2025

2 Warnings
⚠️ Package.swift was changed without updating its corresponding Package.resolved. Please resolve the Swift packages in Xcode.
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@kean
Copy link
Contributor Author

kean commented May 5, 2025

WordPressKit.zip

@kean kean requested a review from crazytonyli May 7, 2025 01:49
@kean
Copy link
Contributor Author

kean commented May 7, 2025

I'll check what's failing on CI tomorrow.

@kean kean changed the title Add search to subscribers Extend SubscriberServiceRemote May 8, 2025
emailAddress = try? container.decodeIfPresent(String.self, forKey: "email_address")
dateSubscribed = try container.decode(Date.self, forKey: "date_subscribed")
isEmailSubscriptionEnabled = try container.decode(Bool.self, forKey: "is_email_subscriber")
subscriptionStatus = try? container.decodeIfPresent(String.self, forKey: "subscription_status")
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if I missed anything, but it looks like this part can be simplified by having a CodingKey-conforming enum, and let Swift synthesis the Decodable implementation for us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Swift synthesis the Decodable implementation for us?

I started with it, until I discovered that a bunch of these String fields can contain false when empty, e.g. country: false. So I replaced parsing for these with try? container.decodeIfPresent(String.self. I'm not sure if there is a generalized solution in the framework – couldn't find it after a brief search.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. Yeah, that caused some headaches in wordpress-rs too, which has been solved. We can potentially start using the WP.com API client in wordpress-rs, once that's implemented.

@kean kean merged commit 83d66fa into wpios-edition May 13, 2025
6 of 8 checks passed
@kean kean deleted the task/add-search-subscribers branch May 13, 2025 13:55
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