Cannot enable plugin in OJS 3.4

pkp / orcidProfile

A plugin to pull ORCID information into a PKP user profile

GNU General Public License v3.0

16 stars 50 forks source link

Cannot enable plugin in OJS 3.4 #276

Closed jonasraoni closed 1 year ago

jonasraoni commented 1 year ago

Describe the bug In a clear installation, it's not possible to enable the plugin, due to a validation that happens within the setEnabled() method.

Solution It should be possible to enable the plugin, otherwise it's not possible to setup the settings which are failing to be validated. If the settings are invalid, the plugin could just skip its normal operation, and perhaps emit an alert.

To Reproduce

Try to enable the plugin
It will get back to disabled without warnings/error messages

Expected behavior I expect the plugin to be enabled right away, the validation can be moved elsewhere.

Origin https://forum.pkp.sfu.ca/t/orcid-profile-plugin-cannot-be-enabled-in-ojs-3-4-0-1/79469/29

jonasraoni commented 1 year ago

@withanage / @asmecher would any of you be able to review the linked PRs?

asmecher commented 1 year ago

@jonasraoni, it looks like some code was rearranged/reformatted in the PR, which makes it hard to read; can you highlight the parts of the PR that corrected the reported problem?

jonasraoni commented 1 year ago

Oh sorry, I've just dropped the setEnabled() override plus another similar approach, and moved the validation check to the plugin initialization: https://github.com/pkp/orcidProfile/pull/277/files#diff-c62bbbc2439724bd77f83a6cf1886cdf24c5eff8207e010372ac84b94c68b91eR103-R112

Looks like it worked fine for a user in the forum.

Also, the build is breaking in stable-3_4_0 (looks like there's a composer dependency requiring PHP >= 8.1), I didn't touch anything there, but I can inspect.

asmecher commented 1 year ago

@jonasraoni, I suspect you need to change the distro in the Travis config from bionic to focal; I had to make a similar change in the webFeed plugin just now.

@withanage, can you review the above, and check into why the automated testing didn't catch this?

asmecher commented 1 year ago

@jonasraoni and @withanage, I ended up manually porting the parts of the PR that Jonas identified to stable-3_4_0. The reformatting made cherry-picking difficult, and Dulip's PR included other things from the main branch that probably shouldn't be back-ported.

Next time, it would be less risky to the stable-3_4_0 branch if...

Development was done first to the stable branch, then forward-ported, rather than vice versa, and
Extra formatting etc. wasn't applied while fixing a bug. (There's more leeway for this in main, but this definitely needed fixing in stable-3_4_0. It'll only get trickier from here as the branches diverge!)

I also put together a Plugin Gallery release for the sake of 3.4.0-0 and 3.4.0-1 users.