Skip to content

Conversation

@wzieba
Copy link
Contributor

@wzieba wzieba commented Dec 18, 2025

Related: AINFRA-542: Migrate MediaModel

Description

This PR removes unused code related to MediaModel.

Test Steps

Smoke test browsing and uploading images to products. Please test both WPCOM and self-hosted connections. And actually maybe something else that you know should be tested when working with media.

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@dangermattic
Copy link
Collaborator

dangermattic commented Dec 18, 2025

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 18, 2025

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App NameWooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commitadfa1e6
Direct Downloadwoocommerce-wear-prototype-build-pr15130-adfa1e6.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 18, 2025

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App NameWooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commitadfa1e6
Direct Downloadwoocommerce-prototype-build-pr15130-adfa1e6.apk

@codecov-commenter
Copy link

codecov-commenter commented Dec 18, 2025

Codecov Report

❌ Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.65%. Comparing base (4cfbab7) to head (adfa1e6).

Files with missing lines Patch % Lines
...n/com/woocommerce/android/media/FileUploadUtils.kt 0.00% 10 Missing ⚠️
...uxc/network/rest/wpcom/media/MediaResponseUtils.kt 0.00% 3 Missing ⚠️
...xc/network/rest/wpapi/media/MediaWPRESTResponse.kt 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #15130      +/-   ##
============================================
+ Coverage     38.64%   38.65%   +0.01%     
+ Complexity    10456    10455       -1     
============================================
  Files          2181     2181              
  Lines        123905   123860      -45     
  Branches      17100    17094       -6     
============================================
- Hits          47877    47876       -1     
+ Misses        71205    71160      -45     
- Partials       4823     4824       +1     

☔ 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.

@wzieba wzieba force-pushed the remove_media_model_properties branch from 753d1e2 to adfa1e6 Compare December 23, 2025 15:51
*
* @return filename with extension, or null if no extension could be determined
*/
private fun ensureFilenameHasExtension(filename: String, mimeType: String): String? {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refactored this to a method for improved readability as fileExtension was no longer used.

@wzieba wzieba added the type: technical debt Represents or solves tech debt of the project. label Dec 23, 2025
@wzieba wzieba requested a review from Copilot December 23, 2025 16:16
Copy link
Contributor

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 removes unused properties and methods from the MediaModel class and related code to clean up technical debt. The changes include removing video-specific properties (VideoPress GUID, dimensions, length), legacy properties (horizontal/vertical alignment, featured flags), and alternative image size URLs that were no longer being used.

Key changes:

  • Removed 20+ unused fields and their getters/setters from MediaModel
  • Updated constructors across the codebase to match the simplified MediaModel signature
  • Removed getUrlForSiteVideoWithVideoPressGuid() method from MediaStore
  • Updated tests to reflect the simplified model

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
MediaModel.java Removed unused properties (authorId, guid, fileExtension, width, height, length, videoPressGuid, fileUrl*Size fields, and legacy alignment/featured properties) and their accessors; simplified constructors and equals() method
MediaStore.java Removed unused getUrlForSiteVideoWithVideoPressGuid() method
MediaXMLRPCClient.java Updated getMediaFromXmlrpcResponse() to use simplified MediaModel constructor without removed parameters
MediaWPComRestResponse.java Removed unused fields from response model (author_ID, guid, extension, width, height, length, videopress fields, and thumbnail size URLs)
MediaRestClient.java Updated to pass localSiteId to getMediaFromRestResponse() and removed unused response parsing call
MediaResponseUtils.kt Updated getMediaFromRestResponse() to accept localSiteId parameter and use simplified MediaModel constructor
MediaWPRESTResponse.kt Removed unused fields from response model and updated toMediaModel() extension function to use simplified constructor
FileUploadUtils.kt Refactored filename extension handling into separate method and updated MediaModel constructor call
MediaTestUtils.java Updated test utility to calculate extension locally instead of storing it in MediaModel
MediaStoreTest.java Removed test for getUrlForSiteVideoWithVideoPressGuid() as the method was deleted
MediaSqlUtilsTest.java Updated test to use simplified MediaModel constructor without removed fields

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wzieba wzieba requested a review from kidinov December 23, 2025 16:27
@wzieba wzieba marked this pull request as ready for review December 23, 2025 16:27
@wzieba wzieba added this to the 23.9 milestone Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: technical debt Represents or solves tech debt of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants