backdrop-contrib / references_dialog

Extends reference fields by adding links to add, edit, and search for references through a dialog.
https://backdropcms.org/project/references
GNU General Public License v2.0
1 stars 3 forks source link

Fatal error: "_element_validate_integer_positive" not found or invalid function name #24

Closed robertgarrigos closed 1 month ago

robertgarrigos commented 2 months ago

I've found this fatal error when saving a field after setting the "Add dialog" check Tags___Robert_-_Backhub_-_Vivaldi

robertgarrigos commented 2 months ago

Line 763 of references_dialog.module:

'#element_validate' => array('_element_validate_integer_positive'),

provably should be:

'#element_validate' => array('references_element_validate_integer_positive'),

which means references should be added as dependency also.

laryn commented 2 months ago

@robertgarrigos See https://docs.backdropcms.org/change-records/element_validate_number-element_validate_integer-and-element_validate_integer_positive-replaced-with-number-form-api-element-type

robertgarrigos commented 2 months ago

Thanks for pointing me to that page. I'll make the proper changes.

robertgarrigos commented 2 months ago

Just updated the PR

argiepiano commented 2 weeks ago

This is not the right approach to fix this problem. You added a function that's available in another module (references) and then added a dependency, which creates problems for people like #33 who use this module with entityreference - this forces them to install a module they don't need.

A better approach is to replace the textfield form element with a number form element, which already includes an integer numerical validation, meaning you can get rid of the #element_validate.

Can we please revisit this fix and fix the fix? ;-)

argiepiano commented 2 weeks ago

I'm talking about this fix, that was merged recently: https://github.com/backdrop-contrib/references_dialog/commit/ea13c9b6b8b636c5841402578424b724f69e5485

argiepiano commented 2 weeks ago

Although, sorry, I see that was reverted in the current release. However, the unnecessary dependency was not removed.

robertgarrigos commented 2 weeks ago

fixed