Media service to support the mobile apps#1338
Merged
Merged
Conversation
…cument custom-status gap
…ndings SparseMedia opted out of PartialEq via #[WpContextualDontDerivePartialEq] because MediaDetails.payload (Box<RawValue>) can't auto-derive PartialEq. That suppression cascaded to MediaWithEditContext and broke Swift's auto-synthesis for FullEntityMediaWithEditContext. Hand-implement PartialEq/Eq/Hash on MediaDetails by raw-JSON-string comparison, mark it exported to Swift via #[uniffi::export(Eq, Hash)], drop the contextual opt-out so the contextual media types derive the traits like the post side, and extend the Swift bindings patch script to add Hashable extensions for the contextual media types (MediaDetails is a reference type so uniffi can't auto-synthesize the conformance on the Record).
Collaborator
XCFramework BuildThis PR's XCFramework is available for testing. Add to your .package(url: "https://github.com/automattic/wordpress-rs", branch: "pr-build/1338")Built from 4f70ea9 |
oguzkocer
approved these changes
May 15, 2026
Contributor
oguzkocer
left a comment
There was a problem hiding this comment.
I raised one important issue, but it's safe to address it in a subsequent PR, so I am approving to unblock.
|
|
||
| ### Changed | ||
|
|
||
| - `MediaDetails` now derives `Eq + Hash` (raw-JSON-string comparison, FIXME left in place) and is exported via `#[uniffi::export(Eq, Hash)]`; `SparseMedia`'s `#[WpContextualDontDerivePartialEq]` opt-out is removed so `MediaWithEditContext` and the generated `FullEntityMediaWithEditContext` Swift wrapper synthesize `Equatable + Hashable` |
Contributor
There was a problem hiding this comment.
I don't think there is a FIXME in the diff, and really, we shouldn't use FIXMEs. So, this is probably a left over from a Claude session.
| // `Missing`/`Stale` until the next full refresh. Failure here is | ||
| // logged but not propagated: the REST delete and local row delete | ||
| // already succeeded. | ||
| let media_list_prefix = format!("site_{:?}:edit:media:", self.db_site.row_id); |
Contributor
There was a problem hiding this comment.
This prefix logic is duplicated in create_media_metadata_collection_with_edit_context which could lead to issues if one is altered.
Extract the shared edit-context media list key prefix into a single helper so the prefix used to scrub a deleted media item from every cached list matches the prefix used when constructing a collection's ListKey. Drop the stale FIXME mention from the MediaDetails changelog entry since no FIXME was ever added to the diff.
Contributor
Author
|
@oguzkocer Thanks for the review. I have addressed your comments in a new commit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Supporting the iOS Media Library V2 work. Adds
MediaService+MediaMetadataCollectionWithEditContextmirroring the existingPostService/PostMetadataCollectionWithEditContextpattern so the iOS side can consume media through the canonical cached + paginated + observable collection path.The mutation surface is deliberately narrower than the post side: no
create_media, noupdate_media, and no membership-update path on the collection. We'll implement that when we reach that stage on the iOS app side.Changes
wp_mobile_cache:media_edit_contexttable (migration 0014),DbTable::MediaEditContext,EntityType::MediaEditContext,MediaRepository<EditContext>(mirrorsPostRepositoryminus term relationships), plus aMediaBuildertest fixture.wp_mobile:MediaListFilter(subset ofMediaListParamsminus pagination, include/exclude, and date ranges),media_list_filter_cache_key,MediaServicewith sync / fetch / state tracking, andMediaMetadataCollectionWithEditContextreusingMetadataCollectionCoreand the sharedwp_mobile_metadata_item!macro. Includes an integration test against the test server.wp_api: hand-implementPartialEq + Eq + HashonMediaDetails(string-compare on the raw JSON payload, FIXME left in place to revisit) and dropSparseMedia's#[WpContextualDontDerivePartialEq]so the contextual media types derive the traits like the post side. Without this, the regeneratedFullEntityMediaWithEditContextSwift wrapper couldn't synthesizeEquatable + Hashable.scripts/swift-bindings.sh'spatch_wp_apiis extended to appendextension MediaWith{Edit,Embed,View}Context: Hashable {}(mirrors how the post side handlesAnyJson).Test plan
cargo test -p wp_api -p wp_mobile_cache -p wp_mobilecargo test -p wp_mobile_integration_tests --test test_media_collection(needsmake test-server)make xcframework+ spot-check that the regenerated Swift wrapper exposesWpService.media(),MediaService.createMediaMetadataCollectionWithEditContext(...),MediaListFilter,MediaMetadataCollectionItem, andMediaItemState.Changelog
CHANGELOG.mdunder## [Unreleased], using the Keep a Changelog categories (Added,Changed,Deprecated,Removed,Fixed,Security). Prefix breaking changes with**BREAKING:**.