Bug 35331 - Add an ILL table actions plugin hook
Summary: Add an ILL table actions plugin hook
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Pedro Amorim
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 35581
  Show dependency treegraph
 
Reported: 2023-11-14 10:31 UTC by Pedro Amorim
Modified: 2024-03-21 09:40 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00


Attachments
Bug 35331: Add table actions hook to ILL table (5.32 KB, patch)
2023-11-14 11:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: Add plugin hook tests (3.61 KB, patch)
2023-11-14 11:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): (1.97 KB, patch)
2023-11-15 15:26 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): Patron ILL history (2.48 KB, patch)
2023-11-16 10:00 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: Add table actions hook to ILL table (5.37 KB, patch)
2023-11-16 13:18 UTC, David Nind
Details | Diff | Splinter Review
Bug 35331: Add plugin hook tests (3.66 KB, patch)
2023-11-16 13:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): (2.01 KB, patch)
2023-11-16 13:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): Patron ILL history (2.53 KB, patch)
2023-11-16 13:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 35331: Add table actions hook to ILL table (5.43 KB, patch)
2023-11-17 18:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35331: Add table actions hook to ILL table (5.43 KB, patch)
2023-11-17 18:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35331: Add plugin hook tests (3.72 KB, patch)
2023-11-17 18:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): (2.07 KB, patch)
2023-11-17 18:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): Patron ILL history (2.58 KB, patch)
2023-11-17 18:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35331: Add table actions hook to ILL table (5.42 KB, patch)
2024-01-08 15:10 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: Add plugin hook tests (3.64 KB, patch)
2024-01-08 15:10 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): (2.07 KB, patch)
2024-01-08 15:10 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): Patron ILL history (2.58 KB, patch)
2024-01-08 15:11 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: Add table actions hook to ILL table (5.57 KB, patch)
2024-03-06 10:26 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: CSRF follow-up (968 bytes, patch)
2024-03-06 10:26 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: Add plugin hook tests (3.65 KB, patch)
2024-03-06 10:26 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): (2.07 KB, patch)
2024-03-06 10:26 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35331: (QA follow-up): Patron ILL history (2.58 KB, patch)
2024-03-06 10:26 UTC, Pedro Amorim
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro Amorim 2023-11-14 10:31:57 UTC

    
Comment 1 Pedro Amorim 2023-11-14 11:04:10 UTC
Created attachment 158916 [details] [review]
Bug 35331: Add table actions hook to ILL table

Test plan, k-t-d, apply patches:
1) Install FreeForm backend, enable ILLModule sys pref, run:
   bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) Navigate to ILL requests, visit:
   /cgi-bin/koha/ill/ill-requests.pl
3) Click '+New ILL request', select a type, enter an existing cardnumber (e.g. '42') and pick a library. Click 'Create'.
4) Visit ILL requests again, repeat 2)
5) Notice the row for the created request has the already existing action button 'Manage request'
6) Install the ILL actions plugin, located at:
   https://github.com/PTFS-Europe/koha-plugin-ill-actions/releases/tag/v1.0.0
7) Restart plack, run:
   koha-plack --restart kohadev
8) Repeat 4) and 5). Notice the row for the created request now has a new custom action in addition to the already existing core one.
Comment 2 Pedro Amorim 2023-11-14 11:04:13 UTC
Created attachment 158917 [details] [review]
Bug 35331: Add plugin hook tests

prove t/db_dependent/Koha/Plugins/Ill_hooks.t
Comment 3 David Nind 2023-11-15 09:29:50 UTC
Everything works for me, except running the tests - which fail 8-(...

kohadev-koha@kohadevbox:koha(bz35331)$ prove t/db_dependent/Koha/Plugins/Ill_hooks.t
t/db_dependent/Koha/Plugins/Ill_hooks.t .. 1/4 
    #   Failed test 'get_staff_table_actions() should return core action plus a custom plugin actions'
    #   at t/db_dependent/Koha/Plugins/Ill_hooks.t line 52.
    #     Structures begin differing at:
    #          $got->[1]{button_class} = 'btn btn-default btn-sm'
    #     $expected->[1]{button_class} = 'test class'
    # Looks like you failed 1 test of 1.
t/db_dependent/Koha/Plugins/Ill_hooks.t .. 4/4 
#   Failed test 'ill_table_actions hook'
#   at t/db_dependent/Koha/Plugins/Ill_hooks.t line 73.
# Looks like you failed 1 test of 4.
t/db_dependent/Koha/Plugins/Ill_hooks.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/4 subtests 

Test Summary Report
-------------------
t/db_dependent/Koha/Plugins/Ill_hooks.t (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  4
  Non-zero exit status: 1
Files=1, Tests=4,  2 wallclock secs ( 0.02 usr  0.00 sys +  0.93 cusr  0.14 csys =  1.09 CPU)
Result: FAIL
Comment 4 David Nind 2023-11-15 09:32:11 UTC
Updated assignee
Comment 5 Pedro Amorim 2023-11-15 10:07:01 UTC
(In reply to David Nind from comment #3)
> Everything works for me, except running the tests - which fail 8-(...
> 
> kohadev-koha@kohadevbox:koha(bz35331)$ prove
> t/db_dependent/Koha/Plugins/Ill_hooks.t
> t/db_dependent/Koha/Plugins/Ill_hooks.t .. 1/4 
>     #   Failed test 'get_staff_table_actions() should return core action
> plus a custom plugin actions'
>     #   at t/db_dependent/Koha/Plugins/Ill_hooks.t line 52.
>     #     Structures begin differing at:
>     #          $got->[1]{button_class} = 'btn btn-default btn-sm'
>     #     $expected->[1]{button_class} = 'test class'
>     # Looks like you failed 1 test of 1.
> t/db_dependent/Koha/Plugins/Ill_hooks.t .. 4/4 
> #   Failed test 'ill_table_actions hook'
> #   at t/db_dependent/Koha/Plugins/Ill_hooks.t line 73.
> # Looks like you failed 1 test of 4.
> t/db_dependent/Koha/Plugins/Ill_hooks.t .. Dubious, test returned 1 (wstat
> 256, 0x100)
> Failed 1/4 subtests 
> 
> Test Summary Report
> -------------------
> t/db_dependent/Koha/Plugins/Ill_hooks.t (Wstat: 256 Tests: 4 Failed: 1)
>   Failed test:  4
>   Non-zero exit status: 1
> Files=1, Tests=4,  2 wallclock secs ( 0.02 usr  0.00 sys +  0.93 cusr  0.14
> csys =  1.09 CPU)
> Result: FAIL

Thank you so much for testing, David!

I believe the tests may be failing for you because you have the plugin installed.
They pass for me without having the plugin installed, as I believe that will be the case for CI/CD.
Having said that, I'm not certain this is what's expected, i.e. a particular hook test fails if a plugin implementing it is installed. As this is my first time writing a hook test, there is a chance the tests need to/could be improved!
Comment 6 Pedro Amorim 2023-11-15 15:26:55 UTC
Created attachment 158993 [details] [review]
Bug 35331: (QA follow-up):

Avoid code pattern iterating on plugin responses (see bug 28010)
Use plugin call pattern established in bug 28211.

Please utilize version 1.0.1 of the plugin to test this follow-up:
https://github.com/PTFS-Europe/koha-plugin-ill-actions/releases/tag/v1.0.1
Comment 7 David Nind 2023-11-15 18:16:08 UTC
Tests pass if the koha-plugin-ill-actions plugin is disabled.

Note: with version 1.0.1 of the plugin, the new action no longer works[1] (it did previously with v1.0.0), but I don't know if that is relevant for signing off.

I'm not a developer, so I don't know whether tests should pass with and without the koha-plugin-ill-actions plugin enabled.

[1] Page not found error when clicking on new action - "New request for this user":

Page URL: http://127.0.0.1:8081/api/v1/contrib/ill_actions/new_request_for_patron/1

Start of the page content ()no CSS style sheet):

Page Not Found
This application is in development mode and will show internal information to help you with debugging.

None of these routes could generate a response for your GET request for /api/v1/contrib/ill_actions/new_request_for_patron/1, maybe you need to add a new one?
Comment 8 Pedro Amorim 2023-11-16 09:20:47 UTC
(In reply to David Nind from comment #7)
> Tests pass if the koha-plugin-ill-actions plugin is disabled.
> 
> Note: with version 1.0.1 of the plugin, the new action no longer works[1]
> (it did previously with v1.0.0), but I don't know if that is relevant for
> signing off.
> 
> I'm not a developer, so I don't know whether tests should pass with and
> without the koha-plugin-ill-actions plugin enabled.
> 
> [1] Page not found error when clicking on new action - "New request for this
> user":
> 
> Page URL:
> http://127.0.0.1:8081/api/v1/contrib/ill_actions/new_request_for_patron/1
> 
> Start of the page content ()no CSS style sheet):
> 
> Page Not Found
> This application is in development mode and will show internal information
> to help you with debugging.
> 
> None of these routes could generate a response for your GET request for
> /api/v1/contrib/ill_actions/new_request_for_patron/1, maybe you need to add
> a new one?

Once again, thank you David.
I was able to reproduce the problem described, but running:
koha-plack --restart kohadev
after having installed the plugin did the trick for me.

Would you please give that a go? Thank you!
Comment 9 Pedro Amorim 2023-11-16 10:00:57 UTC
Created attachment 159037 [details] [review]
Bug 35331: (QA follow-up): Patron ILL history

With the previous patches applied and successfull test plans, access the users ILL history page, visit:
/cgi-bin/koha/members/ill-requests.pl?borrowernumber=51

Notice you get an error.
Apply this patch, and try again, notice the error is gone and the custom plugin action is also in the patron's ILL history table
Comment 10 Pedro Amorim 2023-11-16 13:18:08 UTC
(In reply to David Nind from comment #7) 
> I'm not a developer, so I don't know whether tests should pass with and
> without the koha-plugin-ill-actions plugin enabled.

If we run the following command (no plugins installed):
prove t/db_dependent/Koha/Plugins

All tests pass.

But, if we then install the kitchensink plugin located at:
https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/releases/tag/v2.3.1

And run the plugin tests again:
prove t/db_dependent/Koha/Plugins

A bunch of them are now failing, because the installed plugin is changing the expected static result of the tests.
In conclusion, I think it's safe to assume that no plugins should be installed when running unit tests.
Comment 11 David Nind 2023-11-16 13:18:59 UTC
Created attachment 159046 [details] [review]
Bug 35331: Add table actions hook to ILL table

Test plan, k-t-d, apply patches:
1) Install FreeForm backend, enable ILLModule sys pref, run:
   bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) Navigate to ILL requests, visit:
   /cgi-bin/koha/ill/ill-requests.pl
3) Click '+New ILL request', select a type, enter an existing cardnumber (e.g. '42') and pick a library. Click 'Create'.
4) Visit ILL requests again, repeat 2)
5) Notice the row for the created request has the already existing action button 'Manage request'
6) Install the ILL actions plugin, located at:
   https://github.com/PTFS-Europe/koha-plugin-ill-actions/releases/tag/v1.0.0
7) Restart plack, run:
   koha-plack --restart kohadev
8) Repeat 4) and 5). Notice the row for the created request now has a new custom action in addition to the already existing core one.

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2023-11-16 13:19:02 UTC
Created attachment 159047 [details] [review]
Bug 35331: Add plugin hook tests

prove t/db_dependent/Koha/Plugins/Ill_hooks.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 David Nind 2023-11-16 13:19:05 UTC
Created attachment 159048 [details] [review]
Bug 35331: (QA follow-up):

Avoid code pattern iterating on plugin responses (see bug 28010)
Use plugin call pattern established in bug 28211.

Please utilize version 1.0.1 of the plugin to test this follow-up:
https://github.com/PTFS-Europe/koha-plugin-ill-actions/releases/tag/v1.0.1

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 David Nind 2023-11-16 13:19:08 UTC
Created attachment 159049 [details] [review]
Bug 35331: (QA follow-up): Patron ILL history

With the previous patches applied and successfull test plans, access the users ILL history page, visit:
/cgi-bin/koha/members/ill-requests.pl?borrowernumber=51

Notice you get an error.
Apply this patch, and try again, notice the error is gone and the custom plugin action is also in the patron's ILL history table

Signed-off-by: David Nind <david@davidnind.com>
Comment 16 Pedro Amorim 2023-11-16 15:04:27 UTC
Brilliant, David. Thank you, you're awesome!
Comment 17 Kyle M Hall 2023-11-17 18:26:03 UTC
Created attachment 159105 [details] [review]
Bug 35331: Add table actions hook to ILL table

Test plan, k-t-d, apply patches:
1) Install FreeForm backend, enable ILLModule sys pref, run:
   bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) Navigate to ILL requests, visit:
   /cgi-bin/koha/ill/ill-requests.pl
3) Click '+New ILL request', select a type, enter an existing cardnumber (e.g. '42') and pick a library. Click 'Create'.
4) Visit ILL requests again, repeat 2)
5) Notice the row for the created request has the already existing action button 'Manage request'
6) Install the ILL actions plugin, located at:
   https://github.com/PTFS-Europe/koha-plugin-ill-actions/releases/tag/v1.0.0
7) Restart plack, run:
   koha-plack --restart kohadev
8) Repeat 4) and 5). Notice the row for the created request now has a new custom action in addition to the already existing core one.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Kyle M Hall 2023-11-17 18:32:06 UTC
Created attachment 159106 [details] [review]
Bug 35331: Add table actions hook to ILL table

Test plan, k-t-d, apply patches:
1) Install FreeForm backend, enable ILLModule sys pref, run:
   bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) Navigate to ILL requests, visit:
   /cgi-bin/koha/ill/ill-requests.pl
3) Click '+New ILL request', select a type, enter an existing cardnumber (e.g. '42') and pick a library. Click 'Create'.
4) Visit ILL requests again, repeat 2)
5) Notice the row for the created request has the already existing action button 'Manage request'
6) Install the ILL actions plugin, located at:
   https://github.com/PTFS-Europe/koha-plugin-ill-actions/releases/tag/v1.0.0
7) Restart plack, run:
   koha-plack --restart kohadev
8) Repeat 4) and 5). Notice the row for the created request now has a new custom action in addition to the already existing core one.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Kyle M Hall 2023-11-17 18:32:23 UTC
Created attachment 159107 [details] [review]
Bug 35331: Add plugin hook tests

prove t/db_dependent/Koha/Plugins/Ill_hooks.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 20 Kyle M Hall 2023-11-17 18:32:25 UTC
Created attachment 159108 [details] [review]
Bug 35331: (QA follow-up):

Avoid code pattern iterating on plugin responses (see bug 28010)
Use plugin call pattern established in bug 28211.

Please utilize version 1.0.1 of the plugin to test this follow-up:
https://github.com/PTFS-Europe/koha-plugin-ill-actions/releases/tag/v1.0.1

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 21 Kyle M Hall 2023-11-17 18:32:28 UTC
Created attachment 159109 [details] [review]
Bug 35331: (QA follow-up): Patron ILL history

With the previous patches applied and successfull test plans, access the users ILL history page, visit:
/cgi-bin/koha/members/ill-requests.pl?borrowernumber=51

Notice you get an error.
Apply this patch, and try again, notice the error is gone and the custom plugin action is also in the patron's ILL history table

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Pedro Amorim 2024-01-08 15:10:51 UTC Comment hidden (obsolete)
Comment 23 Pedro Amorim 2024-01-08 15:10:53 UTC Comment hidden (obsolete)
Comment 24 Pedro Amorim 2024-01-08 15:10:57 UTC Comment hidden (obsolete)
Comment 25 Pedro Amorim 2024-01-08 15:11:00 UTC Comment hidden (obsolete)
Comment 26 Pedro Amorim 2024-01-08 15:11:28 UTC Comment hidden (obsolete)
Comment 27 Pedro Amorim 2024-03-06 10:26:10 UTC
Created attachment 162812 [details] [review]
Bug 35331: Add table actions hook to ILL table

Test plan, k-t-d, apply patches:
1) Install FreeForm backend, enable ILLModule sys pref, run:
   bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) Navigate to ILL requests, visit:
   /cgi-bin/koha/ill/ill-requests.pl
3) Click '+New ILL request', select a type, enter an existing cardnumber (e.g. '42') and pick a library. Click 'Create'.
4) Visit ILL requests again, repeat 2)
5) Notice the row for the created request has the already existing action button 'Manage request'
6) Install the ILL actions plugin, located at:
   https://github.com/PTFS-Europe/koha-plugin-ill-actions/releases/tag/v1.0.0
7) Restart plack, run:
   koha-plack --restart kohadev
8) Repeat 4) and 5). Notice the row for the created request now has a new custom action in addition to the already existing core one.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 28 Pedro Amorim 2024-03-06 10:26:14 UTC
Created attachment 162813 [details] [review]
Bug 35331: CSRF follow-up

Update method to op
Comment 29 Pedro Amorim 2024-03-06 10:26:17 UTC
Created attachment 162814 [details] [review]
Bug 35331: Add plugin hook tests

prove t/db_dependent/Koha/Plugins/Ill_hooks.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 30 Pedro Amorim 2024-03-06 10:26:19 UTC
Created attachment 162815 [details] [review]
Bug 35331: (QA follow-up):

Avoid code pattern iterating on plugin responses (see bug 28010)
Use plugin call pattern established in bug 28211.

Please utilize version 1.0.1 of the plugin to test this follow-up:
https://github.com/PTFS-Europe/koha-plugin-ill-actions/releases/tag/v1.0.1

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 31 Pedro Amorim 2024-03-06 10:26:22 UTC
Created attachment 162816 [details] [review]
Bug 35331: (QA follow-up): Patron ILL history

With the previous patches applied and successfull test plans, access the users ILL history page, visit:
/cgi-bin/koha/members/ill-requests.pl?borrowernumber=51

Notice you get an error.
Apply this patch, and try again, notice the error is gone and the custom plugin action is also in the patron's ILL history table

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 32 Pedro Amorim 2024-03-06 10:27:40 UTC
Rebased on bug 34478
Comment 33 Katrin Fischer 2024-03-18 08:56:08 UTC
Had to fix a small conflict in the first patch and the unit test due to CSRF changes. This means that if this was to be backported, we'd probably need some small changes.

Fixed commit message: Bug 35331: (QA follow-up): 

This one is confusing. It's a one line change - I tidied it, the QA issue remained:

Small QA script failures:
 FAIL	ill/ill-requests.pl
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 181, now: 182)
Comment 34 Katrin Fischer 2024-03-18 10:05:16 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 35 Fridolin Somers 2024-03-21 09:40:09 UTC
Does not apply easy on 23.11.x, I prefer not backport