Bug 20968 - Plugins: Add hooks to enable plugin integration into catalogue
Summary: Plugins: Add hooks to enable plugin integration into catalogue
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Andrew Isherwood
QA Contact: Kyle M Hall
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 21900 21905
  Show dependency treegraph
 
Reported: 2018-06-20 11:46 UTC by Andrew Isherwood
Modified: 2018-11-29 10:19 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Sponsored by PTFS Europe


Attachments
Add hooks into cat-toolbar.inc to display buttons (4.44 KB, patch)
2018-06-25 12:45 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Add hooks into cat-toolbar.inc to display buttons (5.05 KB, patch)
2018-06-25 12:50 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: (follow-up) Respond to feedback (1.82 KB, patch)
2018-06-27 16:17 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Add unit tests (2.06 KB, patch)
2018-06-27 17:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button (3.19 KB, patch)
2018-06-27 17:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button (3.18 KB, patch)
2018-06-28 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Add unit tests (2.05 KB, patch)
2018-06-28 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: (follow-up) Replace tabs with spaces (6.15 KB, patch)
2018-06-29 08:24 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Add hooks into cat-toolbar.inc to display buttons (5.13 KB, patch)
2018-06-29 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: (follow-up) Respond to feedback (1.90 KB, patch)
2018-06-29 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: Add unit tests (2.18 KB, patch)
2018-06-29 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button (3.31 KB, patch)
2018-06-29 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: (follow-up) Replace tabs with spaces (6.22 KB, patch)
2018-06-29 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: Add hooks into cat-toolbar.inc to display buttons (5.05 KB, patch)
2018-08-14 12:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: (follow-up) Respond to feedback (1.82 KB, patch)
2018-08-14 12:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Add unit tests (2.05 KB, patch)
2018-08-14 12:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button (3.18 KB, patch)
2018-08-14 12:28 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: (follow-up) Replace tabs with spaces (6.00 KB, patch)
2018-08-14 12:28 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Add hooks into cat-toolbar.inc to display buttons (5.05 KB, patch)
2018-09-06 09:55 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: (follow-up) Respond to feedback (1.82 KB, patch)
2018-09-06 09:55 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Add unit tests (2.05 KB, patch)
2018-09-06 09:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button (3.18 KB, patch)
2018-09-06 09:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: (follow-up) Replace tabs with spaces (6.00 KB, patch)
2018-09-06 09:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20968: Add hooks into cat-toolbar.inc to display buttons (5.03 KB, patch)
2018-10-02 06:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20968: (follow-up) Respond to feedback (1.82 KB, patch)
2018-10-02 06:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20968: Add unit tests (2.10 KB, patch)
2018-10-02 06:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button (3.18 KB, patch)
2018-10-02 06:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20968: (follow-up) Replace tabs with spaces (5.99 KB, patch)
2018-10-02 06:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20968: Add hooks into cat-toolbar.inc to display buttons (5.10 KB, patch)
2018-10-11 10:33 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20968: (follow-up) Respond to feedback (1.88 KB, patch)
2018-10-11 10:33 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20968: Add unit tests (1.92 KB, patch)
2018-10-11 10:33 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button (3.23 KB, patch)
2018-10-11 10:33 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20968: (follow-up) Replace tabs with spaces (6.05 KB, patch)
2018-10-11 10:33 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20968: Add hooks into cat-toolbar.inc to display buttons (5.15 KB, patch)
2018-10-15 16:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: (follow-up) Respond to feedback (1.95 KB, patch)
2018-10-15 16:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: Add unit tests (2.23 KB, patch)
2018-10-15 16:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button (3.30 KB, patch)
2018-10-15 16:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20968: (follow-up) Replace tabs with spaces (6.11 KB, patch)
2018-10-15 16:19 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2018-06-20 11:46:31 UTC
This bug will add Koha Plugin hooks into catalogue related code allowing integration into areas such as koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc
Comment 1 Andrew Isherwood 2018-06-25 12:45:03 UTC
Created attachment 76374 [details] [review]
Add hooks into cat-toolbar.inc to display buttons

This commit adds hooks into cat-toolbar.inc to allow the display of
arbitrary buttons as supplied by plugins within the
intranet_catalog_biblio_enhancements class that provide the method
get_toolbar_button
Comment 2 Andrew Isherwood 2018-06-25 12:50:11 UTC
Created attachment 76375 [details] [review]
Bug 20968: Add hooks into cat-toolbar.inc to display buttons

This commit adds hooks into cat-toolbar.inc to allow the display of
arbitrary buttons as supplied by plugins within the
intranet_catalog_biblio_enhancements class that provide the method
get_toolbar_button

To test:

1) Download the plugin from
https://github.com/PTFS-Europe/cla-permissions-check-plugin
2) Follow the installation instructions from:
https://github.com/PTFS-Europe/cla-permissions-check-plugin
3) TEST: Observe that the "CLA Check Permissions" button takes the user
to the Check Permissions page and a modal displays containing the
results of the query
4) Remove the API key from the plugin "Configuration" page
5) TEST: Ensure that the "CLA Check Permissions" button takes the user
to the Check Permissions page and message displays informing the user
that "Did not receive required request parameters"
Comment 3 Kyle M Hall 2018-06-27 12:57:59 UTC
I see a few issues the shouldn't be hard to resolve:
1) Unit tests ( they are super simple, but should be there anyway )
2) Why test for the method intranet_catalog_biblio_enhancements, but then call the method get_toolbar_button? A) You can check for the method get_toolbar_button directly, and B) You are calling get_toolbar_button twice per plugin, which seems like waste of cycles.
3) After resolving 1 and 2, could you also make pull request to the kitchen sink plugin (https://github.com/bywatersolutions/koha-plugin-kitchen-sink) to add an example to it? That plugin is basically out documentation for the plugin system's functionality at this time.
Comment 4 Kyle M Hall 2018-06-27 13:02:13 UTC
4) I forgot to make explicit that it seems like the method intranet_catalog_biblio_enhancements is not really necessary and should be removed and replaced with a GetPlugins call for get_toolbar_button instead.

5) Can you make the method names a bit more explicit? I think get_opac_toolbar_button would be better than get_toolbar_button so future developers have a bit better an idea what it does without further research.
Comment 5 Kyle M Hall 2018-06-27 13:09:25 UTC
On further inspection, I go with something even more specific, get_opac_catalog_toolbar_button, which completely disambiguates where this button will appear.
Comment 6 Andrew Isherwood 2018-06-27 14:51:29 UTC
Hi Kyle

Many thanks for this feedback, very helpful indeed.

1) Yes indeed, I completely overlooked tests, I'll address this.
2) Good point, well spotted. I'll fix this.
3) Yes, will do
4) I'm not clear whether you're suggesting we should ditch the intranet_catalog_biblio_enhancements method entirely. I don't think we can do this as we need to be able to test in plugins-home.pl/.tt for plugins that are within this class. If you just meant I should drop it from detail.pm as mentioned in 2), yes, I will do this.
5) Yes, good idea, will do

Thanks again. I'll update here once this is all done.
Comment 7 Andrew Isherwood 2018-06-27 15:50:26 UTC
Hi Kyle

Looking at 1), I'm not sure writing tests for this tiny amount of core integration is even possible. All we're doing is populating a template parameter under a certain condition, I don't think we can meaningfully test that.

Any thoughts?
Comment 8 Andrew Isherwood 2018-06-27 16:17:16 UTC
Created attachment 76530 [details] [review]
Bug 20968: (follow-up) Respond to feedback

This patch:

- Removes the unecessary testing for
intranet_catalog_biblio_enhancements method in detail.pl
- Renames get_toolbar_button to get_intranet_catalog_toolbar_button
Comment 9 Kyle M Hall 2018-06-27 17:24:58 UTC
Created attachment 76531 [details] [review]
Bug 20968: Add unit tests
Comment 10 Kyle M Hall 2018-06-27 17:25:06 UTC
Created attachment 76532 [details] [review]
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button
Comment 11 Kyle M Hall 2018-06-27 17:29:13 UTC
> 4) I'm not clear whether you're suggesting we should ditch the
> intranet_catalog_biblio_enhancements method entirely. I don't think we can
> do this as we need to be able to test in plugins-home.pl/.tt for plugins
> that are within this class. If you just meant I should drop it from
> detail.pm as mentioned in 2), yes, I will do this.

I didn't fully understand it's purpose at first. What you've done in the followup is excellent.

I've added to more patches for you, one that add's those unit tests, and another that renames get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button

I know that's quite wordy, but it keeps the naming convention to be in line with the naming convention for the opac payment methods.

If you these followups work for you, I think we'll be good to go! Just sign off on my patches, I'll sign off on yours and we can let someone else QA. Once it's passed qa, I'll add those methods to the Kitchen Sink plugin as well!
Comment 12 Kyle M Hall 2018-06-27 17:33:31 UTC
Apologies for all the typos in my last reply. It's just one of those days I guess ;)
Comment 13 Andrew Isherwood 2018-06-28 11:31:36 UTC
Created attachment 76540 [details] [review]
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button
Comment 14 Andrew Isherwood 2018-06-28 11:31:48 UTC
Created attachment 76541 [details] [review]
Bug 20968: Add unit tests
Comment 15 Andrew Isherwood 2018-06-28 11:34:08 UTC
Hi Kyle

Perfect, thanks very much for that.

I've tested and signed off (I think) your patches.

Cheers
Comment 16 Andrew Isherwood 2018-06-29 08:24:30 UTC
Created attachment 76555 [details] [review]
Bug 20968: (follow-up) Replace tabs with spaces
Comment 17 Kyle M Hall 2018-06-29 13:17:09 UTC
Created attachment 76564 [details] [review]
Bug 20968: Add hooks into cat-toolbar.inc to display buttons

This commit adds hooks into cat-toolbar.inc to allow the display of
arbitrary buttons as supplied by plugins within the
intranet_catalog_biblio_enhancements class that provide the method
get_toolbar_button

To test:

1) Download the plugin from
https://github.com/PTFS-Europe/cla-permissions-check-plugin
2) Follow the installation instructions from:
https://github.com/PTFS-Europe/cla-permissions-check-plugin
3) TEST: Observe that the "CLA Check Permissions" button takes the user
to the Check Permissions page and a modal displays containing the
results of the query
4) Remove the API key from the plugin "Configuration" page
5) TEST: Ensure that the "CLA Check Permissions" button takes the user
to the Check Permissions page and message displays informing the user
that "Did not receive required request parameters"

https://bugs.koha-community.org/show_bug.cgi?id=20968

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Kyle M Hall 2018-06-29 13:17:16 UTC
Created attachment 76565 [details] [review]
Bug 20968: (follow-up) Respond to feedback

This patch:

- Removes the unecessary testing for
intranet_catalog_biblio_enhancements method in detail.pl
- Renames get_toolbar_button to get_intranet_catalog_toolbar_button

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Kyle M Hall 2018-06-29 13:17:19 UTC
Created attachment 76566 [details] [review]
Bug 20968: Add unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 20 Kyle M Hall 2018-06-29 13:17:22 UTC
Created attachment 76567 [details] [review]
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 21 Kyle M Hall 2018-06-29 13:17:25 UTC
Created attachment 76568 [details] [review]
Bug 20968: (follow-up) Replace tabs with spaces

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Jonathan Druart 2018-07-20 16:46:01 UTC
Applying: Bug 20968: (follow-up) Replace tabs with spaces
error: sha1 information is lacking or useless (catalogue/detail.pl).
error: could not build fake ancestor
Patch failed at 0001 Bug 20968: (follow-up) Replace tabs with spaces
Comment 23 Andrew Isherwood 2018-08-08 08:54:54 UTC
Any clue as to why this patch isn't applying for you Jonathan? It's literally replacing tabs with spaces, so no idea why it wouldn't apply. Presumably since Kyle signed it off, it applied for him?
Comment 24 Kyle M Hall 2018-08-08 10:57:57 UTC
(In reply to Andrew Isherwood from comment #23)
> Any clue as to why this patch isn't applying for you Jonathan? It's
> literally replacing tabs with spaces, so no idea why it wouldn't apply.
> Presumably since Kyle signed it off, it applied for him?

It longer applies for me either. Here's the conflict that needs resolved in detail.pl:

313 <<<<<<< HEAD
314
315
316     if ( $analyze ) {
317         # count if item is used in analytical bibliorecords
318         # The 'countanalytics' flag is only used in the templates if analyze is set
319         my $countanalytics = C4::Context->preference('EasyAnalyticalRecords') ? GetAnalyticsCount($item->{itemnumber}) : 0;
320         if ($countanalytics > 0){
321             $analytics_flag=1;
322             $item->{countanalytics} = $countanalytics;
323         }
324 =======
325
326     #count if item is used in analytical bibliorecords
327     my $countanalytics= GetAnalyticsCount($item->{itemnumber});
328     if ($countanalytics > 0){
329         $analytics_flag=1;
330         $item->{countanalytics} = $countanalytics;
331 >>>>>>> Bug 20968: (follow-up) Replace tabs with spaces
Comment 25 Andrew Isherwood 2018-08-14 12:27:44 UTC
Created attachment 77782 [details] [review]
Bug 20968: Add hooks into cat-toolbar.inc to display buttons

This commit adds hooks into cat-toolbar.inc to allow the display of
arbitrary buttons as supplied by plugins within the
intranet_catalog_biblio_enhancements class that provide the method
get_toolbar_button

To test:

1) Download the plugin from
https://github.com/PTFS-Europe/cla-permissions-check-plugin
2) Follow the installation instructions from:
https://github.com/PTFS-Europe/cla-permissions-check-plugin
3) TEST: Observe that the "CLA Check Permissions" button takes the user
to the Check Permissions page and a modal displays containing the
results of the query
4) Remove the API key from the plugin "Configuration" page
5) TEST: Ensure that the "CLA Check Permissions" button takes the user
to the Check Permissions page and message displays informing the user
that "Did not receive required request parameters"
Comment 26 Andrew Isherwood 2018-08-14 12:27:52 UTC
Created attachment 77783 [details] [review]
Bug 20968: (follow-up) Respond to feedback

This patch:

- Removes the unecessary testing for
intranet_catalog_biblio_enhancements method in detail.pl
- Renames get_toolbar_button to get_intranet_catalog_toolbar_button
Comment 27 Andrew Isherwood 2018-08-14 12:27:57 UTC
Created attachment 77784 [details] [review]
Bug 20968: Add unit tests
Comment 28 Andrew Isherwood 2018-08-14 12:28:03 UTC
Created attachment 77785 [details] [review]
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button
Comment 29 Andrew Isherwood 2018-08-14 12:28:09 UTC
Created attachment 77786 [details] [review]
Bug 20968: (follow-up) Replace tabs with spaces
Comment 30 Andrew Isherwood 2018-08-14 12:29:11 UTC
Rebased on top of master.

To test:

1) Download the plugin from
https://github.com/PTFS-Europe/cla-permissions-check-plugin
2) Follow the installation instructions from:
https://github.com/PTFS-Europe/cla-permissions-check-plugin
3) TEST: Observe that the "CLA Check Permissions" button takes the user
to the Check Permissions page and a modal displays containing the
results of the query
4) Remove the API key from the plugin "Configuration" page
5) TEST: Ensure that the "CLA Check Permissions" button takes the user
to the Check Permissions page and message displays informing the user
that "Did not receive required request parameters"
Comment 31 Andrew Isherwood 2018-09-06 09:55:54 UTC
Created attachment 78457 [details] [review]
Bug 20968: Add hooks into cat-toolbar.inc to display buttons

This commit adds hooks into cat-toolbar.inc to allow the display of
arbitrary buttons as supplied by plugins within the
intranet_catalog_biblio_enhancements class that provide the method
get_toolbar_button

To test:

1) Download the plugin from
https://github.com/PTFS-Europe/cla-permissions-check-plugin
2) Follow the installation instructions from:
https://github.com/PTFS-Europe/cla-permissions-check-plugin
3) TEST: Observe that the "CLA Check Permissions" button takes the user
to the Check Permissions page and a modal displays containing the
results of the query
4) Remove the API key from the plugin "Configuration" page
5) TEST: Ensure that the "CLA Check Permissions" button takes the user
to the Check Permissions page and message displays informing the user
that "Did not receive required request parameters"
Comment 32 Andrew Isherwood 2018-09-06 09:55:59 UTC
Created attachment 78458 [details] [review]
Bug 20968: (follow-up) Respond to feedback

This patch:

- Removes the unecessary testing for
intranet_catalog_biblio_enhancements method in detail.pl
- Renames get_toolbar_button to get_intranet_catalog_toolbar_button
Comment 33 Andrew Isherwood 2018-09-06 09:56:05 UTC
Created attachment 78459 [details] [review]
Bug 20968: Add unit tests
Comment 34 Andrew Isherwood 2018-09-06 09:56:10 UTC
Created attachment 78460 [details] [review]
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button
Comment 35 Andrew Isherwood 2018-09-06 09:56:15 UTC
Created attachment 78461 [details] [review]
Bug 20968: (follow-up) Replace tabs with spaces
Comment 36 Martin Renvoize 2018-10-02 06:10:09 UTC
Created attachment 79804 [details] [review]
Bug 20968: Add hooks into cat-toolbar.inc to display buttons

This commit adds hooks into cat-toolbar.inc to allow the display of
arbitrary buttons as supplied by plugins within the
intranet_catalog_biblio_enhancements class that provide the method
get_toolbar_button

To test:

1) Download the plugin from
https://github.com/PTFS-Europe/cla-permissions-check-plugin
2) Follow the installation instructions from:
https://github.com/PTFS-Europe/cla-permissions-check-plugin
3) TEST: Observe that the "CLA Check Permissions" button takes the user
to the Check Permissions page and a modal displays containing the
results of the query
4) Remove the API key from the plugin "Configuration" page
5) TEST: Ensure that the "CLA Check Permissions" button takes the user
to the Check Permissions page and message displays informing the user
that "Did not receive required request parameters"
Comment 37 Martin Renvoize 2018-10-02 06:10:13 UTC
Created attachment 79805 [details] [review]
Bug 20968: (follow-up) Respond to feedback

This patch:

- Removes the unecessary testing for
intranet_catalog_biblio_enhancements method in detail.pl
- Renames get_toolbar_button to get_intranet_catalog_toolbar_button
Comment 38 Martin Renvoize 2018-10-02 06:10:18 UTC
Created attachment 79806 [details] [review]
Bug 20968: Add unit tests
Comment 39 Martin Renvoize 2018-10-02 06:10:22 UTC
Created attachment 79807 [details] [review]
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button
Comment 40 Martin Renvoize 2018-10-02 06:10:26 UTC
Created attachment 79808 [details] [review]
Bug 20968: (follow-up) Replace tabs with spaces
Comment 41 Martin Renvoize 2018-10-02 06:12:34 UTC
Rebased upon master
Comment 42 Andrew Isherwood 2018-10-11 08:54:08 UTC
More detailed test plan:

- Follow the installation and configuration instructions from: https://github.com/PTFS-Europe/cla-permissions-check-plugin
- Go to the bib details for a record, selecting the "Normal" tab
- You should see a "CLA Permissions Check" button in the button bar above the bib details, click it
- This should take you to a new screen and a modal should load containing the results of the permissions check
=> TEST: Observe that the "CLA Check Permissions" button takes the user to the Check Permissions page and a modal displays containing the results of the query
- In order to perform the second test, it is necessary to remove the API key previously entered, so navigate back to the plugin configuration screen and remove the API, then click "Save configuration"
- Navigate back to the bib record and click the "CLA permissions check" button
=> TEST: Ensure that the "CLA Check Permissions" button takes the user to the Check Permissions page and message displays informing the user that "Did not receive required request parameters"
Comment 43 PTFS Europe Sandboxes 2018-10-11 10:33:36 UTC
Created attachment 80394 [details] [review]
Bug 20968: Add hooks into cat-toolbar.inc to display buttons

This commit adds hooks into cat-toolbar.inc to allow the display of
arbitrary buttons as supplied by plugins within the
intranet_catalog_biblio_enhancements class that provide the method
get_toolbar_button

To test:

1) Download the plugin from
https://github.com/PTFS-Europe/cla-permissions-check-plugin
2) Follow the installation instructions from:
https://github.com/PTFS-Europe/cla-permissions-check-plugin
3) TEST: Observe that the "CLA Check Permissions" button takes the user
to the Check Permissions page and a modal displays containing the
results of the query
4) Remove the API key from the plugin "Configuration" page
5) TEST: Ensure that the "CLA Check Permissions" button takes the user
to the Check Permissions page and message displays informing the user
that "Did not receive required request parameters"

https://bugs.koha-community.org/show_bug.cgi?id=20968

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 44 PTFS Europe Sandboxes 2018-10-11 10:33:42 UTC
Created attachment 80395 [details] [review]
Bug 20968: (follow-up) Respond to feedback

This patch:

- Removes the unecessary testing for
intranet_catalog_biblio_enhancements method in detail.pl
- Renames get_toolbar_button to get_intranet_catalog_toolbar_button

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 45 PTFS Europe Sandboxes 2018-10-11 10:33:47 UTC
Created attachment 80396 [details] [review]
Bug 20968: Add unit tests

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 46 PTFS Europe Sandboxes 2018-10-11 10:33:52 UTC
Created attachment 80397 [details] [review]
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 47 PTFS Europe Sandboxes 2018-10-11 10:33:57 UTC
Created attachment 80398 [details] [review]
Bug 20968: (follow-up) Replace tabs with spaces

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 48 Kyle M Hall 2018-10-15 16:18:34 UTC
Created attachment 80615 [details] [review]
Bug 20968: Add hooks into cat-toolbar.inc to display buttons

This commit adds hooks into cat-toolbar.inc to allow the display of
arbitrary buttons as supplied by plugins within the
intranet_catalog_biblio_enhancements class that provide the method
get_toolbar_button

To test:

1) Download the plugin from
https://github.com/PTFS-Europe/cla-permissions-check-plugin
2) Follow the installation instructions from:
https://github.com/PTFS-Europe/cla-permissions-check-plugin
3) TEST: Observe that the "CLA Check Permissions" button takes the user
to the Check Permissions page and a modal displays containing the
results of the query
4) Remove the API key from the plugin "Configuration" page
5) TEST: Ensure that the "CLA Check Permissions" button takes the user
to the Check Permissions page and message displays informing the user
that "Did not receive required request parameters"

https://bugs.koha-community.org/show_bug.cgi?id=20968

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 49 Kyle M Hall 2018-10-15 16:18:50 UTC
Created attachment 80616 [details] [review]
Bug 20968: (follow-up) Respond to feedback

This patch:

- Removes the unecessary testing for
intranet_catalog_biblio_enhancements method in detail.pl
- Renames get_toolbar_button to get_intranet_catalog_toolbar_button

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 50 Kyle M Hall 2018-10-15 16:18:56 UTC
Created attachment 80617 [details] [review]
Bug 20968: Add unit tests

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 51 Kyle M Hall 2018-10-15 16:19:01 UTC
Created attachment 80618 [details] [review]
Bug 20968: Rename method get_intranet_catalog_toolbar_button to intranet_catalog_biblio_enhancements_toolbar_button

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 52 Kyle M Hall 2018-10-15 16:19:05 UTC
Created attachment 80619 [details] [review]
Bug 20968: (follow-up) Replace tabs with spaces

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 53 Nick Clemens 2018-11-02 10:44:08 UTC
Awesome work all!

Pushed to master for 18.11
Comment 54 Martin Renvoize 2018-11-16 12:13:12 UTC
Pushed to 18.05.x for 18.05.06 as per my policy of backporting easily backportable non-consumer facing plugin support changes (to allow for the greatest span of plugin support).
Comment 55 Fridolin SOMERS 2018-11-28 14:14:48 UTC
Before pushing to 17.11.x, I'd like to see it in kitchen sink plugin
Comment 56 Andrew Isherwood 2018-11-29 08:41:58 UTC
Hi Fridolin - It seems that Kyle has added support for it in the Kitchen Sink plugin. The test plan for Bug 21905 tests this area and requires use of the Kitchen Sink
Comment 57 Fridolin SOMERS 2018-11-29 10:10:54 UTC
(In reply to Andrew Isherwood from comment #56)
> Hi Fridolin - It seems that Kyle has added support for it in the Kitchen
> Sink plugin. The test plan for Bug 21905 tests this area and requires use of
> the Kitchen Sink

Ok i see it today in plugin sources
Comment 58 Fridolin SOMERS 2018-11-29 10:19:23 UTC
Pushed to 17.11.x for 17.11.13

"\\//_ Live long and prosper"