Skip to content

Conversation

adalpari
Copy link
Contributor

@adalpari adalpari commented Jul 21, 2025

Description

Depends on: Automattic/wordpress-rs#810

This PR is migrating some of the media calls of our current XML-RPC flow.
Specifically, we are now allowing single media fetching, upload, and delete media.
All of these new flows are integrated with FluxC since is the common point from where all the UI calls are centralised. I have created a new MediaClient to easily isolate the new code from the FluxC module so it will make FluxC migration easier.

Testing instructions

  1. Log into a self-hosted site with the regular credentials
  • Verify the Media list work, and you can upload and delete media as always.
  1. Enable Application Password for that site or add a new one.
  • Verify you can browse media

  • Verify you can upload media

  • Verify you can delete media

  • Verify you can upload media directly when writing a post

  • Do other smoke tests with media

Screen.Recording.2025-07-21.at.16.40.58.mov

@dangermattic
Copy link
Collaborator

dangermattic commented Jul 21, 2025

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 21, 2025

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr22036-ab0ecb1
Commitab0ecb1
Direct Downloadwordpress-prototype-build-pr22036-ab0ecb1.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 21, 2025

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr22036-ab0ecb1
Commitab0ecb1
Direct Downloadjetpack-prototype-build-pr22036-ab0ecb1.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented Jul 21, 2025

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 39.02%. Comparing base (e5bf5b4) to head (ab0ecb1).
Report is 1 commits behind head on trunk.

Files with missing lines Patch % Lines
.../org/wordpress/android/fluxc/store/MediaStore.java 0.00% 6 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #22036   +/-   ##
=======================================
  Coverage   39.02%   39.02%           
=======================================
  Files        2153     2153           
  Lines      101494   101494           
  Branches    15585    15585           
=======================================
  Hits        39613    39613           
  Misses      58384    58384           
  Partials     3497     3497           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@adalpari adalpari requested a review from Copilot July 21, 2025 14:40
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates media operations (fetch, upload, delete) from XML-RPC to WordPress REST API using the WordPress-rs client for self-hosted sites with application passwords. The changes introduce a new REST API flow alongside existing XML-RPC and WordPress.com API flows.

  • Adds new MediaRSApiRestClient for WordPress REST API operations
  • Updates MediaStore to route self-hosted REST API requests to the new client
  • Implements fetch, upload, and delete functionality with proper error handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
MediaStore.java Routes self-hosted REST API calls to new MediaRSApiRestClient
MediaRSApiRestClient.kt Implements media operations using WordPress-rs with authentication and error handling

dispatcher.dispatch(UploadActionBuilder.newUploadedMediaAction(payload))
}

private fun getWpApiClient(site: SiteModel): WpApiClient {
Copy link
Preview

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

The getWpApiClient method creates a new WpApiClient instance on every call. Consider caching the client instance per site to avoid unnecessary object creation and potential connection overhead.

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't want to cache the object because every call is associated with a specific site. Maybe we could create a map to store one client per site. I'm not sure that won't create other problems, though.
Anyway, we were advised that WpApiClient is supporting multiple instances and created to be used that way.

@wpmobilebot
Copy link
Contributor

Project dependencies changes

list
! Upgraded Dependencies
rs.wordpress.api:android:trunk-0d7174336f381588dd577e60384225fa7b9f80c4, (changed from trunk-1ae11b4e897192f5064912d201e92539eb0b3416)
rs.wordpress.api:kotlin:trunk-0d7174336f381588dd577e60384225fa7b9f80c4, (changed from trunk-1ae11b4e897192f5064912d201e92539eb0b3416)
tree
 +--- project :libs:fluxc
-|    \--- rs.wordpress.api:android:trunk-1ae11b4e897192f5064912d201e92539eb0b3416
-|         +--- com.squareup.okhttp3:okhttp:4.12.0 (*)
-|         +--- com.squareup.okhttp3:okhttp-tls:4.12.0
-|         |    +--- com.squareup.okhttp3:okhttp:4.12.0 (*)
-|         |    +--- com.squareup.okio:okio:3.6.0 -> 3.9.0 (*)
-|         |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.10 (*)
-|         +--- net.java.dev.jna:jna:5.17.0
-|         +--- rs.wordpress.api:kotlin:trunk-1ae11b4e897192f5064912d201e92539eb0b3416
-|         |    +--- com.squareup.okhttp3:okhttp:4.12.0 (*)
-|         |    +--- com.squareup.okhttp3:okhttp-tls:4.12.0 (*)
-|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
-|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 (*)
-|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 (*)
+|    \--- rs.wordpress.api:android:trunk-0d7174336f381588dd577e60384225fa7b9f80c4
+|         +--- com.squareup.okhttp3:okhttp:4.12.0 (*)
+|         +--- com.squareup.okhttp3:okhttp-tls:4.12.0
+|         |    +--- com.squareup.okhttp3:okhttp:4.12.0 (*)
+|         |    +--- com.squareup.okio:okio:3.6.0 -> 3.9.0 (*)
+|         |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.10 (*)
+|         +--- net.java.dev.jna:jna:5.17.0
+|         +--- rs.wordpress.api:kotlin:trunk-0d7174336f381588dd577e60384225fa7b9f80c4
+|         |    +--- com.squareup.okhttp3:okhttp:4.12.0 (*)
+|         |    +--- com.squareup.okhttp3:okhttp-tls:4.12.0 (*)
+|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
+|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 (*)
+|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 (*)
-\--- rs.wordpress.api:android:trunk-1ae11b4e897192f5064912d201e92539eb0b3416 (*)
+\--- rs.wordpress.api:android:trunk-0d7174336f381588dd577e60384225fa7b9f80c4 (*)

@adalpari adalpari requested a review from nbradbury July 22, 2025 11:04
@adalpari adalpari marked this pull request as ready for review July 22, 2025 11:04
@adalpari adalpari requested a review from a team as a code owner July 22, 2025 11:04
@nbradbury nbradbury self-assigned this Jul 22, 2025
val client = getWpApiClient(site)

val mediaResponse = client.request { requestBuilder ->
requestBuilder.media().retrieveWithEditContext(media.mediaId)
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 for my own understanding: if we retrieve media with an edit context, does that mean that users without edit privilege will be unable to fetch media?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. And probably @oguzkocer will better answer this question.

Here is the context:
We are not differentiating between media access when requesting it in the client right now, so I opted to ask for the less restrictive media mode like iOS seems to be doing.

Copy link
Contributor

Choose a reason for hiding this comment

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

if we retrieve media with an edit context, does that mean that users without edit privilege will be unable to fetch media?

That's correct. I think we shouldn't even show the Media page if the user doesn't have these permissions. Best way to go about this is probably to check how wp-admin looks/works for a user with subscriber role.

Copy link

Copy link
Contributor

@nbradbury nbradbury 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, nice work! :shipit:

@adalpari adalpari merged commit 301bbc8 into trunk Jul 22, 2025
30 of 32 checks passed
@adalpari adalpari deleted the feature/CMM-577-support-media-upload-and-delete-application-password branch July 22, 2025 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants