fix: Overwrite thumbnails by default, persist selected custom thumbnail #1444
+53
−102
Conversation
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
🔍 WordPress Plugin Check Report
📊 Report
❌ Errors (12)📁 inc/classes/wpforms/wpforms-field-godam-record-frontend.php (1 error)
📁 inc/classes/wpforms/wpforms-field-godam-record-entry-view.php (1 error)
📁 inc/classes/wpforms/wpforms-field-godam-record-entry-edit.php (1 error)
📁 inc/classes/class-rewrite.php (1 error)
📁 inc/classes/class-elementor-widgets.php (1 error)
📁 inc/classes/everest-forms/everest-forms-field-godam-record-frontend.php (1 error)
📁 inc/classes/assets/class-ima-assets.php (1 error)
📁 inc/classes/assets/class-jetpack-form-assets.php (1 error)
📁 inc/classes/class-video-preview.php (1 error)
📁 inc/classes/lifter-lms/class-lifter-lms.php (1 error)
📁 inc/classes/fluentforms/class-form-submit.php (1 error)
📁 inc/classes/class-video-engagement.php (1 error)
|
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
missing_composer_json_file | The "/vendor" directory using composer exists, but "composer.json" file is missing. |
📁 readme.txt (2 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
mismatched_plugin_name | Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM". |
0 |
trademarked_term | The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name. |
📁 inc/classes/media-library/class-media-folder-utils.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
91 |
PluginCheck.Security.DirectDB.UnescapedDBParameter | Unescaped parameter $where_clause used in $wpdb->get_var($wpdb->prepare(\n\t\t\t\t"\n\t\t\tSELECT COUNT(DISTINCT p.ID)\n\t\t\tFROM {$wpdb->posts} p\n\t\t\tINNER JOIN {$wpdb->term_relationships} tr ON p.ID = tr.object_id\n\t\t\tINNER JOIN {$wpdb->term_taxonomy} tt ON tr.term_taxonomy_id = tt.term_taxonomy_id\n\t\t\tWHERE {$where_clause}\n\t\t",\n\t\t\t\t$query_params\n\t\t\t))\n$where_clause assigned unsafely at line 82:\n $where_clause .= ' AND p.post_mime_type LIKE %s'\n$query_params[] used without escaping.\n$mime_type used without escaping. |
📁 assets/build/css/main.css (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
EnqueuedStylesScope | This style is being loaded in all contexts. |
📁 assets/src/libs/analytics.min.js (6 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
EnqueuedScriptsScope | This script is being loaded in all frontend contexts. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/?p=1 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/?page_id=2 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/?attachment_id=6 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/?godam-video=demo-godam-video-post (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead. |
📁 assets/build/js/main.min.js (6 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
EnqueuedScriptsScope | This script is being loaded in all frontend contexts. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/?p=1 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/?page_id=2 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/?attachment_id=6 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/?godam-video=demo-godam-video-post (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead. |
🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request removes the "Overwrite video thumbnails" setting and simplifies the thumbnail management logic. The new behavior automatically overwrites thumbnails during transcoding and retranscoding, but preserves any custom-uploaded thumbnails that users have explicitly selected.
Key changes:
- Removed the
overwrite_thumbnailssetting from all frontend and backend code - Simplified thumbnail overwrite logic to check if the current thumbnail is in the custom thumbnails array
- Updated both
add_media_thumbnailsandtranscoded_thumbnails_addedmethods to use consistent logic
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pages/godam/redux/slice/media-settings.js | Removed overwrite_thumbnails from Redux initial state |
| pages/godam/components/tabs/VideoSettings/VideoThumbnails.jsx | Removed the "Over-write video thumbnails" toggle control and its associated state selector |
| inc/helpers/custom-functions.php | Removed overwrite_thumbnails from default settings array and adjusted alignment |
| inc/classes/rest-api/class-settings.php | Removed overwrite_thumbnails from default settings and sanitization logic |
| admin/godam-transcoder-functions.php | Removed the rtgodam_is_override_thumbnail helper function entirely |
| admin/class-rtgodam-transcoder-handler.php | Simplified add_media_thumbnails to preserve custom thumbnails; removed overwrite_thumbnails from default settings |
| admin/class-rtgodam-retranscodemedia.php | Updated transcoded_thumbnails_added to preserve custom thumbnails with early return pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+629
to
+635
| // rtMedia support. | ||
| update_post_meta( $post_id, '_rt_media_video_thumbnail', $first_thumbnail_url ); | ||
|
|
||
| /** | ||
| * Determines the default thumbnail behavior: | ||
| * - For newly uploaded videos: always assign the first generated thumbnail. | ||
| * - For retranscoding jobs: assign the first thumbnail only when either: | ||
| * • the overwrite option is enabled, or | ||
| * • no existing thumbnail is currently set. | ||
| */ | ||
| $current_thumbnail = get_post_meta( $post_id, 'rtgodam_media_video_thumbnail', true ); | ||
| $should_set_thumbnail = ! $is_retranscoding_job || rtgodam_is_override_thumbnail() || empty( $current_thumbnail ); | ||
|
|
||
| if ( $should_set_thumbnail ) { | ||
| // rtMedia support. | ||
| update_post_meta( $post_id, '_rt_media_video_thumbnail', $first_thumbnail_url ); | ||
| if ( class_exists( 'RTMediaModel' ) ) { | ||
| $model->update( array( 'cover_art' => $first_thumbnail_url ), array( 'media_id' => $post_id ) ); | ||
| update_activity_after_thumb_set( $media_id ); | ||
| } |
There was a problem hiding this comment.
The logic unconditionally overwrites the rtMedia thumbnail metadata (_rt_media_video_thumbnail and cover_art) without checking if the current thumbnail is a custom-uploaded one. This inconsistency means that while rtgodam_media_video_thumbnail will be preserved if it's a custom thumbnail (lines 637-644), the rtMedia-specific metadata will always be overwritten.
The custom thumbnail check (lines 637-644) should be performed before updating the rtMedia metadata (lines 630-635) to ensure consistent behavior across both thumbnail storage systems.
| max="10" | ||
| value={ videoThumbnails } | ||
| onChange={ ( value ) => setVideoThumbnails( value ) } | ||
| help={ __( `This field specifies the number of video thumbnails that will be generated by the GoDAM. To choose from the generated thumbnails for a video, go to Media > Edit > Video Thumbnails. Thumbnails are only generated when the video is first uploaded. Please enter a value between 1 and 10`, 'godam' ) } |
There was a problem hiding this comment.
The help text states "Thumbnails are only generated when the video is first uploaded", but this may be misleading since thumbnails are also regenerated during retranscoding. Consider updating this text to clarify that thumbnails are generated both on initial upload and during retranscoding, or rephrasing to accurately reflect the current behavior.
Suggested change
| help={ __( `This field specifies the number of video thumbnails that will be generated by the GoDAM. To choose from the generated thumbnails for a video, go to Media > Edit > Video Thumbnails. Thumbnails are only generated when the video is first uploaded. Please enter a value between 1 and 10`, 'godam' ) } | |
| help={ __( `This field specifies the number of video thumbnails that will be generated by the GoDAM. To choose from the generated thumbnails for a video, go to Media > Edit > Video Thumbnails. Thumbnails are generated when the video is first uploaded and may also be regenerated during retranscoding. Please enter a value between 1 and 10`, 'godam' ) } |
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.
Issue - #1433
This pull request refactors the logic for handling video thumbnail overwriting during media transcoding and retranscoding. The main change is the removal of the "overwrite thumbnails" setting and its related code, simplifying how and when thumbnails are updated. Now, the system only overwrites the primary thumbnail if it is not one of the custom-uploaded thumbnails, regardless of whether the media is being transcoded or retranscoded.
Backend Logic Simplification:
overwrite_thumbnailssetting and all related code, including its retrieval, storage, and usage in both PHP and JavaScript code. [1] [2] [3] [4] [5]add_media_thumbnailsandtranscoded_thumbnails_addedto only overwrite the primary thumbnail if it is not currently set to a custom-uploaded thumbnail, regardless of job type. [1] [2] [3]rtgodam_is_override_thumbnailhelper function, as it is no longer needed.Frontend/UI Cleanup:
Documentation and Comments:
These changes make the thumbnail overwrite behavior more predictable and reduce complexity for both users and developers.
Recording
Before Transcoding
After.T.mov
After Transcoding
Before.T.mov