Guard plugin action links pro check when EDAC_KEY_VALID is undefined#1408
Conversation
Summary of ChangesHello @SteveJonesDev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a PHP 8 compatibility issue by addressing an Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThis change replaces the direct constant/flag Pro check with the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a PHP 8 error caused by referencing an undefined constant. The fix correctly replaces the direct constant check with the edac_is_pro() helper function, which is a safer and more robust approach. The addition of a new regression test in a separate process is excellent, as it specifically verifies the fix for the undefined constant scenario. I have one suggestion to improve the mock function in the new test file to make it more accurate and maintainable.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/phpunit/Admin/PluginActionLinksUndefinedKeyTest.php`:
- Around line 21-24: The test method
test_add_plugin_action_links_adds_pro_link_when_key_constant_missing should
explicitly assert/guard the precondition that EDAC_KEY_VALID is undefined and
EDACP_VERSION is not set so edac_is_pro() takes the “key missing” branch; update
the test to check for and unset (or skip if defined) EDAC_KEY_VALID and
EDACP_VERSION before running the assertions (and mark the test skipped if the
environment cannot be coerced), and apply the same explicit precondition
handling to the other related test(s) that exercise the undefined-key branch
(referencing edac_is_pro and EDAC_KEY_VALID/EDACP_VERSION to locate the code).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cc7c1576-0d6c-4da7-ad92-ab2ff2283b1c
📒 Files selected for processing (1)
tests/phpunit/Admin/PluginActionLinksUndefinedKeyTest.php
…VALID and EDACP_VERSION
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/phpunit/Admin/PluginActionLinksUndefinedKeyTest.php (1)
54-58: Consider tightening the regression assertion to validate link content.Line 58 confirms the
go_prokey exists; adding a content assertion would make the test more specific to the “Get Pro” behavior.As per coding guidelines:
tests/phpunit/**/*.php: Write unit tests for new PHP functions and classes in /tests/phpunit directory using PHPUnit.Optional assertion hardening
$this->assertArrayHasKey( 'go_pro', $result, 'Pro link should be present when EDAC_KEY_VALID is undefined' ); + $this->assertStringContainsString( 'Get Pro', $result['go_pro'] ); + $this->assertStringContainsString( 'equalizedigital.com/accessibility-checker/pricing/', $result['go_pro'] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpunit/Admin/PluginActionLinksUndefinedKeyTest.php` around lines 54 - 58, The test currently only asserts the presence of the 'go_pro' key; tighten it by also asserting the link content returned by Plugin_Action_Links::add_plugin_action_links contains the expected “Get Pro” anchor (or specific href/text you expect) to ensure the correct pro link is rendered when EDAC_KEY_VALID is undefined. Find the Plugin_Action_Links instance in Admin/PluginActionLinksUndefinedKeyTest and add an assertion like asserting the $result['go_pro'] string contains the expected anchor text or URL (e.g., 'Get Pro' or the pro link path) so the test validates both key presence and correct link content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/phpunit/Admin/PluginActionLinksUndefinedKeyTest.php`:
- Around line 54-58: The test currently only asserts the presence of the
'go_pro' key; tighten it by also asserting the link content returned by
Plugin_Action_Links::add_plugin_action_links contains the expected “Get Pro”
anchor (or specific href/text you expect) to ensure the correct pro link is
rendered when EDAC_KEY_VALID is undefined. Find the Plugin_Action_Links instance
in Admin/PluginActionLinksUndefinedKeyTest and add an assertion like asserting
the $result['go_pro'] string contains the expected anchor text or URL (e.g.,
'Get Pro' or the pro link path) so the test validates both key presence and
correct link content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7e02e1bd-3dfe-48de-a077-376416508376
📒 Files selected for processing (1)
tests/phpunit/Admin/PluginActionLinksUndefinedKeyTest.php
1 participant
Summary
EDAC_KEY_VALIDusage in plugin action links withedac_is_pro()guardEDAC_KEY_VALIDis undefinedRoot Cause
Plugin_Action_Links::add_plugin_action_links()referencedEDAC_KEY_VALIDdirectly. In free-plugin contexts where the constant is not defined, PHP 8 raises an undefined constant error.Test Added
tests/phpunit/Admin/PluginActionLinksUndefinedKeyTest.phptest_add_plugin_action_links_adds_pro_link_when_key_constant_missingRisk Assessment
Low risk. The change narrows to pro-state detection in one UI link condition and reuses existing helper logic (
edac_is_pro()) already used elsewhere.Environment Limitations
npm run lint:js:fixandnpm run lint:jscompleted successfully.npm run lint:php:fixandnpm run lint:phpcould not run becausevendor/bin/phpcbfandvendor/bin/phpcsare missing after incomplete Composer install.npm run test:phpis blocked by Docker socket permission in this sandbox (operation not permitted).composer installfailed due DNS resolution forapi.github.com, preventing full PHP tooling install.Summary by CodeRabbit
Tests
Refactor