Handle plugin settings with up to date patterns

[jardakotesovec]

[jardakotesovec]

Goal is update plugin settings to use vue.js on the frontend and latest patterns for API on the backend

• It requires new FormModal component, which I just added to ui-library (its available in literally latest stable-3_5_0). That component basically handles retrieving and saving the settings from configured end points.
• It exposes get&put endpoint to get and update the plugin settings
• its using api/v1/plugin/allowedUploads url - not sure whether there would be already convention to follow?
• Its using pattern mentioned here to create these endpoints - How should plugins create API endpoints pkp/pkp-lib#11991
• Not sure whether the access check via middleware is sufficient && recommended latest practice
• Compared to previous implementation it adds validation, which is optional, but its nice to have example how that comes together.

@ewhanson @Touhidur - can you please help with reviewing the backend patterns (creating endpoints, permission check, validation) to make sure I got these right?

[ajnyga]

The old way how this was handled with the manage method relied a lot on the Plugin class I think. There is quite a lot of code for each plugin to handle by themselves. Just as a general question would it be possible to have some of this in the core which plugins could use with less code of their own?

[jardakotesovec]

@ajnyga Thats for sure to consider. On the front-end part I addressed that with the FormModal, so unless you need something very custom - you can handle lots of use cases with just configuring the form on php side.

On the API side - there might be opportunity to move something to the Plugin.php. Want to make sure first I am using right building blocks there.. and than maybe we will find some parts that would work for 90% plugins that could be added to the Plugin.php. But not 100% sure yet if we find enough that could be abstracted out to justify abstractions.

[israelcefrin]

Hi @jardakotesovec , just a note about a minor instructions issue. It says:

For example doc;dox;pdf;gif; jpg and click OK

However, the button label is "Save"

[jardakotesovec]

@ajnyga Maybe we could add PkpBasePluginSettingsController or something.. so plugin creator does not need to think about the permissions and endpoint urls. And they could just customise the edit/get methods.

cc @ewhanson - Hi Erik tagging you here in case you have some ideas..

[ewhanson]

Hey @jardakotesovec!

Maybe we could add PkpBasePluginSettingsController or something..

I think a general approach like this is a good idea, but not something I would want to rush in as part of a sprint. If you have some feedback that could go into an issue of what types of things it would be helpful if a plugin provided some scaffolding for (particularly around permissions and endpoints), we could use that as a starting point.

In the meantime, if you haven't already looked at this, there have been some improvements in handling custom plugin API endpoints, added in pkp/pkp-lib#12050.

[ewhanson]

Hey @jardakotesovec, just a couple of thoughts and questions related to some of the backend/API work. Nothing really a "must do," just some ideas. Thanks!

[ewhanson]

I think including plugin in the path is good practice in general to help make the endpoint more specific and it's function more apparent, but my preference would be to have it be plural, e.g. plugins/allowedUploads. This would also be applicable anywhere else the path is used.

I also wonder if it would be good practice (assuming it's used entirely within PHP in this case) to use a constant for the handler path so it's not being manually duplicated across the plugin.

[ewhanson]

Not sure if this matters, but something to consider: it looks like this plugin is not available as a site-wide plugin (which makes sense). What happens if people attempt to access it in a site wide way? Is it impossible to reach that state here (since it's assumed there is a context and that it cannot be null? It seems like this is covered in the API controller fine, but thought I would check.

[ewhanson]

I don't know if this changes anything based on the updates here, but how does the plugin handle an empty string for this value? Is an empty string distinct from the idea of a null value? (Not necessarily in the form field, but for the setting value in general?)

[ajnyga]

These permissions should probably follow somehow whether the plugin is a site level plugin or a context level plugin and like you mentioned elsewhere, this could maybe be logic that each plugin does not have to define by themselves?

[touhidurabir]

These permissions should probably follow somehow whether the plugin is a site level plugin or a context level plugin

kind of but not also fully like that . I do agree that has.context is related to being site/context only but others are more like generic one that control all the routes global requirements .

Actually getRouteGroupMiddleware is build with that purpose where we can can have the general middleware at global level for the controller class so that any routes defined in the getGroupRoutes will have those auto applied .