Skip to content

Conversation

@adalpari
Copy link
Contributor

Description

Added file size display functionality to the media details screens. The file size is now shown in a human-readable format (KB, MB, GB) in MediaSettingsActivity.

This new functionality has been added only to site logged in with Application Password. Will add it to WP sites in the next iteration.

Testing instructions

  1. Log into a site WITHOUT Application Password
  2. Go to Media -> Open any of the media file
  • Verify you don't see the filesize nor the app crashes
  1. Log into a site with Application Password
  2. Go to Media -> Open any of the media file
  • Verify you DO see the filesize
Screen.Recording.2025-09-17.at.12.43.07.mov

@adalpari adalpari requested a review from Copilot September 17, 2025 10:50
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 adds file size display functionality to media details screens, showing file sizes in human-readable format (KB, MB, GB) for sites using Application Password authentication.

  • Adds file size field to MediaModel and database schema
  • Implements file size parsing from media API responses for all media types
  • Adds UI components to display formatted file size in MediaSettingsActivity

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
WellSqlConfig.kt Database migration to add FILE_SIZE column to MediaModel table
MediaRSApiRestClient.kt Parse and populate file size from API responses for all media types
MediaModel.java Add file size field, getter/setter methods, and equality comparison
strings.xml Add file size caption string resource
media_settings_activity.xml Add UI layout for file size display with label and value
MediaSettingsActivity.java Display formatted file size with conditional visibility logic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +652 to +658
final String[] units = new String[] {
getString(R.string.file_size_in_bytes),
getString(R.string.file_size_in_kilobytes),
getString(R.string.file_size_in_megabytes),
getString(R.string.file_size_in_gigabytes),
getString(R.string.file_size_in_terabytes)
};
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

The code references string resources (file_size_in_bytes, file_size_in_kilobytes, etc.) that are not defined in the strings.xml file. This will cause a compilation error or runtime crash.

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.

These strings are defined in the strings file...

Image

Copy link
Member

Choose a reason for hiding this comment

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

An aside: is it typical to use lowercase for kB, but uppercase for MB, GB, TB? The divergence seems odd to me.

Copy link
Member

Choose a reason for hiding this comment

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

The lowercase kB is even more odd now that I note that the server-provided values for WPCOM sites in #22220 return KB. I suggest we update the string from kBKB. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I'll update it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dangermattic
Copy link
Collaborator

dangermattic commented Sep 17, 2025

1 Warning
⚠️ strings.xml files should only be updated on release branches, when the translations are downloaded by our automation.

Generated by 🚫 Danger

}

209 -> {
db.execSQL("ALTER TABLE MediaModel ADD FILE_SIZE INTEGER")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I tested how the library creates the DB column, it looks like even it being a long variable, WellSql uses an INTEGER. So, I've just followed the approach to avoid side problems.

Copy link
Member

Choose a reason for hiding this comment

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

What is the "library" in this context?

Copy link
Contributor Author

@adalpari adalpari Sep 18, 2025

Choose a reason for hiding this comment

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

WellSql. The library used to manage MySql DB.

@adalpari adalpari marked this pull request as ready for review September 17, 2025 10:56
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 17, 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
Versionpr22216-03b2deb
Commit03b2deb
Direct Downloadwordpress-prototype-build-pr22216-03b2deb.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 17, 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
Versionpr22216-03b2deb
Commit03b2deb
Direct Downloadjetpack-prototype-build-pr22216-03b2deb.apk
Note: Google Login is not supported on these builds.

@codecov
Copy link

codecov bot commented Sep 17, 2025

Codecov Report

❌ Patch coverage is 38.46154% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.22%. Comparing base (4560f1f) to head (03b2deb).
⚠️ Report is 3 commits behind head on trunk.

Files with missing lines Patch % Lines
...c/network/rest/wpapi/media/MediaRSApiRestClient.kt 16.66% 3 Missing and 2 partials ⚠️
...rdpress/android/fluxc/persistence/WellSqlConfig.kt 33.33% 2 Missing ⚠️
.../org/wordpress/android/fluxc/model/MediaModel.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #22216   +/-   ##
=======================================
  Coverage   39.22%   39.22%           
=======================================
  Files        2164     2164           
  Lines      103310   103310           
  Branches    15881    15881           
=======================================
  Hits        40522    40522           
  Misses      59193    59193           
  Partials     3595     3595           

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

private SeekBar mImageSizeSeekBarView;
private Spinner mAlignmentSpinnerView;
private FloatingActionButton mFabView;
private TextView mFileSizeView;

Check notice

Code scanning / Android Lint

Nullable/NonNull annotation missing on field Note

Missing null annotation
private Spinner mAlignmentSpinnerView;
private FloatingActionButton mFabView;
private TextView mFileSizeView;
private TextView mFileSizeLabelView;

Check notice

Code scanning / Android Lint

Nullable/NonNull annotation missing on field Note

Missing null annotation
Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

Testing worked well for me. 🎉 I noted that VideoPress files did not display a file size—is that expected?

VideoPress file

Image

Also, should we address the null annotations before merging?

}

209 -> {
db.execSQL("ALTER TABLE MediaModel ADD FILE_SIZE INTEGER")
Copy link
Member

Choose a reason for hiding this comment

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

What is the "library" in this context?

Comment on lines +652 to +658
final String[] units = new String[] {
getString(R.string.file_size_in_bytes),
getString(R.string.file_size_in_kilobytes),
getString(R.string.file_size_in_megabytes),
getString(R.string.file_size_in_gigabytes),
getString(R.string.file_size_in_terabytes)
};
Copy link
Member

Choose a reason for hiding this comment

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

An aside: is it typical to use lowercase for kB, but uppercase for MB, GB, TB? The divergence seems odd to me.

@sonarqubecloud
Copy link

@adalpari adalpari enabled auto-merge (squash) September 18, 2025 16:48
@adalpari adalpari merged commit 3e7e021 into trunk Sep 18, 2025
23 of 26 checks passed
@adalpari adalpari deleted the feature/CMM-628-Add-filesize-to-the-media-screen branch September 18, 2025 17:02
@dcalhoun dcalhoun added this to the 26.3 milestone Sep 19, 2025
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