Skip to content

New function FrmForm::prepare_form_row_data#1547

Merged
Crabcyborg merged 6 commits into
masterfrom
new_function_prepare_frm_form_form_row_data
Mar 4, 2024
Merged

New function FrmForm::prepare_form_row_data#1547
Crabcyborg merged 6 commits into
masterfrom
new_function_prepare_frm_form_form_row_data

Conversation

@Crabcyborg
Copy link
Copy Markdown
Contributor

@Crabcyborg Crabcyborg commented Feb 29, 2024

Related ticket https://secure.helpscout.net/conversation/2518729092/191090/

A lot of code works on the assumption that $form->options is an array.

I tried adding some checks in some updates yesterday (in #1543, https://github.com/Strategy11/formidable-pro/pull/4845 and https://github.com/Strategy11/formidable-pro/pull/4844) but I think we're better off just forcing $form->options as an array right away. Trying to fix this in every place where we assume $form->options is an array is turning into too much work and too many undetected bugs.

This update now forces all results from FrmForm::getOne to have an array $form->options value when $form is not null. I used the lite form defaults so some basic keys are defined, to help fix an issue with a PHP warning.

Questions

  1. Would we want to try calling FrmFormsHelper::get_default_opts() as well to define some Pro defaults?
  2. Would we want to start only filtering frm_form_object when it's an object and not null? That might help with stability as well.

Fatal error: Uncaught TypeError: Cannot access offset of type string on string in .../wp-content/plugins/formidable/classes/models/FrmForm.php:605 Stack trace: #0 .../wp-content/plugins/formidable/classes/models/FrmForm.php(559): FrmForm::trash() #1 .../wp-content/plugins/formidable/classes/controllers/FrmFormsController.php(658): FrmForm::set_status() #2 .../wp-content/plugins/formidable/classes/controllers/FrmFormsController.php(626): FrmFormsController::change_form_status() #3 .../wp-content/plugins/formidable/classes/controllers/FrmFormsController.php(1754): FrmFormsController::trash() #4 .../wp-includes/class-wp-hook.php(324): FrmFormsController::route() #5 .../wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters() #6 .../wp-includes/plugin.php(517): WP_Hook->do_action() #7 .../wp-admin/admin.php(259): do_action() #8 {main} thrown in .../wp-content/plugins/formidable/classes/models/FrmForm.php on line 605

Coming from this line of code:

$options['trash_time'] = time();

Since $options is a string, trying to set $options['trash_time'] triggers a fatal error.

@Crabcyborg Crabcyborg added this to the 6.8.3 milestone Feb 29, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 29, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 29.54%. Comparing base (058d0ed) to head (b42e971).
Report is 51 commits behind head on master.

Files Patch % Lines
classes/models/FrmForm.php 88.88% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1547      +/-   ##
============================================
+ Coverage     29.45%   29.54%   +0.08%     
- Complexity     7696     7735      +39     
============================================
  Files           115      115              
  Lines         25404    25482      +78     
============================================
+ Hits           7484     7529      +45     
- Misses        17920    17953      +33     

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

@ghost
Copy link
Copy Markdown

ghost commented Feb 29, 2024

@Crabcyborg Hi!
I installed the provided FormidableForms version, and I tested the following:

  • [ x ]Moving the form to the Trash.
  • [ x ]Clicking on Undo when moving the form to the trash.
  • [ x ]Build, Settings, Entries, and Views tabs work fine.
  • [ ]Style tab is returning this warning
"Warning: Undefined array key "custom_style" in
/home/u103776375/domains/protecontech.com/public_html/wp-content/plugins/formidable/classes/views/styles/_styles-list.php
on line
11"

-[ x ]Added field and updated the form successfully
-[ x ]Form looks good when previewing it.
-[ x ]Form submitted fine.

@Crabcyborg
Copy link
Copy Markdown
Contributor Author

Thanks @joaovictorangeline.

I'm thinking we may want to set the default form options instead of an empty array to solve that issue.

@ghost
Copy link
Copy Markdown

ghost commented Feb 29, 2024

Thanks @joaovictorangeline.

I'm thinking we may want to set the default form options instead of an empty array to solve that issue.

Yes, @Crabcyborg when I enabled the default style, it worked fine!
Should I wait for a new patch or should I answer the customer with what we already have? The warning will disappear when WP_DEBUG is false.

@Crabcyborg
Copy link
Copy Markdown
Contributor Author

Crabcyborg commented Feb 29, 2024

@joaovictorangeline

Here's a more up to date pre-release you can share that should fix that issue as well,
formidable-6.8.3b.zip

I went ahead and started using the lite defaults when there are no options. This includes the custom_style and should resolve that warning.

@ghost
Copy link
Copy Markdown

ghost commented Feb 29, 2024

I tested and it worked @Crabcyborg !
Thanks for putting your efforts on this.

@Crabcyborg Crabcyborg changed the title New function FrmForm::prepare_form_row_data New function FrmForm::prepare_form_row_data Feb 29, 2024
Copy link
Copy Markdown
Contributor

@stephywells stephywells left a comment

Choose a reason for hiding this comment

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

This looks like a great approach.

  1. I think it's fine to not set the defaults for the pro options. The code should handle that already since many forms would start as lite forms before an upgrade.
  2. That sounds like a good adjustment to only run the hook if it's an object.

@Crabcyborg Crabcyborg merged commit c26c25a into master Mar 4, 2024
@Crabcyborg Crabcyborg deleted the new_function_prepare_frm_form_form_row_data branch March 4, 2024 19: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.

2 participants