feat: show bulk library progress stats in free settings#66
Conversation
Move attachment listing and shared progress counting into free so the Bulk Optimization upsell can display real "X of Y optimized" numbers. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a permission-gated attachment REST endpoint, shared bulk progress and savings calculations, free-tier progress and upsell UI, incremental upload statistics, updated rating notice controls, and packaging failure handling. ChangesBulk optimization progress
Rating notice controls
Project and packaging support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AdminSettings
participant BulkProgressStatsProvider
participant CimoREST
participant BulkStats
participant BulkOptimizationUpsell
AdminSettings->>BulkProgressStatsProvider: Render free-tier bulk settings
BulkProgressStatsProvider->>CimoREST: GET /cimo/v1/attachments
CimoREST-->>BulkProgressStatsProvider: Normalized attachment records
BulkProgressStatsProvider->>BulkStats: Count attachment progress
BulkStats-->>BulkProgressStatsProvider: Optimized, unoptimized, skipped, total
BulkOptimizationUpsell->>BulkProgressStatsProvider: Read shared progress
BulkOptimizationUpsell-->>AdminSettings: Render progress and pricing actions
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/admin/class-bulk-library.php`:
- Around line 47-52: Update get_all_attachments() to avoid querying and
returning the entire Media Library in one request. Add server-side pagination
with bounded query limits and an offset/page parameter, returning only the
requested attachment details while preserving the existing response contract; if
aggregate counts are needed, provide them separately rather than materializing
every attachment.
- Around line 61-65: Update the MIME-type filter in the bulk library query to
also allow attachments with an empty stored post_mime_type, so they reach
resolveAttachmentMimeType() for extension-based resolution. Preserve the
existing image/, video/, and audio/ MIME matches.
In `@src/admin/js/bulk-optimizer/bulk-optimization-upsell.js`:
- Around line 19-29: Update the attachment-loading effect around apiFetch and
the bulk optimization stats UI to track request failures separately from valid
statistics. In the catch handler, set an error state rather than resetting stats
to zero, and render a “progress unavailable” message when that state is present
while preserving normal stats rendering for successful requests and the existing
loading completion behavior.
In `@src/admin/js/bulk-optimizer/bulk-optimizer-stats.js`:
- Around line 52-65: Update the skipped-results control around onSkippedClick so
it only exposes button semantics and keyboard focusability when a handler
exists. When onSkippedClick is absent, render the skipped label as
non-interactive plain text without click or keydown behavior; preserve the
existing interactive behavior when the callback is provided.
In `@src/shared/bulk-stats.js`:
- Around line 39-41: Add descriptive text to each of the seven `@return`
annotations in the documented functions of bulk-stats.js, including the
locations listed in the comment, while preserving their existing boolean or
other return types so jsdoc/require-returns-description passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20c68216-e2ab-43cf-8713-7cf149f8d064
📒 Files selected for processing (7)
cimo.phpsrc/admin/class-bulk-library.phpsrc/admin/css/admin-page.csssrc/admin/js/bulk-optimizer/bulk-optimization-upsell.jssrc/admin/js/bulk-optimizer/bulk-optimizer-stats.jssrc/admin/js/page/admin-settings.jssrc/shared/bulk-stats.js
| public static function get_all_attachments() { | ||
| global $wpdb; | ||
|
|
||
| // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
| $attachments = $wpdb->get_results( | ||
| $wpdb->prepare( |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Avoid loading the entire Media Library into one REST response.
get_all_attachments() performs an unbounded query, materializes metadata for every result, and sends the full dataset to the browser. Large libraries can exceed PHP memory/request limits or freeze the settings page. Add pagination, or expose server-side aggregate statistics and paginate file details separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/admin/class-bulk-library.php` around lines 47 - 52, Update
get_all_attachments() to avoid querying and returning the entire Media Library
in one request. Add server-side pagination with bounded query limits and an
offset/page parameter, returning only the requested attachment details while
preserving the existing response contract; if aggregate counts are needed,
provide them separately rather than materializing every attachment.
| AND ( | ||
| p.post_mime_type LIKE %s | ||
| OR p.post_mime_type LIKE %s | ||
| OR p.post_mime_type LIKE %s | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include attachments with an empty stored MIME so extension fallback can run.
The SQL excludes rows without an image/, video/, or audio/ MIME before resolveAttachmentMimeType() sees them. Consequently, the promised extension fallback cannot recover attachments whose post_mime_type is empty.
Proposed fix
AND (
p.post_mime_type LIKE %s
OR p.post_mime_type LIKE %s
OR p.post_mime_type LIKE %s
+ OR p.post_mime_type = ''
+ OR p.post_mime_type IS NULL
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| AND ( | |
| p.post_mime_type LIKE %s | |
| OR p.post_mime_type LIKE %s | |
| OR p.post_mime_type LIKE %s | |
| ) | |
| AND ( | |
| p.post_mime_type LIKE %s | |
| OR p.post_mime_type LIKE %s | |
| OR p.post_mime_type LIKE %s | |
| OR p.post_mime_type = '' | |
| OR p.post_mime_type IS NULL | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/admin/class-bulk-library.php` around lines 61 - 65, Update the MIME-type
filter in the bulk library query to also allow attachments with an empty stored
post_mime_type, so they reach resolveAttachmentMimeType() for extension-based
resolution. Preserve the existing image/, video/, and audio/ MIME matches.
| { !! skipped && ( | ||
| <div | ||
| role="button" | ||
| tabIndex={ 0 } | ||
| className="cimo-bulk-optimizer-skipped-label" | ||
| onClick={ onSkippedClick } | ||
| onKeyDown={ e => { | ||
| if ( e.key === 'Enter' || e.key === ' ' ) { | ||
| e.preventDefault() | ||
| onSkippedClick?.() | ||
| } | ||
| } } | ||
| title={ __( 'View skipped files', 'cimo-image-optimizer' ) } | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not expose button semantics when onSkippedClick is absent.
The free upsell omits this callback, but skipped results still render a focusable “View skipped files” button that does nothing. Render plain text unless a handler exists, or disable the control explicitly.
Proposed approach
- <div
- role="button"
- tabIndex={ 0 }
+ <div
+ role={ onSkippedClick ? 'button' : undefined }
+ tabIndex={ onSkippedClick ? 0 : undefined }
className="cimo-bulk-optimizer-skipped-label"
onClick={ onSkippedClick }
onKeyDown={ e => {
+ if ( ! onSkippedClick ) {
+ return
+ }
if ( e.key === 'Enter' || e.key === ' ' ) {
e.preventDefault()
- onSkippedClick?.()
+ onSkippedClick()
}
} }
- title={ __( 'View skipped files', 'cimo-image-optimizer' ) }
+ title={ onSkippedClick ? __( 'View skipped files', 'cimo-image-optimizer' ) : undefined }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { !! skipped && ( | |
| <div | |
| role="button" | |
| tabIndex={ 0 } | |
| className="cimo-bulk-optimizer-skipped-label" | |
| onClick={ onSkippedClick } | |
| onKeyDown={ e => { | |
| if ( e.key === 'Enter' || e.key === ' ' ) { | |
| e.preventDefault() | |
| onSkippedClick?.() | |
| } | |
| } } | |
| title={ __( 'View skipped files', 'cimo-image-optimizer' ) } | |
| > | |
| { !! skipped && ( | |
| <div | |
| role={ onSkippedClick ? 'button' : undefined } | |
| tabIndex={ onSkippedClick ? 0 : undefined } | |
| className="cimo-bulk-optimizer-skipped-label" | |
| onClick={ onSkippedClick } | |
| onKeyDown={ e => { | |
| if ( ! onSkippedClick ) { | |
| return | |
| } | |
| if ( e.key === 'Enter' || e.key === ' ' ) { | |
| e.preventDefault() | |
| onSkippedClick() | |
| } | |
| } } | |
| title={ onSkippedClick ? __( 'View skipped files', 'cimo-image-optimizer' ) : undefined } | |
| > |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/admin/js/bulk-optimizer/bulk-optimizer-stats.js` around lines 52 - 65,
Update the skipped-results control around onSkippedClick so it only exposes
button semantics and keyboard focusability when a handler exists. When
onSkippedClick is absent, render the skipped label as non-interactive plain text
without click or keydown behavior; preserve the existing interactive behavior
when the callback is provided.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/admin/class-admin-notices.php`:
- Around line 302-317: Update the notice heading around
Cimo_Stats::get_additional_savings_estimate_label() so it does not claim
unoptimized images exist when the savings label is empty. Reuse the existing
$savings_label condition to show generic copy for an empty estimate, while
preserving the current unoptimized-image heading when a savings estimate is
available.
In `@src/admin/class-stats.php`:
- Around line 220-222: Align both savings estimators to the same
attachment-based unit model: update src/admin/class-stats.php lines 220-222 and
src/shared/estimate-additional-savings.js lines 33-35 to calculate savings from
unoptimized bytes or an attachment-based unoptimized count, preserving identical
results between the PHP and JavaScript implementations.
- Around line 253-274: Update update_stats_upload_optimized to make
per-attachment accounting idempotent or otherwise synchronize the option
read-modify-write so overlapping uploads cannot lose totals. Establish
last_processed_post_id from the existing scan baseline before allowing this
method to advance it, and only advance the cursor when doing so cannot skip
older unscanned metadata. Preserve totals and cursor updates across concurrent
and initially missing-option updates.
In `@src/admin/css/admin-page.css`:
- Line 96: Update the default link color declaration in the admin page
stylesheet from `#3c83f6` to the existing darker focus color or another shade that
provides at least 4.5:1 contrast against white for 13px text.
In `@src/admin/js/bulk-optimizer/use-bulk-progress-stats.js`:
- Line 18: Add descriptive return text to the JSDoc annotations for both
affected hooks, including the one near the `80` reference, while preserving
their existing return types and documenting the returned progress statistics
object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 436dd4b2-4596-4368-8fe4-6f28f8cec20d
📒 Files selected for processing (11)
src/admin/class-admin-notices.phpsrc/admin/class-admin.phpsrc/admin/class-bulk-library.phpsrc/admin/class-metadata.phpsrc/admin/class-stats.phpsrc/admin/css/admin-page.csssrc/admin/js/bulk-optimizer/bulk-optimization-upsell.jssrc/admin/js/bulk-optimizer/use-bulk-progress-stats.jssrc/admin/js/page/admin-settings.jssrc/admin/js/page/premium-savings-estimate.jssrc/shared/estimate-additional-savings.js
💤 Files with no reviewable changes (1)
- src/admin/class-admin.php
| $avg_original_kb = $original_kb / $optimized; | ||
| $unoptimized_original_kb = $avg_original_kb * $unoptimized; | ||
| $savings_kb = $unoptimized_original_kb * ( $reduction / 100 ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Align the unit basis of both savings estimators.
Bulk progress counts each full file and generated image size, while stored upload statistics primarily count and size attachments. Applying the attachment-sized average to the larger per-size count inflates the upsell estimate.
src/admin/class-stats.php#L220-L222: derive savings from unoptimized bytes or an attachment-based unoptimized count.src/shared/estimate-additional-savings.js#L33-L35: apply the same corrected unit model so PHP and JavaScript remain consistent.
📍 Affects 2 files
src/admin/class-stats.php#L220-L222(this comment)src/shared/estimate-additional-savings.js#L33-L35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/admin/class-stats.php` around lines 220 - 222, Align both savings
estimators to the same attachment-based unit model: update
src/admin/class-stats.php lines 220-222 and
src/shared/estimate-additional-savings.js lines 33-35 to calculate savings from
unoptimized bytes or an attachment-based unoptimized count, preserving identical
results between the PHP and JavaScript implementations.
| public static function update_stats_upload_optimized( $attachment_id, $original_size, $optimized_size ) { | ||
| $stats = get_option( self::OPTION_KEY ); | ||
| if ( ! is_array( $stats ) ) { | ||
| $stats = [ | ||
| 'last_processed_post_id' => 0, | ||
| 'media_optimized_num' => 0, | ||
| 'total_original_size' => 0, | ||
| 'total_optimized_size' => 0, | ||
| ]; | ||
| } | ||
|
|
||
| $stats['media_optimized_num'] = (int) ( $stats['media_optimized_num'] ?? 0 ) + 1; | ||
| $stats['total_original_size'] = (float) ( $stats['total_original_size'] ?? 0 ) + ( (int) $original_size / 1024 ); | ||
| $stats['total_optimized_size'] = (float) ( $stats['total_optimized_size'] ?? 0 ) + ( (int) $optimized_size / 1024 ); | ||
|
|
||
| // Advance the cursor so a later incremental scan does not double-count this file. | ||
| $attachment_id = (int) $attachment_id; | ||
| if ( $attachment_id > (int) ( $stats['last_processed_post_id'] ?? 0 ) ) { | ||
| $stats['last_processed_post_id'] = $attachment_id; | ||
| } | ||
|
|
||
| update_option( self::OPTION_KEY, $stats, false ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make upload-stat updates atomic and cursor advancement lossless.
Overlapping uploads can read the same option and overwrite each other, losing totals. The winning write may then advance last_processed_post_id beyond lost or previously unscanned records, preventing later scans from recovering them. Initializing the option from one new upload similarly skips older Cimo metadata.
Use idempotent per-attachment accounting or synchronize updates and establish the scan baseline before advancing the cursor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/admin/class-stats.php` around lines 253 - 274, Update
update_stats_upload_optimized to make per-attachment accounting idempotent or
otherwise synchronize the option read-modify-write so overlapping uploads cannot
lose totals. Establish last_processed_post_id from the existing scan baseline
before allowing this method to advance it, and only advance the cursor when
doing so cannot skip older unscanned metadata. Preserve totals and cursor
updates across concurrent and initially missing-option updates.
| font-size: 13px; | ||
| font-weight: 600; | ||
| line-height: 1.3; | ||
| color: #3c83f6; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a darker default link color.
#3c83f6 on white is approximately 3.6:1, below the 4.5:1 requirement for 13px text. Use the existing darker focus color or another compliant shade.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/admin/css/admin-page.css` at line 96, Update the default link color
declaration in the admin page stylesheet from `#3c83f6` to the existing darker
focus color or another shade that provides at least 4.5:1 contrast against white
for 13px text.
🤖 Pull request artifacts
|
So CI fails at the package step instead of continuing and failing later when dist/ is missing. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
Cimo_Bulk_Library+GET /cimo/v1/attachments) so free can read bulk progress without Premium.bulk-stats.js,BulkOptimizerStats) and show real “X of Y optimized” in the free Bulk Optimization upsell (non-working controls).Test plan
Made with Cursor
Summary by CodeRabbit