-
Notifications
You must be signed in to change notification settings - Fork 19
Add free-plugin foundation for Manual Issues feature #1803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: william/pro-668-extra-data-column-and-collection
Are you sure you want to change the base?
Changes from all commits
372a039
83d7bc3
4798148
a5dd310
51ba634
c530a29
e6523e9
a0ce7d1
912ffd4
c1f2169
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,7 @@ public function edac_update_database() { | |
| ruletype text NOT NULL, | ||
| object mediumtext NOT NULL, | ||
| extra_data text NULL, | ||
| source text NULL, | ||
| recordcheck mediumint(9) NOT NULL, | ||
| created timestamp NOT NULL default CURRENT_TIMESTAMP, | ||
| user bigint(20) NOT NULL, | ||
|
|
@@ -92,6 +93,15 @@ public function edac_update_database() { | |
|
|
||
| // 1.0.8: Added extra_data column. dbDelta() handles ADD COLUMN automatically | ||
| // when the column appears in the CREATE TABLE DDL above; no data migration required. | ||
|
|
||
| // 1.0.9: Added source column. Backfill existing rows to 'automated' since text | ||
| // columns cannot carry a DB-level DEFAULT in MySQL 5.7. Also grant plugin | ||
| // capabilities here so existing users who update (rather than reactivate) | ||
| // receive them — register_activation_hook does not fire on plugin updates. | ||
| if ( version_compare( $db_version, '1.0.9', '<' ) ) { | ||
| $this->migrate_source_column( $table_name ); | ||
| $this->grant_plugin_capabilities(); | ||
| } | ||
| } | ||
|
|
||
| // Update database version option. | ||
|
|
@@ -122,6 +132,52 @@ private function migrate_license_key_to_shared_option() { | |
| delete_option( 'edac_license_key' ); | ||
| } | ||
|
|
||
| /** | ||
| * Backfill existing rows so every row has source = 'automated'. | ||
| * | ||
| * The source column is declared NULL in the CREATE TABLE DDL so dbDelta can add it | ||
| * to existing tables without a DEFAULT. PHP always writes the value explicitly on | ||
| * insert, but rows created before 1.0.9 need a one-time backfill. | ||
| * | ||
| * @since x.x.x | ||
| * @param string $table_name The full table name including prefix. | ||
| * @return void | ||
| */ | ||
| private function migrate_source_column( string $table_name ): void { | ||
| global $wpdb; | ||
| // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- One-time migration query. | ||
| $wpdb->query( | ||
| $wpdb->prepare( | ||
| 'UPDATE %i SET source = %s WHERE source IS NULL', | ||
| $table_name, | ||
| 'automated' | ||
| ) | ||
| ); | ||
| } | ||
|
Comment on lines
+146
to
+156
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add the helper method private function migrate_source_column( string $table_name ): void {
global $wpdb;
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- One-time migration query.
$wpdb->query(
$wpdb->prepare(
'UPDATE %i SET source = %s WHERE source IS NULL',
$table_name,
'automated'
)
);
}
/**
* Grant manual issues capabilities to administrator and editor.
*
* @since 1.0.9
* @return void
*/
private function grant_manual_issues_capabilities(): void {
$manual_caps = [
'edac_create_manual_issues',
'edac_edit_manual_issues',
'edac_delete_manual_issues',
];
foreach ( [ 'administrator', 'editor' ] as $role_name ) {
$role = get_role( $role_name );
if ( $role ) {
foreach ( $manual_caps as $cap ) {
$role->add_cap( $cap );
}
}
}
}
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in the same commit a0ce7d1 — see reply on the companion comment above for the approach taken. |
||
|
|
||
| /** | ||
| * Grant plugin capabilities to their default roles. | ||
| * | ||
| * Called during the 1.0.9 migration so that users who update the plugin | ||
| * (rather than deactivate and reactivate) also receive the capabilities. | ||
| * Uses edac_get_plugin_capabilities() so that the pro plugin's caps — registered | ||
| * via the edac_plugin_capabilities filter at plugins_loaded — are included when | ||
| * this migration runs on admin_init. | ||
| * | ||
| * @since x.x.x | ||
| * @return void | ||
| */ | ||
| private function grant_plugin_capabilities(): void { | ||
| foreach ( edac_get_plugin_capabilities() as $cap => $roles ) { | ||
| foreach ( $roles as $role_name ) { | ||
| $role = get_role( $role_name ); | ||
| if ( $role ) { | ||
| $role->add_cap( $cap ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Migrate existing records to use selector-based unique identifiers. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,7 @@ import { computePosition, autoUpdate } from '@floating-ui/dom'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { createFocusTrap } from 'focus-trap'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { isFocusable } from 'tabbable'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { __, _n, sprintf } from '@wordpress/i18n'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { doAction, applyFilters } from '@wordpress/hooks'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { saveFixSettings } from '../common/saveFixSettingsRest'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { fillFixesModal, fixSettingsModalInit, openFixesModal } from './fixesModal'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getLandmarkType as getLandmarkTypeUtil } from './getLandmarkType'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -184,6 +185,17 @@ class AccessibilityCheckerHighlight { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Docked panel restored on page load — fetch issue data so the panel isn't empty. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.panelOpen(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Fires after the highlighter has finished initialising. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Pro plugin uses this to register its manual-issues JS module. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since x.x.x | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {AccessibilityCheckerHighlight} highlighter The highlighter instance. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| doAction( 'edac.highlighter.init', this ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| toggleMenu() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -641,6 +653,17 @@ class AccessibilityCheckerHighlight { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| document.body.insertAdjacentHTML( 'afterbegin', newElement ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const panel = document.getElementById( 'edac-highlight-panel' ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Fires after the highlighter menu is added to the DOM. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Pro plugin uses this to append additional menu items such as "Add Manual Issue". | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since x.x.x | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {HTMLElement} menuEl The `<ul role="menu">` element. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| doAction( 'edac.highlighter.menuItems', document.getElementById( 'edac-highlight-menu' ) ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Override --wp-admin-theme-color with the correct value from the user's | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // admin color scheme, since WordPress does not update this variable on the frontend. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( edacFrontendHighlighterApp?.adminThemeColor ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1348,6 +1371,18 @@ class AccessibilityCheckerHighlight { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( issueContent ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| issueContent.style.display = 'block'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Fires after the issue detail panel has been rendered. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Pro plugin uses this to append Edit / Delete buttons for manual issues. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since x.x.x | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {Object} issue The issue data object. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {HTMLElement} contentEl The `.edac-highlight-panel-controls-content-issue` element. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| doAction( 'edac.highlighter.issueDetail', matchingObj, issueContent ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
1372
to
1387
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The If Moving the
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid — fixed in commit 912ffd4. Moved |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1625,7 +1660,20 @@ class AccessibilityCheckerHighlight { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| div.innerHTML = `<span class="edac-highlight-summary-total" role="heading" aria-level="3">${ totalLabel }</span><span class="edac-highlight-summary-breakdown">${ breakdownParts.join( ' · ' ) }</span>`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Filters the summary breakdown parts in the highlighter panel footer. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Pro plugin uses this to append a "X Manual" count alongside the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Problems / Needs Review / Dismissed breakdown. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since x.x.x | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {string[]} parts Summary parts array. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {Object} counts Raw counts: errorCount, warningCount, ignoredCount. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const summaryParts = applyFilters( 'edac.highlighter.issueGroups', breakdownParts, { errorCount, warningCount, ignoredCount } ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| div.innerHTML = `<span class="edac-highlight-summary-total" role="heading" aria-level="3">${ totalLabel }</span><span class="edac-highlight-summary-breakdown">${ summaryParts.join( ' · ' ) }</span>`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new capabilities (
edac_create_manual_issues,edac_edit_manual_issues,edac_delete_manual_issues) are currently only granted inedac_activation()insideincludes/activation.php.However, in WordPress,
register_activation_hookonly runs when a plugin is explicitly activated or reactivated, and not when a plugin is updated. This means existing users who update the plugin to version1.0.9will receive the database schema updates (viaUpdate_Database::edac_update_database()), but will not have the new capabilities granted to their roles, breaking the manual issues feature for them.To ensure all existing users receive these capabilities upon updating, we should also execute the capability-granting logic within the database update/migration path when upgrading to version
1.0.9.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch — this is a real gap. Activation hook doesn't fire on updates so existing users would have missed the caps.
Fixed in commit a0ce7d1, but with a different approach than suggested: rather than a private method with hardcoded cap names, the migration calls
edac_get_plugin_capabilities()with the same loop used inedac_activation(). Since the migration runs onadmin_init(afterplugins_loaded), the pro plugin'sedac_plugin_capabilitiesfilter callback is already registered, so pro caps are included automatically. This keeps the free/pro boundary intact — the free plugin never needs to know which specific caps the pro plugin defines.