Allow form actions to update 'fields' & 'thanks' data

[dregad]

• $fields and $thanks parameters are passed by reference in bureaucracy_handler_interface::handleData() and helper_plugin_bureaucracy_action::run() methods (including descendants).
• helper_plugin_bureaucracy_field::setVal() method is now public

This allows "chaining" of several actions in a single form, enabling e.g. a script action handler to pre-process submitted data before it is passed on to a template or mail action.

Fixes #223

[dregad]

Is there anything preventing this from being merged ?

[dregad]

@micgro42 after your processing of my other 3 PR's, maybe there is a chance you can review and merge this one as well ? Thanks in advance !

[dregad]

It would be appreciated if you guys could merge this PR, or let me know if there is something I need to do before you do so. Thanks !

[dregad]

Just a friendly bump 😃

[frafum]

If I get it right, this patch could also solve the problem I am having in the linked message, if I create a hidden "result" field in the form and fill its value with the result of the calculation from the script.
https://forum.dokuwiki.org/post/63861;nocount

Thus, a big +1 from my part for this PR. :-)

PS: However, I would need a more explicit example about how to access the single fields in the action script; unfortunately, the example on the page of the bureaucracy plugin showing how to write the value of the fields in a log is not helpful for me, as I am not a developer.

[frafum]

The following RFE might also become redundant, as it would be possible to act on the data submitted by the user in the form before passing it to the email template.
#259

[dregad]

Thanks for your interest and support @frafum 😃

Unfortunately, considering that I first raised this issue over a year ago (#223) and despite multiple reminders never got any kind of feedback from the powers that be except for a request to submit a pull request, I'm afraid that there is not much interest in this feature from their end 😞

[Klap-in]

For me the feature is fine. But I didn't have look in how it really works. I don't know yet an idea what the script action is actually doing.

@micgro42 or @splitbrain did introduce the script action and they are building more complex stuff with this plugin. So I hope they could have a technical look into it.

[dregad]

Thanks for the feedback @Klap-in.

To clarify and maybe facilitate the review process: the PR does not introduce any change in behavior (i.e. the script action works exactly as it did before). The only thing it does is allowing an action script to update the parameters it receives, since they are now passed by reference.

This enables action chaining, e.g.

<form>
textbox "field1" @
submit 
action script update.php
action template ns:tmpl :
thanks "Page created"
</form>

Assuming update.php does something like appending text

	function handleData(&$fields, &$thanks) {
		$fields[0]->setVal($fields[0]->getParam('value') . "_updated");
		return ''; // return empty string a we don't want to display anything at this time
	}

Submitting the form with data as contents of field1, the plugin would

• first call update.php
• then execute the template action, that will use the updated field's value to create page data_updated

That's all. Of course, since the handleData method's signature has changed merging the PR would introduce a regression for existing scripts as PHP would throw Fatal error: Declaration of helper_plugin_bureaucracy_handler_XXX::handleData($fields, $thanks) must be compatible with dokuwiki\plugin\bureaucracy\interfaces\bureaucracy_handler_interface::handleData(&$fields, &$thanks). Adapting them is as simple as adding the & operator to the $fields and $thanks parameters.

[Klap-in]

Looks fine for me.
However, I have no script actions which need an update.

I guess @micgro42 has some of these?

[dregad]

I am still hoping that this would get merged in at some point... Please ? 🙏

[solewniczak]

This API change will break all the plugins that extends the bureaucracy classes(struct plugin for example) and although I like the idea, we must also preserve backwards compatibility. I will think about it. Maybe it can be done by some dokuwiki event? @dregad do you have any idea?

[dregad]

@solewniczak thanks for responding and looking into this ! 😃

I'm sorry I do not know enough about the internals of dokuwiki event system to really be able to help in this area, and I can't think of any other way to achieve the objective at the moment.

While it is true that the approach I proposed it would break dependent plugins due to the modified function signatures, please consider that the change is very simple. (just adding a & to pass parameters by reference).

Moreover, I don't think there are that many plugins out there that extend these methods, so adapting the struct plugin (and any others) wouldn't require a big effort - to avoid any regression, the child class only needs to be checked to ensure the updated functions are not altering the parameters' values, and store them in a local variable if they are.

Finally, considering that PHP would throw error/warning messages due to the modified signature, it would be easy to detect if and where corrective action is required.

[dregad]

I had a look at struct plugin's code. Note that I don't use it myself, so I don't really know how it works and my analysis may therefore not be sufficient. Anyway, AFAICT there are only 2 occurrences of classes extending bureaucracy:

• helper_plugin_struct_lookup extends helper_plugin_bureaucracy_action - run() method: the $fields param is only used for reading, $thanks and $argv are not used.
• helper_plugin_struct_field extends helper_plugin_bureaucracy_field - setVal() method: here the $value param is modified (set to '' if not empty, effectively just changing the type to string), so a check is needed, whether this needs to be propagated or not and if yes what the impact of that would be.

Hope this helps.

[dregad]

@solewniczak any update ?

[dregad]

Friendly bump... Some feedback would be nice 🙏

[iainhallam]

@solewniczak A breaking change could be issued for the Bureaucracy plugin with a major version bump. This seems like really useful functionality to add.

[dregad]

@iainhallam thanks for your support. Unfortunately, after 4 years, multiple reminders and additional analysis to respond to compatibility concerns, it is sadly quite obvious by now that the plugin maintainers have zero interest in merging this feature... 😞

[dregad]

friendly bump...