Skip to content

Add error count summary to plugin check UI #1180

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.

Sign up for GitHub

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

Open
faisalahammad wants to merge 8 commits into
base: trunk
Choose a base branch
from
+90 −3
Open

Add error count summary to plugin check UI #1180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4fdc53e
Add error count summary to plugin check UI
faisalahammad Feb 12, 2026
31179bf
Fix lint errors in plugin-check-admin.js
faisalahammad Feb 19, 2026
2e123c2
Revert unrelated changes in package-lock.json
faisalahammad Mar 4, 2026
2625b54
Exclude package-lock.json from PR
faisalahammad Mar 4, 2026
cba93e1
Update version number to 1.9.0 in countResults() comment
faisalahammad Mar 5, 2026
7ada770
Localize error and warning count strings in JS
faisalahammad Mar 5, 2026
d592676
Fix plural JS translation warnings
faisalahammad Mar 6, 2026
4003cb1
Revert JS refactoring, use simple __() with translator comments in PHP
faisalahammad Mar 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 82 additions & 3 deletions assets/js/plugin-check-admin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,46 @@
return false;
}

/**
* Counts the total number of errors and warnings in the aggregated results.
*
* @since 1.9.0
*
* @return {Object} Object with errorCount and warningCount properties.
*/
function countResults() {
let errorCount = 0;
let warningCount = 0;

// Count errors.
for ( const file of Object.keys( aggregatedResults.errors ) ) {
const lines = aggregatedResults.errors[ file ] || {};

for ( const line of Object.keys( lines ) ) {
const columns = lines[ line ] || {};

for ( const column of Object.keys( columns ) ) {
errorCount += ( columns[ column ] || [] ).length;
}
}
}

// Count warnings.
for ( const file of Object.keys( aggregatedResults.warnings ) ) {
const lines = aggregatedResults.warnings[ file ] || {};

for ( const line of Object.keys( lines ) ) {
const columns = lines[ line ] || {};

for ( const column of Object.keys( columns ) ) {
warningCount += ( columns[ column ] || [] ).length;
}
}
}

return { errorCount, warningCount };
}

function defaultString( key ) {
if (
pluginCheck.strings &&
Expand Down Expand Up @@ -545,9 +585,48 @@
*/
function renderResultsMessage( isSuccessMessage ) {
const messageType = isSuccessMessage ? 'success' : 'error';
const messageText = isSuccessMessage
? pluginCheck.successMessage
: pluginCheck.errorMessage;
let messageText;

if ( isSuccessMessage ) {
messageText = pluginCheck.successMessage;
} else {
Comment on lines 586 to +592
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

messageType is derived only from isSuccessMessage, so a warnings-only run will still render notice-error even when errorCount is 0 (but warningCount > 0). Consider deriving the notice type from the counted totals (e.g., success when both 0, warning when only warnings, error when errors > 0) so the notice styling matches the actual result severity.

Copilot uses AI. Check for mistakes.
// Count errors and warnings.
const { errorCount, warningCount } = countResults();

// Build the message with counts.
let errorPart = '';
if ( errorCount > 0 ) {
errorPart =
errorCount === 1
? pluginCheck.errorString.replace( '%d', errorCount )
: pluginCheck.errorsString.replace( '%d', errorCount );
}

let warningPart = '';
if ( warningCount > 0 ) {
warningPart =
warningCount === 1
? pluginCheck.warningString.replace(
'%d',
warningCount
)
: pluginCheck.warningsString.replace(
'%d',
warningCount
);
Comment on lines +601 to +616
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Using .replace( '%d', count ) is fragile for translations because translators may use positional placeholders like %1$d (or multiple placeholders), which won't be replaced. Prefer a sprintf-style formatter (e.g., wp.i18n.sprintf) or another formatting helper that supports standard printf placeholders.

Suggested change
? pluginCheck.errorString.replace( '%d', errorCount )
: pluginCheck.errorsString.replace( '%d', errorCount );
}
let warningPart = '';
if ( warningCount > 0 ) {
warningPart =
warningCount === 1
? pluginCheck.warningString.replace(
'%d',
warningCount
)
: pluginCheck.warningsString.replace(
'%d',
warningCount
);
? wp.i18n.sprintf( pluginCheck.errorString, errorCount )
: wp.i18n.sprintf( pluginCheck.errorsString, errorCount );
}
let warningPart = '';
if ( warningCount > 0 ) {
warningPart =
warningCount === 1
? wp.i18n.sprintf( pluginCheck.warningString, warningCount )
: wp.i18n.sprintf( pluginCheck.warningsString, warningCount );

Copilot uses AI. Check for mistakes.
Comment on lines +608 to +616
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Same placeholder-formatting issue here: .replace() won't handle positional placeholders (e.g. %1$d) or multiple occurrences used by some translations. Use a printf-style formatter for these localized strings to avoid broken output in non-English locales.

Copilot uses AI. Check for mistakes.
}

if ( errorPart && warningPart ) {
messageText = errorPart + ' and ' + warningPart + ' found.';
} else if ( errorPart ) {
messageText = errorPart + ' found.';
} else if ( warningPart ) {
messageText = warningPart + ' found.';
Comment on lines +619 to +624
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

The summary message concatenates hard-coded English fragments (' and ', ' found.'). This makes the overall sentence partially non-translatable and prevents languages from reordering words/clauses. Consider providing fully translatable message templates (e.g., strings for errors-only, warnings-only, both) with placeholders, rather than composing English sentence parts in JS.

Suggested change
if ( errorPart && warningPart ) {
messageText = errorPart + ' and ' + warningPart + ' found.';
} else if ( errorPart ) {
messageText = errorPart + ' found.';
} else if ( warningPart ) {
messageText = warningPart + ' found.';
// Use fully translatable summary templates with placeholders.
const summaryBothTemplate =
pluginCheck.summaryBothTemplate ||
'%1$s and %2$s found.';
const summarySingleTemplate =
pluginCheck.summarySingleTemplate || '%s found.';
if ( errorPart && warningPart ) {
messageText = summaryBothTemplate
.replace( '%1$s', errorPart )
.replace( '%2$s', warningPart );
} else if ( errorPart ) {
messageText = summarySingleTemplate.replace(
'%s',
errorPart
);
} else if ( warningPart ) {
messageText = summarySingleTemplate.replace(
'%s',
warningPart
);

Copilot uses AI. Check for mistakes.
} else {
// Fallback to default message if somehow no errors/warnings.
messageText = pluginCheck.errorMessage;
}
}

resultsContainer.innerHTML =
renderTemplate( 'plugin-check-results-complete', {
Expand Down
8 changes: 8 additions & 0 deletions includes/Admin/Admin_Page.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,14 @@ public function enqueue_scripts() {
'actionExportResults' => Admin_AJAX::ACTION_EXPORT_RESULTS,
'successMessage' => __( 'No errors found.', 'plugin-check' ),
'errorMessage' => __( 'Errors were found.', 'plugin-check' ),
/* translators: %d: Number of errors found. */
'errorString' => __( '%d error', 'plugin-check' ),
/* translators: %d: Number of errors found. */
'errorsString' => __( '%d errors', 'plugin-check' ),
/* translators: %d: Number of warnings found. */
'warningString' => __( '%d warning', 'plugin-check' ),
/* translators: %d: Number of warnings found. */
'warningsString' => __( '%d warnings', 'plugin-check' ),
'strings' => array(
'exportCsv' => __( 'Export CSV', 'plugin-check' ),
'exportJson' => __( 'Export JSON', 'plugin-check' ),
Expand Down
Loading