Ping @DenisChenu and @gabrieljenik if you're interested. We will work on this internally and initially only support global settings form and input checkbox.

Added to LS5 LTS, I guess (dev branch).

You mean : for GroupSettings ? UserSettings ? near SurveySettings

Something related to an object with an id ?

https://manual.limesurvey.org/BeforeSurveySettings
https://manual.limesurvey.org/NewSurveySettings

This would be a general event for any form that supports it. So, one form event to rule them all. :)

But : you need to review all existing form then ?

Before : need to review core form …

Each form has an id used by the event, so that objects are attached to the correct form

https://github.com/LimeSurvey/LimeSurvey/blob/baf7a27369fab4bb10bacb467790e046385da591/application/models/PluginSetting.php#L21-L22

Yeah, but we're just gonna support global settings for now. Rest are added when needed by a plugin author.

$event->append('globalsettings', new ExtendFormCheckbox('input_id', function($post, $connection) { ... save logic here }));

:(

To add 3elements

$event->append('globalsettings', new ExtendFormCheckbox('input_id', function($post) { ... save logic here }));
$event->append('globalsettings', new ExtendFormCheckbox('input_id2', function($post) { ... save logic here /* Same than previous*/ }));
$event->append('globalsettings', new ExtendFormCheckbox('input_id3', function($post) { ... save logic here /* copy/paste from previous*/  }));

I never have to update global settings ,

See how i do to create forum since some years now (and it work without update plugin …) : https://gitlab.com/SondagesPro/mailing/sendMailCron/-/blob/master/sendMailCron.php#L1403

We already hae 2 form constructor : one for Survey and one for QuestionAttribute.
You add another one …

No no.

$saveMethod = function($post, $db) { ... };
$event->append('globalsettings', new ExtendFormCheckbox('input_id', $saveMethod));
etc

We already have 2 form constructor : one for Survey and one for QuestionAttribute.

Let me read those again, then. But I doubt they are really flexible, nor object oriented...

Let me read those again, then. But I doubt they are really flexible, nor object oriented...

Not really flexible … right.

OK, so how would you add a new select2 or datepicker as a survey setting...?

Ah, it's using the SettingsWidget. Also used by plugin settings, right? I thought about that one...

Like always … «je me débrouille comme je peux»

example : https://gitlab.com/SondagesPro/OrcidAuthenticate/-/blob/master/views/admin/IntroductionSetting.php

So you'd prefer $event->append('globalsettings.tab', ...settings widget data)?

I prefer something easy and stable ;)

I think i already create a mantis issue about moving QuestoionAttributes + CoreForms + PluginSettings to same extension .

Else : i thionk have a definition array without need to create new object each time still instresting

Arrays require more writing than objects, tho. And they have no validation, or defaults. But a middle-ground is to have objects with $input->toSettingsWidget() function that converts them. Idk.

Arrays require more writing than objects, tho.

?
You mean by core team ?

And they have no validation, or defaults.

core can offer some … PluginSettings have htmlOptions and current

$saveMethod = function($post, $db) { ... };
$event->append('globalsettings', new ExtendFormCheckbox('input_id', $saveMethod));

default saveMethod by core can be great
(for example : i think i creat an issue about save Plugin settings password encrypted by default …)

Idk, the settings widget is doing a lot of shit... Starting form tags, registering css and js files, ... It should have been an object hierarchy rather, SettingsWidgetButton, SettingsWidgetBoolean, etc, instead of a function for every setting type. Then it could have been reused in different contexts.

Yes …
but : if you create another way, do you deprecated and removed old ways ?
And then : we have to redo whole plugins in 7 or 8 ?

default saveMethod by core can be great

Ya, but not sure how... xD Needs info about database table and column and validation. Basically model data.

$saveMethod = new DefaultSaveMethod('table', 'column', $validationRules);

??

but : if you create another way, do you deprecated and removed old ways ?
And then : we have to redo whole plugins in 7 or 8 ?

Urgh. No.

Why not use PluginSettings table by default ?

When i mean default : a default table + way to save.
Plugin dev can replace by it's own function.

I already use PluginSettings table to save User related data.

Urgh. No.

Then seem:s best to move (after) PluginSettings to the new ExtendForm system, no ?

We also have AdvancedSettingWidget and GeneralOptionWidget for question editor.

Already based on DTOs.

https://github.com/LimeSurvey/LimeSurvey/tree/master/application/extensions/GeneralOptionWidget/settings

Yes,
And we have 2 separate system with a lot of same code …

PS : AdvancedSettingWidget are for QuestionAttribute

Why not use PluginSettings table by default ?

Hmmm, yes, this works, true.

We also have AdvancedSettingWidget and GeneralOptionWidget for question editor.

Lack of optionnal htmlOptions (html5) and way to extend

What's HTML5 html options?

Extend how? Something else than new attributes?

Oups …
I mean any attributes : min/max for number, pattern, disabled, placeholder etc … :)

Actually, does this even need to be an event? We can have a service provider or module instead, where you can add input elements during system init. So.

// Plugin init
public function init()
{
  App()->extendedForm->append('globalsettings', new Input());
}

And then application code just fetch inputs from that service locator when needed.

Need to be in API then , not in App only.

https://github.com/LimeSurvey/LimeSurvey/blob/master/application/libraries/PluginManager/LimesurveyApi.php

Then it's clearly set it can be used without issue since API version are not updated.

Hm, yes, correct.

Would you prefer to have two new events instead, Denis?

BeforeGlobalSettings - to add new global setting
NewGlobalSettings - to save new global setting

Unsure …
I don't really like 2 events, but like to be allowed to replace the way to save …

See wiith @gabrieljenik

Yes, I've pinged Gabriel on Cliq too.

I think I caught the main ideas.
Still, a summary or a document where the blueprint can be consolidated would be great.

Some issues/discussions I believe I noticed:

Event for each control

I think this will be too complex, either for implementing from the plugin side, as well as for the plgin invocation side.
I would rather go with something like BeforeGlobalSettings, the parameters received there can have all necesarry to add the extra controls.

Also, the saving event needs to be one. We can't have each control with its own saving rutine. (If I understood correctly).

Validation Event

I believe a validation event is missing and an error message flow could be missing.
Validation not only on the custom controls, but also on the standard controls.

Other events

I would also allow a place to add JS and CSS when the form is shown.

Control Setup

I would reuse what we have in the plugin settings.
Actually try to share it as to have it evolved hand in hand.

It could be important to be able to set the position of the extra controls.

AfterCloseForm

Would be great to be able to have an event like that to inject extra HTML

Comments

Can we reuse something from WP hooks maybe?

Also, on the plugin setup, maybe we can have something for enabling and disabling for specific forms?
Would be easier to deactivate in case of mal-functioning.

We could have the plugin to setup the events by providing an object of a specific class?

Hm, yeah, maybe I can try to write a short spec in the manual, including some example code.

Good point about validation. I guess the input objects could throw an exception, like InvalidInputException, and then the application code could catch it and show a warning.

Scripts could also be added as objects, but classes like class ScriptInput that output <script> at rendering. Same for CSS.

I wanted to avoid injecting raw HTML. But I guess we can do a RawHtml class too, for those cases. :d

I was thinking of trying to re-use the plugin settings widget, if possible. Would have to look more closely.

Some more conceptual example code:

class MyPlugin ...
public function init() {
  App()->extendForm->add('globalsettings', new RawHtmlInput('<h1>Moo</h1>'));
  App()->extendForm->add('globalsettings', new ScriptInput($fileLocation));
  App()->extendForm->add('globalsettings', new CheckboxInput('checkbox_id', function ($request) { throw new InvalidInputException("Wrong!")); }));
}

Then in the global settings view file, you'd have something like

<?= PluginInputWidget->render(App()>->extendForm->get('globalsettings'); ?>

And in the controller save action:

// Loops all input objects associated to "globalsettings" and apply their save() methods.
// Also catches the InvalidInputException and adds warning/error messages
App()->extendForm->applySave('globalsettings', $request, $database);  

Hm, maybe formInputService is a better module name than extendForm.

And also, maybe applySave() must return bool rather then echoing warning HTML, so that each controller can decide what to do with the information. Might be different.

Good point about validation. I guess the input objects could throw an exception, like InvalidInputException, and then the application code could catch it and show a warning.

I was thinking something a but more similar to how Yii validations work.
That exception mechanism you mention seems a bit more complicated and not sure if will allow to handle multiple error messages.

Some more conceptual example code:

By how you wrote it there, seems like you are adding individual fields, as opposed to adding an extension, which has fields, validations, ....
I am more inclined towards the later, as some settings or behaviour will be attached to the extension.

Also, how to handle multuple extensions to the same form?
Just multiple plugins?

I was thinking something a but more similar to how Yii validations work.

Hm, you're thinking of active record rules? Or the form functionality?

That exception mechanism you mention seems a bit more complicated and not sure if will allow to handle multiple error messages.

No problem, the formExtensionService module will just loop the inputs and insert a message for each exception it catches. :)

By how you wrote it there, seems like you are adding individual fields, as opposed to adding an extension, which has fields, validations, ....

Yeah, the formExtensionService module would be core code, that accepts individual fields from the plugin init() function.
Maybe you can show some pseudocode or concept on how you'd like to add an extension from the plugin itself?

Also, how to handle multiple extensions to the same form?

The formExtensionService maps form names to input arrays, like

class FormExtensionService {
  private $extensions = []
  public function add(string $formName, FormExtensionInterface $input) {
    $this->extensions[$formName][] = $input;
  }
}

But we would probably also have to consider tabs. So $formName might be "globalsettings", or like "globalsettings.email_settings", where the word after the dot is the name of the tab.

Some issues:

• Localization and label
• Permission

Work started on branch feature/18204-extend-form, trying to use the plugin SettingsWidget at first

As you can see, the SettingsWidget HTML is not adapted to global settings form HTML.

Missing:

• Populate from database (need a load() function?)
• Conditions for viewing (might include permission, so maybe a function for condition() or such...?)

And of course each stupid form has some extra CSS classes that must be obeyed... Hm, maybe factor out render-classes? That can be populated from the form name? So form "globalsettings" has render classes for each input type?

Got something basic running now. Used the exampleSettings plugin, with this code in init():

        Yii::app()->formExtensionService->add(
            'globalsettings.general',
            new TextInput(
                'myinput',
                'Label',
                // Save function
                function($request, $connection) {
                    SettingGlobal::setSetting('myinput', $request->getPost('myinput'));
                },
                // Load function
                function () {
                    return getGlobalSetting('myinput');
                }
            )
        );

Here's the renderer class for global settings: https://github.com/LimeSurvey/LimeSurvey/blob/feature/18204-extend-form/application/libraries/FormExtension/Inputs/GlobalSettingsRenderer.php

Here's the renderer class for global settings:

Seems great !
Lot of work , but after : just some file to update to BS6 (for example)

And of course each stupid form has some extra CSS classes that must be obeyed

Ye s: but here : maybe all form need to have same presentation, same css class etc …

Lot of work , but after : just some file to update to BS6 (for example)

Yes, the plugin authors would not have to touch the plugins to make sure the HTML is up to date, then. :)

Hm, if save() is taken care of in the plugins, beforeControllerAction(), then it could be enough with this widget snippet to make a form extensible (assuming there's a renderer for that form):

<?= FormExtensionWidget::render(
    App()->formExtensionService->getAll('globalsettings.general'),
    new GlobalSettingsRenderer()
); ?>

Else you need the save code in the controller too:

Yii::app()->formExtensionService->applySave('globalsettings', $request);

 Ye s: but here : maybe all form need to have same presentation, same css class etc …

There should be a DefaultRenderer class that should work in most places...

The downside of letting the FormExtensionService class add flash messages - in the pic, the example input threw an exception, and now we have two messages - one success and on failing. :/

But this is necessary to make the change as small as possible to add support to customize one of the forms.

Hm, I also made applySave() return a bool for success, so that the controller can adapt, if it wants to.

Nice work!!!

Please find some comments:

https://bugs.limesurvey.org/view.php?id=18204#c71617
Got something basic running now. Used the exampleSettings plugin, with this code in init():

You would need to do a save and a run for every new field?
I believe the load() and save() should be at form-extension level.

https://bugs.limesurvey.org/view.php?id=18204#c71614
Missing

What about setting default values?

https://bugs.limesurvey.org/view.php?id=18204#c71618
Renderer

Is there a place where plugin authors can set which renderer to use per field?

https://bugs.limesurvey.org/view.php?id=18204#c71625
The downside of letting the FormExtensionService class add flash messages

Maybe we can show a summary instead of 2 separated flash messages.
I believe that could be possible. I remember working on something close to that couple of months ago.

You would need to do a save and a run for every new field?

Ya, it's not performant. It's designed to have minimal code impact on the core code-base, so that's easy to add support for more forms, with a very small PR.

What about setting default values?

Default value can be part of the load() function. :) Just return a default value if non is found in database.

Is there a place where plugin authors can set which renderer to use per field?

That would defeat the purpose. :D The purpose is to make plugin author oblivious to the html, since it might be different in different LS versions, and for different forms.

Maybe we can show a summary instead of 2 separated flash messages.

Maybe, not sure if that would require more changes to the controllers tho. Or standardize it somehow.

Do we really need classes for JS and CSS scripts? That can also be done in beforeControllerAction, right?

You would need to do a save and a run for every new field?

Ya, it's not performant. It's designed to have minimal code impact on the core code-base, so that's easy to add support for more forms, with a very small PR.

Where is that hook called from?
Can you point me out? As to get some context

Is there a place where plugin authors can set which renderer to use per field?

That would defeat the purpose. :D The purpose is to make plugin author oblivious to the html, since it might be different in different LS versions, and for different forms.

Yes, but it will allow to do easier customizations per field if needed and wanted

Maybe we can show a summary instead of 2 separated flash messages.

Maybe, not sure if that would require more changes to the controllers tho. Or standardize it somehow.

We can think of something for that I believe. Standarized. Don't think of that as the reatest challenge
Still, of course can be left for later.

What about validations?
How can we stop everything from being saved (std + custom)?

How can we stop everything from being saved (std + custom)?

You can't. That's also a thing that causes too much code impact to core code. You can only prevent save + validate in your save() method, per input field.
You have a use-case for preventing the entire save from happening?
I mean, you can always throw an Exception exception, to stop the entire process. :)

Yes, but it will allow to do easier customizations per field if needed and wanted

You have the RawHtml thing for that, I guess. Currently it doesn't try to save, but I can change that, if you want. Or you use beforeActionController to save it.

Where is that hook called from?

Here's the hook for saving: https://github.com/LimeSurvey/LimeSurvey/blob/feature/18204-extend-form/application/controllers/admin/globalsettings.php#L408

Here's the hook for rendering: https://github.com/LimeSurvey/LimeSurvey/blob/feature/18204-extend-form/application/views/admin/globalsettings/_general.php#L377

So you only need two small changes to support a new form (but, sadly, also in this case a new GlobalSettingsRenderer class).

@DenisChenu You have any other comments here, regarding different use-cases? As mentioned, it's adapted for small changes, not to add entire new pages or tabs.

Manual entry here: https://manual.limesurvey.org/Plugins_-_advanced#Form_extension_.28New_in_6_.29

Quick thought: If this approach will be for individual fields being added to specific places... Why not already providing the means for saving it? I mean KeyValue table, saving routine, Loading as well, ...

Quick thought: If this approach will be for individual fields being added to specific places... Why not already providing the means for saving it? I mean KeyValue table, saving routine, Loading as well, ...

Flexibility, mostly. The obvious place to store is in plugin settings storage, but maybe some plugin would like to do some logic or who knows? So passing a save-function gives the dev access to custom logic. But yes, we could include default save-functions, sure. :) new DefaultSaveFunction("input_name"), and make the class implement __invoke.

Another thing to take into account then would be:
The form type: Per User, Global, Per Survey, ...

We could provide the related object on the context.
Also, that would impact the automatic saving routines.

Hmmm, what do you mean with form type? The specific form?

The related form object? If there is such?

Merged into develop, because I need to backport it to our cloud, but we can still make changes. Dev branch won't be released for 2 months at least.

Hmmm, what do you mean with form type? The specific form?
The related form object? If there is such?

Yes. Is this a user level form? A survey level form, ... ?

Yes. Is this a user level form? A survey level form, ... ?

Aha. What's the related use-case? I see your point, but the usefulness is not obvious to me, sorry. :)

This has been merged into develop. Branch deleted.