Skip to content

Fix API PHP unit tests#3051

Open
AbdiTolesa wants to merge 7 commits intomasterfrom
fix/api_unit_tests
Open

Fix API PHP unit tests#3051
AbdiTolesa wants to merge 7 commits intomasterfrom
fix/api_unit_tests

Conversation

@AbdiTolesa
Copy link
Copy Markdown
Contributor

@AbdiTolesa AbdiTolesa commented Mar 31, 2026

Fix https://github.com/Strategy11/formidable-api/issues/213

Updates:

  • Changed end_year of field with key date-field to 2050 to fix errors like below in the API plugin:
1) test_FrmAPIAppController::test_send_webhooks
Array
(
    [code] => frmapi_validate_entry
    [message] => Array
        (
            [field63] => Date is invalid
        )

    [data] => Array
        (
            [status] => 409
        )

)

Failed asserting that 409 matches expected 200.

/tmp/wordpress/src/wp-content/plugins/formidable-api/tests/base/FrmAPIUnitTest.php:190
/tmp/wordpress/src/wp-content/plugins/formidable-api/tests/test_FrmAPIAppController.php:65
/tmp/wordpress/src/wp-content/plugins/formidable-api/tests/test_FrmAPIAppController.php:31
phpvfscomposer:///home/runner/.composer/vendor/phpunit/phpunit/phpunit:106
  • Make sure that the generated test entry values for option based test fields like the Checkbox field are actually found in the field options.

Summary by CodeRabbit

  • Tests

    • Extended date-field test fixtures to allow years up to 2050.
    • Improved test data generation to better handle checkbox, select, and radio fields by selecting valid random options.
  • Chores

    • Removed restrictive access-control rules so static assets (scripts, styles, fonts, images) are served normally.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Updated test fixture for a date field (end_year set to "2050"), adjusted the test field factory to handle checkbox, select, and radio option selection, and removed the repository .htaccess access-control rules file.

Changes

Cohort / File(s) Summary
Test Data Update
tests/phpunit/base/testdata.xml
Edited field_options JSON for date field (id 8115): end_year changed from "2020" to "2050" (1-line edit).
Field Factory Logic
tests/phpunit/base/frm_factory.php
Field_Factory::set_field_value() now special-cases checkbox, select, and radio: returns '' if options empty; otherwise selects a random option and returns the option's 'value' when FrmField::is_option_true($field, 'separate_value') is true, or the full option array/object otherwise.
Access-Control Removal
/.htaccess
Removed .htaccess rules that previously denied direct access by default and explicitly allowed asset file types (.js, .css, fonts, images). (~56 lines removed).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

I nibble at fixtures beneath the moon,
Years leap to 2050 — what a boon,
Checkboxes dance, radios spin free,
Options chosen at random for me,
A rabbit cheers with a crunchy tune 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix API PHP unit tests' is vague and generic. While it mentions 'API' and 'PHP unit tests', it doesn't clearly convey what specific issue or aspect of the tests is being fixed. The actual changes span multiple unrelated components (test data, factory logic, and .htaccess rules), making the title overly broad and non-descriptive. Use a more specific title that describes the primary change, such as 'Update test factory to handle checkbox, select, and radio field values' or reference the specific issue being fixed with more detail.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/api_unit_tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@deepsource-io
Copy link
Copy Markdown

deepsource-io bot commented Mar 31, 2026

DeepSource Code Review

We reviewed changes in e0c1736...38cd428 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
PHP Apr 1, 2026 5:44p.m. Review ↗
JavaScript Apr 1, 2026 5:44p.m. Review ↗

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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/base/frm_factory.php`:
- Around line 145-153: The options-handling branch can fail when $field->options
is empty or when an option is a string rather than an array; update the branch
that handles types 'checkbox','select','radio' to first guard that
$field->options is non-empty and return a safe default (e.g., null or empty
string) if empty, then normalize the chosen $random_option so you safely extract
a value: if FrmField::is_option_true($field, 'separate_value') return the
option's 'value' when present otherwise return the option itself (or the string)
after converting array/string shapes into a consistent scalar; adjust the code
around $random_option and FrmField::is_option_true to avoid PHP notices and
invalid return types.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7dd35f42-24f6-4841-a0ca-c3f0f1657b20

📥 Commits

Reviewing files that changed from the base of the PR and between a04e344 and 0abf9cf.

📒 Files selected for processing (1)
  • tests/phpunit/base/frm_factory.php

Comment on lines +145 to +153
if ( in_array( $field->type, array( 'checkbox', 'select', 'radio' ), true ) ) {
// If checkbox, dropdown or radio field, return a random choice from the options.
$random_option = $field->options[ array_rand( $field->options ) ];
if ( FrmField::is_option_true( $field, 'separate_value' ) ) {
return $random_option['value'];
}

return $random_option;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "1) Option-based field definitions in tests (check for missing options nearby):"
rg -nP --type=php -C3 "['\"]type['\"]\s*=>\s*['\"](checkbox|select|radio)['\"]" tests

echo
echo "2) separate_value usage (inspect whether selected options are arrays/strings in those setups):"
rg -nP --type=php -C3 "separate_value" tests classes

echo
echo "3) Option-array shapes (value/label/fallback patterns):"
rg -nP --type=php -C2 "['\"](value|label)['\"]\s*=>" tests classes

Repository: Strategy11/formidable-forms

Length of output: 50384


Guard empty options and normalize option shape before returning values.

Line 147 can fail when $field->options is empty, and line 149 assumes every selected option is an array with 'value'. In this codebase, options can be strings or arrays with variable keys, so this code path can produce runtime errors and invalid test values. Empty options arrays are created in tests, confirming the empty-check is necessary.

Proposed fix
 	public function set_field_value( $field ) {
-		if ( in_array( $field->type, array( 'checkbox', 'select', 'radio' ), true ) ) {
-			// If checkbox, dropdown or radio field, return a random choice from the options.
-			$random_option = $field->options[ array_rand( $field->options ) ];
-			if ( FrmField::is_option_true( $field, 'separate_value' ) ) {
-				return $random_option['value'];
-			}
-
-			return $random_option;
-		}
+		if ( in_array( $field->type, array( 'checkbox', 'select', 'radio' ), true ) ) {
+			// If checkbox, dropdown or radio field, return a random choice from the options.
+			$options = is_array( $field->options ) ? array_values( $field->options ) : array();
+			if ( ! empty( $options ) ) {
+				$random_option = $options[ array_rand( $options ) ];
+
+				if ( ! is_array( $random_option ) ) {
+					return $random_option;
+				}
+
+				if ( FrmField::is_option_true( $field, 'separate_value' ) ) {
+					return $random_option['value'] ?? $random_option['label'] ?? reset( $random_option );
+				}
+
+				return $random_option['label'] ?? $random_option['value'] ?? reset( $random_option );
+			}
+		}
 
 		$value        = rand_str();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/phpunit/base/frm_factory.php` around lines 145 - 153, The
options-handling branch can fail when $field->options is empty or when an option
is a string rather than an array; update the branch that handles types
'checkbox','select','radio' to first guard that $field->options is non-empty and
return a safe default (e.g., null or empty string) if empty, then normalize the
chosen $random_option so you safely extract a value: if
FrmField::is_option_true($field, 'separate_value') return the option's 'value'
when present otherwise return the option itself (or the string) after converting
array/string shapes into a consistent scalar; adjust the code around
$random_option and FrmField::is_option_true to avoid PHP notices and invalid
return types.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
tests/phpunit/base/frm_factory.php (1)

151-154: ⚠️ Potential issue | 🔴 Critical

Normalize option shape before reading ['value'] to avoid runtime failure.

Line 154 assumes the random option is always an array. If options include scalar strings, this can fatally fail when separate_value is enabled.

Proposed fix
 		if ( in_array( $field->type, array( 'checkbox', 'select', 'radio' ), true ) ) {
-			if ( empty( $field->options ) ) {
+			$options = is_array( $field->options ) ? array_values( $field->options ) : array();
+			if ( empty( $options ) ) {
 				return '';
 			}
 
 			// If checkbox, dropdown or radio field, return a random choice from the options.
-			$random_option = $field->options[ array_rand( $field->options ) ];
+			$random_option = $options[ array_rand( $options ) ];
 
 			if ( FrmField::is_option_true( $field, 'separate_value' ) ) {
-				return $random_option['value'];
+				if ( is_array( $random_option ) ) {
+					return $random_option['value'] ?? $random_option['label'] ?? (string) reset( $random_option );
+				}
+				return $random_option;
 			}
 
+			if ( is_array( $random_option ) ) {
+				return $random_option['label'] ?? $random_option['value'] ?? (string) reset( $random_option );
+			}
 			return $random_option;
 		}
#!/bin/bash
set -euo pipefail

echo "1) Confirm current set_field_value implementation context:"
rg -nP --type=php -C4 "function set_field_value|array_rand|separate_value" tests/phpunit/base/frm_factory.php

echo
echo "2) Find option-based field definitions and option value shapes in tests/classes:"
rg -nP --type=php -C3 "['\"]type['\"]\s*=>\s*['\"](checkbox|select|radio)['\"]|['\"]options['\"]\s*=>|['\"]label['\"]\s*=>|['\"]value['\"]\s*=>" tests classes

echo
echo "3) Find fixtures/usages where separate_value is enabled:"
rg -nP -C2 "separate_value"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/phpunit/base/frm_factory.php` around lines 151 - 154, In the block that
picks $random_option (used by set_field_value in
tests/phpunit/base/frm_factory.php) normalize the option shape before accessing
['value']: detect if $random_option is a scalar (string/int) or an object and
convert it to an associative array with a 'value' key (e.g. treat scalar as
['value' => $random_option]), or fall back to using a 'label' when 'value' is
missing; perform this normalization right after $random_option =
$field->options[ array_rand( $field->options ) ] and before the
FrmField::is_option_true(...) branch so that references to
$random_option['value'] are safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@tests/phpunit/base/frm_factory.php`:
- Around line 151-154: In the block that picks $random_option (used by
set_field_value in tests/phpunit/base/frm_factory.php) normalize the option
shape before accessing ['value']: detect if $random_option is a scalar
(string/int) or an object and convert it to an associative array with a 'value'
key (e.g. treat scalar as ['value' => $random_option]), or fall back to using a
'label' when 'value' is missing; perform this normalization right after
$random_option = $field->options[ array_rand( $field->options ) ] and before the
FrmField::is_option_true(...) branch so that references to
$random_option['value'] are safe.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 45feb7d2-91f0-4412-b033-82c587f2939d

📥 Commits

Reviewing files that changed from the base of the PR and between 9a397fc and 4407082.

📒 Files selected for processing (2)
  • .htaccess
  • tests/phpunit/base/frm_factory.php
💤 Files with no reviewable changes (1)
  • .htaccess

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 26.59%. Comparing base (7058fde) to head (cc5bfe2).
⚠️ Report is 115 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3051      +/-   ##
============================================
- Coverage     26.60%   26.59%   -0.02%     
- Complexity     8917     9078     +161     
============================================
  Files           145      148       +3     
  Lines         30018    30476     +458     
============================================
+ Hits           7987     8105     +118     
- Misses        22031    22371     +340     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AbdiTolesa AbdiTolesa requested a review from truongwp April 1, 2026 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant