Bug 35331 - Add an ILL table actions plugin hook
Summary: Add an ILL table actions plugin hook
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: master
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-01-08 15:11 UTC (History)
5 users (show)

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


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

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
Created attachment 160634 [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 23 Pedro Amorim 2024-01-08 15:10:53 UTC
Created attachment 160635 [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 24 Pedro Amorim 2024-01-08 15:10:57 UTC
Created attachment 160636 [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 25 Pedro Amorim 2024-01-08 15:11:00 UTC
Created attachment 160637 [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 26 Pedro Amorim 2024-01-08 15:11:28 UTC
It no longer applied after bug 35070. Rebased.