Fix some phpcs warnings#2676
Conversation
WalkthroughThis PR performs comprehensive code cleanup across multiple files, removing unused variable assignments, unused global declarations, simplifying loop constructs, and normalizing whitespace alignment. Most changes are cosmetic refactors with minimal functional impact; however, a test file and one authentication method contain logic modifications that warrant review. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/phpunit/forms/test_FrmForm.php (1)
64-66: Valid cleanup of unused variable assignment.Removing the unused return value assignment aligns with the PR's phpcs cleanup objective. The test still validates deletion through the
FrmForm::getOne()check on line 65.Consider explicitly asserting the return value for more precise test reporting:
$result = FrmForm::destroy( $form->id ); $this->assertNotFalse( $result, 'Failed to destroy form ' . $form->form_key );This would provide clearer feedback if
destroy()fails versus if the subsequent check fails.classes/views/frm-form-actions/_action_inside.php (1)
29-41: Initializeaction_options['event']to a sane defaultSetting
$action_control->action_options['event']to'create'when it’s missing ensures:
- The event list always contains at least one valid value.
- Subsequent normalization (
explode+count) and rendering (hidden input vs multiselect) behave predictably instead of producing empty/invalid options.This is a safe, backward‑compatible default and removes the risk of rendering an empty event option when
eventwasn’t set.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
classes/controllers/FrmAddonsController.phpclasses/controllers/FrmFormTemplatesController.phpclasses/controllers/FrmStylesController.phpclasses/controllers/FrmXMLController.phpclasses/helpers/FrmAppHelper.phpclasses/helpers/FrmCSVExportHelper.phpclasses/helpers/FrmDashboardHelper.phpclasses/helpers/FrmEntriesHelper.phpclasses/helpers/FrmFormsListHelper.phpclasses/helpers/FrmHtmlHelper.phpclasses/models/FrmCreateFile.phpclasses/models/FrmField.phpclasses/models/FrmForm.phpclasses/models/FrmFormAction.phpclasses/models/FrmFormMigrator.phpclasses/models/fields/FrmFieldName.phpclasses/models/fields/FrmFieldType.phpclasses/views/frm-entries/form.phpclasses/views/frm-fields/back-end/settings.phpclasses/views/frm-form-actions/_action_inside.phpclasses/views/frm-form-actions/_email_settings.phpclasses/views/frm-settings/email/email-styles.phpclasses/views/summary-emails/base.phpclasses/views/xml/posts_xml.phpsquare/controllers/FrmSquareLiteActionsController.phpstripe/controllers/FrmStrpLiteActionsController.phpstripe/controllers/FrmTransLiteActionsController.phpstripe/helpers/FrmStrpLiteConnectHelper.phpstripe/helpers/FrmTransLiteListHelper.phpstripe/models/FrmStrpLiteAuth.phptests/phpunit/fields/test_FrmFieldValidate.phptests/phpunit/forms/test_FrmForm.phptests/phpunit/misc/test_FrmAppHelper.phptests/phpunit/misc/test_FrmDirectFileAccess.phptests/phpunit/stripe/FrmStrpLiteUnitTest.phptests/phpunit/styles/test_FrmStylesHelper.php
💤 Files with no reviewable changes (10)
- classes/views/frm-settings/email/email-styles.php
- classes/models/FrmForm.php
- tests/phpunit/styles/test_FrmStylesHelper.php
- stripe/helpers/FrmTransLiteListHelper.php
- classes/views/frm-entries/form.php
- tests/phpunit/misc/test_FrmAppHelper.php
- classes/views/xml/posts_xml.php
- classes/views/frm-form-actions/_email_settings.php
- classes/views/summary-emails/base.php
- tests/phpunit/fields/test_FrmFieldValidate.php
🧰 Additional context used
🧬 Code graph analysis (7)
classes/controllers/FrmXMLController.php (1)
classes/models/FrmDb.php (2)
FrmDb(6-867)get_results(328-330)
classes/controllers/FrmAddonsController.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5049)make_affiliate_url(101-110)
tests/phpunit/forms/test_FrmForm.php (1)
classes/models/FrmForm.php (2)
FrmForm(6-1412)destroy(707-747)
stripe/models/FrmStrpLiteAuth.php (1)
stripe/helpers/FrmStrpLiteLinkRedirectHelper.php (1)
delete_temporary_referer_meta(180-183)
stripe/helpers/FrmStrpLiteConnectHelper.php (1)
square/helpers/FrmSquareLiteConnectHelper.php (1)
post_with_authenticated_body(527-548)
classes/views/frm-form-actions/_action_inside.php (1)
classes/controllers/FrmFormActionsController.php (1)
action_control(444-450)
classes/helpers/FrmFormsListHelper.php (3)
square/js/settings.js (1)
mode(7-7)classes/helpers/FrmAppHelper.php (1)
get_param(736-773)classes/helpers/FrmListHelper.php (1)
get_param(195-204)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (27)
classes/models/fields/FrmFieldType.php (1)
843-857: ✓ Whitespace cleanup in get_new_field_defaults looks good.The indentation adjustment on line 844 aligns with the PR's phpcs cleanup objective. The method logic remains unchanged—it correctly builds and merges field default arrays without behavioral impact.
square/controllers/FrmSquareLiteActionsController.php (1)
548-556: Formatting change looks good.The $square_vars array assignment has been reformatted without any functional impact. The array structure remains clear and maintainable.
To confirm this PR fully resolves the phpcs warnings mentioned in the title, please verify that running phpcs against this file now produces no violations. If you have access to the specific warning messages that were reported before this change, confirming they're resolved would be helpful.
classes/models/fields/FrmFieldName.php (1)
284-284: LGTM! Clean removal of unused loop variable.The
$namekey variable was never referenced inside the loop body, which only accesses$sub_field. This refactor correctly addresses the phpcs warning while preserving behavior.classes/controllers/FrmXMLController.php (1)
601-601: LGTM!Formatting adjustment only—no functional changes.
stripe/controllers/FrmTransLiteActionsController.php (1)
566-566: LGTM!Whitespace normalization with no impact on behavior.
tests/phpunit/misc/test_FrmDirectFileAccess.php (1)
23-23: LGTM!Good cleanup—removes an unused loop index variable.
classes/helpers/FrmEntriesHelper.php (1)
429-429: LGTM!Whitespace alignment—no functional change.
classes/controllers/FrmFormTemplatesController.php (1)
569-572: LGTM!Array formatting normalized; no behavioral impact.
stripe/helpers/FrmStrpLiteConnectHelper.php (1)
891-891: LGTM!Whitespace adjustment—no logic changes.
classes/models/FrmFormAction.php (1)
663-663: LGTM!Whitespace cleanup—no functional changes.
stripe/models/FrmStrpLiteAuth.php (1)
820-823: Clarify the lifecycle of temporary referer meta in message flows.The temporary referer meta is intentionally retained when
$delete_meta=false(line 782). This meta persists for entries using the message confirmation method, whereas entries using redirect flows delete it immediately. This is acceptable because the meta is cleaned up when the entry itself is deleted (FrmEntry.php line 381 deletes all metas for an entry). However, the design intent should be documented—clarify whether temporary referer meta should persist for the entry lifetime, or whether it should be explicitly deleted after the message is displayed.classes/controllers/FrmAddonsController.php (1)
888-901: Affiliate URL preparation remains correctThe change here is purely formatting;
FrmAppHelper::make_affiliate_url( $link );is still called in the same place in the pipeline before UTM tagging, so behavior is unchanged and correct.classes/helpers/FrmHtmlHelper.php (1)
76-111: Unit input attribute merge unchangedOnly whitespace around
$input_number_attrs = array_merge(...)changed; attribute computation (type, value, class) is intact, so the rendered input remains identical.classes/helpers/FrmAppHelper.php (1)
437-445: Admin-page detection logic untouchedThe only change here is spacing on the
$pageassignment; the logic for detecting Formidable admin pages is identical and remains correct.classes/helpers/FrmCSVExportHelper.php (1)
505-523: Entry batching query unchangedThe
$wherearray forFrmEntry::getAll()is identical aside from whitespace, preserving the intended parent/child query optimization for Pro vs Lite.classes/models/FrmFormMigrator.php (1)
41-52: Migrator state defaults preservedOnly spacing on
$fields_mapand$current_sectioninitializations changed; both still default to empty arrays, so migration logic is unaffected.classes/models/FrmField.php (1)
8-17: Static cache flag unchanged
public static $use_cachestill defaults totrue; only whitespace changed, leaving all caching behavior intact.classes/views/frm-fields/back-end/settings.php (1)
222-222: LGTM! Whitespace normalization.This formatting change improves code consistency without affecting functionality.
classes/helpers/FrmDashboardHelper.php (1)
192-192: LGTM! Removed unused loop index.The loop index
$iwas never referenced in the loop body, so removing it simplifies the code without affecting functionality.classes/models/FrmCreateFile.php (1)
36-41: LGTM! Whitespace normalization.These formatting changes improve code consistency in property declarations without affecting functionality.
classes/controllers/FrmStylesController.php (2)
295-295: LGTM! Removed unused variable fetch.The settings were fetched but never used in this function. Removing the unused variable improves code clarity.
347-347: LGTM! Whitespace normalization.These formatting changes improve code consistency in array initialization without affecting functionality.
Also applies to: 365-365
classes/helpers/FrmFormsListHelper.php (3)
30-30: LGTM! Removed unused global declaration.The
$wpdbglobal was declared but never used in theprepare_items()method. Removing it reduces unnecessary global scope pollution.
56-56: LGTM! Whitespace normalization.This formatting change improves code consistency without affecting functionality.
240-240: LGTM! Removed unused global declaration.The
$frm_varsglobal was declared but never used in thesingle_row()method. Removing it reduces unnecessary global scope pollution.tests/phpunit/stripe/FrmStrpLiteUnitTest.php (1)
452-452: LGTM! Whitespace normalization.This formatting change improves code consistency without affecting test functionality.
stripe/controllers/FrmStrpLiteActionsController.php (1)
320-320: LGTM! Whitespace normalization.This formatting change improves code consistency without affecting functionality.
Fix some phpcs warnings and treat another warning as error
=for single line variable declarations.$delete_metavariable before deleting meta (Like in https://github.com/Strategy11/formidable-stripe/pull/265/files).issetcheck, where it wasn't setting the right variable.$eventsis never used, but the variable behind theissetcheck is then assumed to be set moving forward.