Conversation
moving all internal private repo development to public repo Co-authored-by: AhmarZaidi <ahmarzaidi07@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a media revisions system that allows tracking and restoring previous versions of synced media files. The implementation includes backend version management, health checks for connected brand sites before updates, REST API endpoints for version operations, and a React-based UI for version selection and restoration. The changes span PHP backend logic for version tracking and site connectivity validation, JavaScript/React components for the user interface, and CSS styling for the new version management modal.
- Backend: Version tracking metadata, health check utilities, refactored admin handlers for replace/restore operations
- Frontend: New VersionModal component, updated browser uploader with connectivity checks, API functions for version management
- Infrastructure: New REST endpoints, database cleanup updates, snackbar notifications for error handling
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| uninstall.php | Added cleanup for onemedia_sync_versions postmeta on plugin uninstall |
| languages/OneMedia.pot | Updated translation file with new strings for version management and error messages |
| inc/classes/rest/class-media-sharing.php | Added sync-attachment-versions endpoint, replaced html_entity_decode with Utils::decode_filename, added revision field to media response |
| inc/classes/rest/class-basic-options.php | Added check-sites-connected endpoint for validating brand site connectivity |
| inc/classes/plugin-configs/class-constants.php | Added ONEMEDIA_SYNC_VERSIONS_POSTMETA_KEY constant |
| inc/classes/class-utils.php | Added version tracking functions, health check for attachment brand sites, filename decoding utility |
| inc/classes/class-hooks.php | Added pre-update hooks to prevent attachment updates when sites are unreachable, refactored to use Utils::get_brand_site_api_key |
| inc/classes/class-assets.php | Added uploadNonce and allowedMimeTypes to localized script data |
| inc/classes/class-admin.php | Refactored media replacement to support version restore, added version history management, caching for sync status |
| inc/classes/brand-site/class-admin-hooks.php | Fixed error message capitalization and response code |
| assets/src/js/utils.js | Added showSnackbarNotice and formatLastUsedDate utility functions |
| assets/src/js/media-frame.js | Added isSyncAttachment function to check sync status from Backbone model, snackbar notice component, AJAX error interceptor |
| assets/src/css/main.scss | Added styles for version button and version modal |
| assets/src/css/admin.scss | Updated snackbar z-index and added selector for snackbar container |
| assets/src/components/governing-settings/VersionModal.js | New component for displaying and selecting media versions to restore |
| assets/src/components/governing-settings/SiteModal.js | Added __nextHasNoMarginBottom prop to TextControl |
| assets/src/components/constants.js | Refactored to consolidate settings exports and move constants to top-level |
| assets/src/components/api.js | Added checkIfAllSitesConnected and fetchSyncAttachmentVersions API functions |
| assets/src/admin/media-sharing/versionIcon.js | New icon component for version button |
| assets/src/admin/media-sharing/index.js | Integrated version modal, added version icon button, health check before operations |
| assets/src/admin/media-sharing/browser-uploader.js | Added health checks before media conversion and replacement operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
justlevine
left a comment
justlevine
left a comment
There was a problem hiding this comment.
I tried to only flag the things I thought we'd be unlikely to catch/invalidate after the upcoming codesharing - or more importantly would make that effort harder -, so if there's things you think are beyond the scope, we can punt.
| 'ajaxUrl' => admin_url( 'admin-ajax.php' ), | ||
| 'nonce' => wp_create_nonce( 'wp_rest' ), | ||
| 'restUrl' => esc_url( home_url( '/wp-json/' . Constants::NAMESPACE ) ), | ||
| 'uploadNonce' => wp_create_nonce( 'onemedia_upload_media' ), |
There was a problem hiding this comment.
What's the value of having a separate none from wp_rest ?
There was a problem hiding this comment.
There was a problem hiding this comment.
I think one is for ajax upload & second is for rest but wp_rest can be used for both which we can do in refactor may be or currently, your call...
There was a problem hiding this comment.
If it's in use and would take time to detangle, we can leave it as is.
Co-authored-by: Dovid Levine <david@axepress.dev>
justlevine
left a comment
justlevine
left a comment
There was a problem hiding this comment.
Last 3 open comments also seem preexisting to mediasync, deferring to you
|
@justlevine we can review them in upcoming code refactoring. |
2 participants
This PR adds code from private repo PR's which are not merged to public repo yet.
https://github.com/rtCamp/mediasync/pull/23
https://github.com/rtCamp/mediasync/pull/22
#9
Closes