New.Code.Server Requirements - #731
Open
alexander-b-clean wants to merge 4 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The current warning-to-row mapping relies on substring matching translated messages (breaks in non-English locales) and checkRequirements() can accumulate duplicate warnings across calls.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces a “System/Server Requirements” check surfaced in the React admin UI (within the Summary and Support tab), backed by a new PHP requirements checker and a docs link built via the existing UTM link constructor.
Changes:
- Added PHP-side server requirements evaluation and a React-data view formatter for the “system_requirements” tab payload.
- Embedded a new React
SystemRequirementsblock into the Summary tab via a dedicatedTabSummarycomponent and portal mounting. - Added a new UTM preset for the System Requirements documentation link.
File summaries
| File | Description |
|---|---|
| lib/CleantalkSP/SpbctWP/ServerRequirementsChecker/View.php | Builds the React payload and formats requirement rows for the UI. |
| lib/CleantalkSP/SpbctWP/ServerRequirementsChecker/ServerRequirementsChecker.php | Implements server requirement checks and warning generation. |
| lib/CleantalkSP/SpbctWP/LinkConstructor.php | Adds a UTM preset used for the System Requirements docs link. |
| js/src/react/config/Tabs.js | Wires the Summary tab to a custom component to host the new block. |
| js/src/react/components/TabContent/TabSummary/TabSummary.js | Adds the System Requirements block to the Summary tab composition. |
| js/src/react/components/SystemRequirements/SystemRequirements.js | Fetches and renders the requirements list and docs link in React (with portal mounting). |
| js/src/react/components/SystemRequirements/styles.css | Styles for the System Requirements block. |
| inc/spbc-admin.php | Adds a new AJAX tab handler branch for system_requirements. |
Review details
- Files reviewed: 8/9 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are verified runtime-resilience and i18n-extraction issues (plus a misleading warning string) that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
js/src/react/components/SystemRequirements/SystemRequirements.js:49
- On request failure (network error, invalid JSON, nonce failure), the catch block only logs to console and
sectionDataremainsnull, leaving the preloader displayed indefinitely. Consider setting a fallback state (or an explicit error state) so the block can render a non-loading UI.
js/src/react/components/SystemRequirements/SystemRequirements.js:75 - If the AJAX endpoint returns an error payload (e.g.,
wp_send_json_error()),sectionData.objects/flags/stringsmay be missing and this render path will throw. Use optional chaining/defaults when reading nested data to keep the UI resilient to unexpected responses.
lib/CleantalkSP/SpbctWP/ServerRequirementsChecker/ServerRequirementsChecker.php:42 $requirement_itemslabels are plain strings and later passed into__()dynamically (see View.php). Gettext tooling typically won’t extract dynamic translation strings, so these labels won’t be localizable. Prefer storing already-translated label templates here using literal__()calls so they can be extracted into the POT.
lib/CleantalkSP/SpbctWP/ServerRequirementsChecker/View.php:70
- If
$item['label']is stored as an already-translated template (recommended for gettext extraction), wrapping it in__()here will either double-translate or prevent extraction when it’s not a literal. Use the template directly insprintf()and keep translation at the definition site.
'label' => sprintf(
__($item['label'], 'security-malware-firewall'),
self::formatValue($key, $checker->requirements[$key])
),
- Files reviewed: 8/9 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+127
to
+131
| public function checkRequirements() | ||
| { | ||
| $this->warnings = []; | ||
| // Check PHP version | ||
| if (version_compare((string) $this->getRequiredParameterValue('php_version'), $this->requirements['php_version'], '<')) { |
Comment on lines
+173
to
+177
| if ( ! $this->getRequiredParameterValue('curl_multi_funcs_array') ) { | ||
| $funcs_list = esc_html(implode(', ', $this->curl_multi_funcs_array)); | ||
| $template = __('At least one of cURL multiple request functions (%s) is not available, but required for the Security FireWall features', 'security-malware-firewall'); | ||
| $this->warnings['curl_multi_funcs_array'] = sprintf($template, $funcs_list); | ||
| } |
svfcode
approved these changes
Sep 9, 2026
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
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.
https://app.doboard.com/1/task/42654