Conversation
XCFramework BuildThis PR's XCFramework is available for testing. Add to your .package(url: "https://github.com/automattic/wordpress-rs", branch: "pr-build/1210")Built from 34de769 |
jkmassel
left a comment
There was a problem hiding this comment.
I'm wondering a few things about this:
- Can we avoid using
PostStatus::Customfor this? It seems like this is basically aPostStatus::Defaultas far as the API is concerned? - It seems like
PostStatus::any ==PostStatus::Published && PostStatus::any == PostStatus::draft`, etc?
This has me wondering if the type for PostListFilter.status should be u64 and be backed by a bit set? That way you can represent a set of values without needing this extra custom type – you could just say "this should filter on PostListStatus::Draft|PostListStatus::Published"? This is a bit more ceremony, but it should be erased at runtime and I think more closely models what you're trying to do?
WDYT?
FWIW, the bot thinks there's some awkwardness:
If status contains ["any", "trash"], the "any" branch wins and trash posts are excluded — contradicting the explicit "trash" filter. In practice, the app likely never constructs this combination, but the behavior is silently wrong if it does. A comment noting this assumption would help.
The PR hardcodes trash and auto-draft as the only excluded statuses. Plugins can register custom statuses with exclude_from_search = true, which this won’t handle.
I don't know if we support custom status values or not, so I don't know how that affects this PR.
The bot also suggested more test coverage:
Tests cover publish, draft, trash, and auto-draft with “any”. Missing:
Pending, Private, Future statuses with “any” (should all match)
Empty status list (already covered by existing tests, just not with “any” context)
It also had one cool suggestion about avoiding allocation, but it might not be applicable if we end up changing anything:
This method is called from update_post_membership in a loop over all live collections on every post change. The allocations can be avoided with zero-cost pattern matching:
if self.status.iter().any(|s| matches!(s, PostStatus::Custom(st) if st == "any")) {
if post.status == PostStatus::Trash
|| matches!(&post.status, PostStatus::Custom(st) if st == "auto-draft")
{
return false;
}
}
"any" is not a real post status. It's used as a hard-coded magic string in post query. I'm not sure if we should declare it in "Default" probably means different things in different endpoints. The default status in the wp/v2/posts endpoint is published.
Not entirely. This is what the bot says:
I'm using the "any" status to show posts in the iOS app's "All" tab. I want the apps' All tab to show the same data in the "All" tab in the wp-admin. Using status=any seems to be the easiest approach. This is what the bot suggests if we really want to match the wp-admin: 🤖The wp-admin "All" tab passes no post_status to WP_Query. When post_status is empty and is_admin is true, WP_Query includes posts with:
It notably excludes trash. So on the client side, you'd:
This dynamically picks up any custom statuses registered with those properties, matching the admin "All" tab behavior.
Agreed. Probably don't need to be a bit set, as I explained above, "any" does not equal to a list of hard-coded status. I can update this PR to perform this refactor, if you think we should continue to use |
e3615bb to
34de769
Compare
|
@jkmassel Do you think we can use this PR for now. And, if needed, we can refactor later?
|
jkmassel
left a comment
There was a problem hiding this comment.
Approving to unblock – we can iterate further later.
* Support filtering by "any" status * Use PostStatus::Any
* Support filtering by "any" status * Use PostStatus::Any
* Add stats insights endpoint
Add support for the WP.com REST API v1.1 stats/insights endpoint,
which returns posting activity patterns including best hour/day to post,
hourly view breakdowns, and yearly posting summaries.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Handle empty array responses in stats insights deserialization
The WP.com API returns `[]` instead of `{}` for empty map fields
(days, hours, hourly_views). Use `deserialize_empty_array_or_hashmap`
to handle both cases gracefully.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Handle integer values in map deserialization for stats insights
The WP.com API returns `0` instead of `{}` for empty map fields
(days, hours, hourly_views) on some sites. Extend
`deserialize_empty_array_or_hashmap` to also accept integers,
treating them as empty HashMaps.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Make map deserializer handle all non-map placeholder values
The WP.com API can return various non-map values (0, null, false, [])
for map fields on sites with no data. Make deserialize_empty_array_or_hashmap
handle all of them by adding visit_bool, visit_none, visit_unit, and
visit_f64 handlers, returning empty HashMap for any non-map value.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Rewrite map deserializers to use serde_json::Value intermediate
Replace the visitor-based approach with a simpler Value-based strategy
that first deserializes any JSON value, then only converts JSON objects
into HashMaps. All non-object values (integers, null, false, arrays,
strings, etc.) are treated as empty map / None. This is more robust
against unexpected API response formats.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Add stats summary endpoint
Add GET /rest/v1.1/sites/<site_id>/stats endpoint that returns
aggregate site statistics and recent visit time-series data.
Reuses StatsVisitsResponse for the visits field. Supports
locale query parameter.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Add locale parameter to stats insights endpoint
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Bump fastlane-plugin-wpmreleasetoolkit from 14.1.0 to 14.2.0 (#1223)
Bumps [fastlane-plugin-wpmreleasetoolkit](https://github.com/wordpress-mobile/release-toolkit) from 14.1.0 to 14.2.0.
- [Release notes](https://github.com/wordpress-mobile/release-toolkit/releases)
- [Changelog](https://github.com/wordpress-mobile/release-toolkit/blob/trunk/CHANGELOG.md)
- [Commits](wordpress-mobile/release-toolkit@14.1.0...14.2.0)
---
updated-dependencies:
- dependency-name: fastlane-plugin-wpmreleasetoolkit
dependency-version: 14.2.0
dependency-type: direct:production
update-type: version-update:semver-minor
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Bump fluent-templates from 0.13.2 to 0.13.3 (#1222)
Bumps [fluent-templates](https://github.com/XAMPPRocky/fluent-templates) from 0.13.2 to 0.13.3.
- [Release notes](https://github.com/XAMPPRocky/fluent-templates/releases)
- [Changelog](https://github.com/XAMPPRocky/fluent-templates/blob/master/CHANGELOG.md)
- [Commits](XAMPPRocky/fluent-templates@fluent-templates-v0.13.2...fluent-templates-v0.13.3)
---
updated-dependencies:
- dependency-name: fluent-templates
dependency-version: 0.13.3
dependency-type: direct:production
update-type: version-update:semver-patch
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Fix unsafe row indexing and remove unnecessary Hash derives in stats endpoints (#1218)
Address two issues flagged in PR #1200 review:
1. Fix unsafe row indexing in stats_visits.rs: The get_stats_data() function
hard-coded row[0] for period and used unwrap() on position(). Now it safely
looks up both "period" and data field indices, using .get() for bounds-safe
access instead of panicking on out-of-bounds indices.
2. Remove Hash derive from all stats types: Hash was blindly copied across
12 stats Period/Unit enums and 8 DataPoint/Response types in stats_visits.rs,
but none are used as HashMap/HashSet keys. Removed Hash from:
- StatsVisitsUnit, StatsClicksPeriod, StatsReferrersPeriod, StatsSearchTermsPeriod,
StatsTopAuthorsPeriod, StatsTopPostsPeriod, StatsCountryViewsPeriod,
StatsRegionViewsPeriod, StatsCityViewsPeriod, StatsDevicesPeriod,
StatsFileDownloadsPeriod, StatsVideoPlaysPeriod
- StatsVisitsResponse, StatsVisitsDataPoint, StatsVisitorsDataPoint,
StatsLikesDataPoint, StatsReblogsDataPoint, StatsCommentsDataPoint,
StatsPostsDataPoint, StatsVisitsDataValue
All 1256 unit tests pass.
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* Bump chrono from 0.4.43 to 0.4.44 (#1221)
Bumps [chrono](https://github.com/chronotope/chrono) from 0.4.43 to 0.4.44.
- [Release notes](https://github.com/chronotope/chrono/releases)
- [Changelog](https://github.com/chronotope/chrono/blob/main/CHANGELOG.md)
- [Commits](chronotope/chrono@v0.4.43...v0.4.44)
---
updated-dependencies:
- dependency-name: chrono
dependency-version: 0.4.44
dependency-type: direct:production
update-type: version-update:semver-patch
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Bump proc-macro-crate from 3.4.0 to 3.5.0 (#1225)
Bumps [proc-macro-crate](https://github.com/bkchr/proc-macro-crate) from 3.4.0 to 3.5.0.
- [Release notes](https://github.com/bkchr/proc-macro-crate/releases)
- [Commits](bkchr/proc-macro-crate@v3.4.0...v3.5.0)
---
updated-dependencies:
- dependency-name: proc-macro-crate
dependency-version: 3.5.0
dependency-type: direct:production
update-type: version-update:semver-minor
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Bump quote from 1.0.44 to 1.0.45 (#1224)
Bumps [quote](https://github.com/dtolnay/quote) from 1.0.44 to 1.0.45.
- [Release notes](https://github.com/dtolnay/quote/releases)
- [Commits](dtolnay/quote@1.0.44...1.0.45)
---
updated-dependencies:
- dependency-name: quote
dependency-version: 1.0.45
dependency-type: direct:production
update-type: version-update:semver-patch
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Add a migration script to ensure custom terms are re-fetched (#1217)
* Fix loading trashed post by ids (#1226)
* Add integration tests for fetching trashed posts by id
* Fix loading trashed post by ids
* Add PostStatus::Any
* Support filtering by "any" status (#1210)
* Support filtering by "any" status
* Use PostStatus::Any
* Add stats tags endpoint
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Revert changes to deserialize_empty_array_or_hashmap helpers
Restores the original Visitor-based implementation. These helpers
don't need to be modified for the stats tags endpoint.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tony Li <tony.li@automattic.com>
Description
The app uses the "any" status in the "All" tab. Support "any" status so that the "All" tab displays content correctly after updates.