| Summary: | Having multiple pluginsdir causes plugin_upload to try to write to the opac-tmpl folder | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Michael Hafen <michael.hafen> |
| Component: | Plugin architecture | Assignee: | Michael Hafen <michael.hafen> |
| Status: | CLOSED FIXED | QA Contact: | Joonas Kylmälä <joonas.kylmala> |
| Severity: | trivial | ||
| Priority: | P5 - low | CC: | david, dcook, fridolin.somers, joonas.kylmala, julian.maurice, kyle, m.de.rooy, victor |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
21.11.00,21.05.05,20.11.11
|
|
| Circulation function: | |||
| Attachments: |
Proposed fix
Bug 28303 - Fix plugins system with multiple pluginsdir settings Bug 28303: Add unit test for badtemplatecheck Bug 28303: Fix plugins system with multiple pluginsdir settings |
||
|
Description
Michael Hafen
2021-05-07 20:15:22 UTC
Created attachment 120722 [details] [review] Proposed fix Hi Michael. Could you create a test plan for this? Then I will attempt to sign off. Thanks. David Test plan: 1. edit a koha instance's koha-conf.xml to set enable_plugins to 1 1.a restart memcached service as necessary 2. upload plugin in Administration -> Manage plugins -> Upload plugin 3. plugin will be listed in the Manage plugins page 4. create a directory, for example `mkdir -p /var/lib/koha-common/plugins` 5. edit koha-conf.xml to add a pluginsdir tag pointing to the directory created in step 4 5.a restart memcached service as necessary 6. uninstall plugin 7. try to upload plugin again. Observe error message to verify that the Apache user can write to the plugins directory. 8. apply patch 9. try to upload plugin again. It should succeed. Created attachment 123444 [details] [review] Bug 28303 - Fix plugins system with multiple pluginsdir settings C4/Templates::badtemplatecheck mucks with the config('pluginsdir') array ref. This makes sure it operates on a copy of the array. Signed-off-by: David Nind <david@davidnind.com> Comment on attachment 120722 [details] [review] Proposed fix Not sure what happened here - obsoleting patch so there is only one (the signed off one). Thanks Michael for the test plan! I have now tested and signed off. Testing notes (using koha-testing-docker): - Step 7: . I didn't see an error (maybe because I changed the user/group for the new directory to match the koha instance) . The plugin looked okay (I used ByWaterSolutions' dev-koha-plugin-kitchen-sink) but there were errors when i tried to do anything with it - it looks like it was uploaded to koha-tmpl/opac-tmpl - After applying the patch and restarting the plugin showed as having errors. After reinstalling the plugin everything was back to working as expected and i could install other plugins. Manually deleted the plugin files added to koha-tmpl/opac-tmpl/Koha/ David Looks good but missing unit test and fails QA script: * Commit title does not start with 'Bug XXXXX: ' - b73fd7c915 Created attachment 125667 [details] [review] Bug 28303: Add unit test for badtemplatecheck This tests that pluginsdir value is not modified by the call to badtemplatecheck to make sure badtemplatecheck operates with its own copy of pluginsdir configuration value. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 125668 [details] [review] Bug 28303: Fix plugins system with multiple pluginsdir settings C4/Templates::badtemplatecheck mucks with the config('pluginsdir') array ref. This makes sure it operates on a copy of the array. To test: 1) $ prove t/db_dependent/Templates.t Signed-off-by: David Nind <david@davidnind.com> JK: Fix commit message styling and add test plan Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> (In reply to Joonas Kylmälä from comment #7) > Looks good but missing unit test and fails QA script: > > * Commit title does not start with 'Bug XXXXX: ' - b73fd7c915 I added the unit test and fixed the commit title. Passing QA. Thank you, Joonas! Pushed to master for 21.11, thanks to everybody involved! Pushed to 21.05.x for 21.05.05 Pushed to 20.11.x for 20.11.11 Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed. *** Bug 28143 has been marked as a duplicate of this bug. *** |