Created attachment 160035 [details] ILL 2.0 flow Currently WIP but open to feedback from those interested/available: https://github.com/PTFS-Europe/koha/commits/ill_2.0/ This work depends on bug 35570 (native Standard ILL backend) and is currently being tested with an ILL backend plugin: https://github.com/PTFS-Europe/koha-ill-reprintsdesk/tree/pluginify Even if no one is able to test this at this point, the provided diagram should represent roughly the final intended result, feedback from that alone would be greatly appreciated!
Created attachment 164883 [details] [review] Bug 35604: New Backends.pm class. installed_backends method
Created attachment 164884 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends
Created attachment 164885 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs
Created attachment 164886 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled
Created attachment 164887 [details] [review] Bug 35604: ConfirmAuto workflow stage
Created attachment 164888 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff.
Created attachment 165135 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page
Created attachment 165164 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t squash to tests
Created attachment 165165 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t
Squashed additional tests for Backends::installed_backends() in the "Add tests" patch. Added unit test files commands to commit message.
Created attachment 165294 [details] [review] Bug 35604: New Backends.pm class. installed_backends method
Created attachment 165295 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends
Created attachment 165296 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs
Created attachment 165297 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled
Created attachment 165298 [details] [review] Bug 35604: ConfirmAuto workflow stage
Created attachment 165299 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t
Created attachment 165300 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page
Rebase needed after bug 35151 and bug 35568 pushed to main
Created attachment 166897 [details] [review] Bug 35604: New Backends.pm class. installed_backends method
Created attachment 166898 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends
Created attachment 166899 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs
Created attachment 166900 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled
Created attachment 166901 [details] [review] Bug 35604: ConfirmAuto workflow stage
Created attachment 166902 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t
Created attachment 166903 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page
Rebase needed after bug 34655 and bug 36503
Everything in the test plan works, except for one test. I don't know what this means.... (note: the two plugins from the test plan are still installed). prove t/db_dependent/Koha/ILL/Backends.t t/db_dependent/Koha/ILL/Backends.t .. 1/4 # Failed test 'Only one backend installed, happens to be a plugin' # at t/db_dependent/Koha/ILL/Backends.t line 59. # Structures begin differing at: # $got->[0] = 'PluginBackend' # $expected->[0] = 'Test Plugin' # Failed test 'Two backends are installed, one plugin and one directory backend' # at t/db_dependent/Koha/ILL/Backends.t line 75. # Structures begin differing at: # $got->[1] = 'PluginBackend' # $expected->[1] = 'Test Plugin' # Looks like you failed 2 tests of 2. # Failed test 'installed_backends() tests' # at t/db_dependent/Koha/ILL/Backends.t line 84. # Looks like you failed 1 test of 4. t/db_dependent/Koha/ILL/Backends.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/4 subtests Test Summary Report ------------------- t/db_dependent/Koha/ILL/Backends.t (Wstat: 256 (exited 1) Tests: 4 Failed: 1) Failed test: 4 Non-zero exit status: 1 Files=1, Tests=4, 1 wallclock secs ( 0.01 usr 0.01 sys + 1.12 cusr 0.19 csys = 1.33 CPU) Result: FAIL
Thanks David. I believe plugin tests should be run without having any plugin (at all) installed. The unit test is expecting a particular result but because you have other plugins installed is returning something different. Would you please test it again without any plugin installed? Apologies, should've made this clear in the test plan! Very much appreciated for your testing!!
Created attachment 166995 [details] [review] Bug 35604: New Backends.pm class. installed_backends method Signed-off-by: David Nind <david@davidnind.com>
Created attachment 166996 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends Signed-off-by: David Nind <david@davidnind.com>
Created attachment 166997 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs Signed-off-by: David Nind <david@davidnind.com>
Created attachment 166998 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled Signed-off-by: David Nind <david@davidnind.com>
Created attachment 166999 [details] [review] Bug 35604: ConfirmAuto workflow stage Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167000 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167001 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page Signed-off-by: David Nind <david@davidnind.com>
(In reply to Pedro Amorim from comment #28) > Thanks David. > I believe plugin tests should be run without having any plugin (at all) > installed. > The unit test is expecting a particular result but because you have other > plugins installed is returning something different. > Would you please test it again without any plugin installed? > Apologies, should've made this clear in the test plan! > Very much appreciated for your testing!! Retested without plugins, and test pass. Now signed off. Thanks! David
Created attachment 171326 [details] [review] Bug 35604: New Backends.pm class. installed_backends method Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171327 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171328 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171329 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171330 [details] [review] Bug 35604: ConfirmAuto workflow stage Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171331 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171332 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174515 [details] [review] Bug 35604: New Backends.pm class. installed_backends method Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174516 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174517 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174518 [details] [review] Bug 35604: atomicupdate
Created attachment 174519 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174520 [details] [review] Bug 35604: ConfirmAuto workflow stage Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174521 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174522 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174524 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174792 [details] [review] Bug 35604: Follow-up Consider success message from backend and present it. Add btn-primary class to auto ILL screen
Testing notes (using KTD): 1. Note that the ConfirmAuto tests are now in a different location: prove t/db_dependent/Koha/ILL/Request/Workflow/ConfirmAuto.t
Created attachment 175828 [details] [review] Bug 35604: New Backends.pm class. installed_backends method Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175829 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175830 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175831 [details] [review] Bug 35604: atomicupdate
Created attachment 175832 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175833 [details] [review] Bug 35604: ConfirmAuto workflow stage Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175834 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175835 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175836 [details] [review] Bug 35604: Follow-up Consider success message from backend and present it. Add btn-primary class to auto ILL screen
Created attachment 176036 [details] [review] Bug 35604: (QA follow-up): Cosmetic improvements These are improvements to the confirm auto backend stage screen on the Staff UI. Result of a chat with Tomas about this development.
Created attachment 176261 [details] [review] Bug 35604: New Backends.pm class. installed_backends method Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176262 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176263 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176264 [details] [review] Bug 35604: atomicupdate Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176265 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176266 [details] [review] Bug 35604: ConfirmAuto workflow stage Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176267 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176268 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176269 [details] [review] Bug 35604: Follow-up Consider success message from backend and present it. Add btn-primary class to auto ILL screen Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176270 [details] [review] Bug 35604: (QA follow-up): Cosmetic improvements These are improvements to the confirm auto backend stage screen on the Staff UI. Result of a chat with Tomas about this development. Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176572 [details] [review] Bug 35604: (QA follow-up): Fix find-missing-op-in-forms.t prove xt/find-missing-op-in-forms.t
Created attachment 176662 [details] [review] Bug 35604: (QA follow-up): Prevent more than one 'op' in form This may happen if for example getting to the automatic backend screen from the ILL availability screen. 'op' is already being added, skip it if already exists in ported metadata
Created attachment 176681 [details] [review] Bug 35604: New Backends.pm class. installed_backends method Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176682 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176683 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176684 [details] [review] Bug 35604: atomicupdate Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176685 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176686 [details] [review] Bug 35604: ConfirmAuto workflow stage Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176687 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176688 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176689 [details] [review] Bug 35604: Follow-up Consider success message from backend and present it. Add btn-primary class to auto ILL screen Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176690 [details] [review] Bug 35604: (QA follow-up): Cosmetic improvements These are improvements to the confirm auto backend stage screen on the Staff UI. Result of a chat with Tomas about this development. Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176691 [details] [review] Bug 35604: (QA follow-up): Fix find-missing-op-in-forms.t prove xt/find-missing-op-in-forms.t
Created attachment 176692 [details] [review] Bug 35604: (QA follow-up): Prevent more than one 'op' in form This may happen if for example getting to the automatic backend screen from the ILL availability screen. 'op' is already being added, skip it if already exists in ported metadata
I didn't manage to rebase it using the `auto_rebaser.pl` tool.
Created attachment 178305 [details] [review] Bug 35604: New Backends.pm class. installed_backends method Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178306 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178307 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178308 [details] [review] Bug 35604: atomicupdate Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178309 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178310 [details] [review] Bug 35604: ConfirmAuto workflow stage Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178311 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178312 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178313 [details] [review] Bug 35604: Follow-up Consider success message from backend and present it. Add btn-primary class to auto ILL screen Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178314 [details] [review] Bug 35604: (QA follow-up): Cosmetic improvements These are improvements to the confirm auto backend stage screen on the Staff UI. Result of a chat with Tomas about this development. Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178315 [details] [review] Bug 35604: (QA follow-up): Fix find-missing-op-in-forms.t prove xt/find-missing-op-in-forms.t
Created attachment 178316 [details] [review] Bug 35604: (QA follow-up): Prevent more than one 'op' in form This may happen if for example getting to the automatic backend screen from the ILL availability screen. 'op' is already being added, skip it if already exists in ported metadata
Patches rebased.
A very simple rebase is needed here now.
Created attachment 178707 [details] [review] Bug 35604: New Backends.pm class. installed_backends method Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178708 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178709 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178710 [details] [review] Bug 35604: atomicupdate Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178711 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178712 [details] [review] Bug 35604: ConfirmAuto workflow stage Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178713 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178714 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178715 [details] [review] Bug 35604: Follow-up Consider success message from backend and present it. Add btn-primary class to auto ILL screen Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178716 [details] [review] Bug 35604: (QA follow-up): Cosmetic improvements These are improvements to the confirm auto backend stage screen on the Staff UI. Result of a chat with Tomas about this development. Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178717 [details] [review] Bug 35604: (QA follow-up): Fix find-missing-op-in-forms.t prove xt/find-missing-op-in-forms.t
Created attachment 178718 [details] [review] Bug 35604: (QA follow-up): Prevent more than one 'op' in form This may happen if for example getting to the automatic backend screen from the ILL availability screen. 'op' is already being added, skip it if already exists in ported metadata
Starting with a first code review (always got to start somewhere :)) 0) QA script I am aware that some might be false positives, but I believe there are some that need fixing as well: FAIL installer/data/mysql/atomicupdate/bug_35604.pl FAIL file permissions File must have the exec flag FAIL installer/data/mysql/mandatory/sysprefs.sql FAIL semicolon simicolon found instead of comma at line 879 FAIL koha-tmpl/intranet-tmpl/prog/js/ill-autobackend.js FAIL forbidden patterns forbidden pattern: console.log (line 68) FAIL koha-tmpl/opac-tmpl/bootstrap/js/ill-autobackend.js FAIL forbidden patterns forbidden pattern: console.log (line 68) FAIL t/db_dependent/Koha/ILL/Backends.t FAIL file permissions File must have the exec flag FAIL t/db_dependent/Koha/ILL/Request/Workflow/ConfirmAuto.t FAIL file permissions File must have the exec flag 1) Koha/ILL/Request/Workflow/ConfirmAuto.pm From the code comments this looks like display components handled in a module? Maybe we can talk things through a bit, when I got a little deeper into the feature set. 2) Tiny typo + description => "Add new AutoILLBackendPriority system preferences", We only add one :) 3) System preference - Availability checking is mentioned the first time in the pref description, but not in other places. - type is given as ILLBackends in kohastructure.sql, but as ill-backends in the yaml. If I understand correctly the preference won't work with only Standard. You need at least 2 backends installed and you need the availability check plugin/capability implemented in backend for these, is that correct? 4) Capitalization a) + <li>Make an automatic <a href="/cgi-bin/koha/opac-illrequests.pl?op=create&backend=Standard">Interlibrary loan request</a></li> Also wrong in the old part of the code: <li>Make an <a href="/cgi-bin/koha/opac-illrequests.pl?op=create">Interlibrary loan request</a></li> 5) Translatability + let message = "Error";
Created attachment 179698 [details] [review] Bug 35604: Multiple QA followups Multiple fixes: - File permmissions - Capitalization - Translatability - Use of console.log - Atomic update description typo fix
Created attachment 179699 [details] [review] Bug 35604: Multiple QA followups Multiple fixes: - File permmissions - Capitalization - Translatability - Use of console.log - Atomic update description typo fix
(In reply to Katrin Fischer from comment #116) > 3) System preference > > - Availability checking is mentioned the first time in the pref description, > but not in other places. > - type is given as ILLBackends in kohastructure.sql, but as ill-backends in > the yaml. > > If I understand correctly the preference won't work with only Standard. You > need at least 2 backends installed and you need the availability check > plugin/capability implemented in backend for these, is that correct? Thank you so much for looking Katrin. The pref will work with at least 1 installed backend, and the backend must provide the provides_backend_availability_check capability and implement the availability_check_info method, that's correct. Examples can be found here: https://github.com/PTFS-Europe/koha-ill-libkey-lending-tool/blob/main/Koha/Plugin/Com/PTFSEurope/IncDocs.pm#L142 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/blob/pluginify%2B%2B/Koha/Plugin/Com/PTFSEurope/ReprintsDesk.pm#L225 If only one supporting backend is installed, and the syspref is enabled with that single backend, the automatic backend screen will first check that backend, recommend it if the document being queried is available in it, or default to Standard otherwise, effectivelly making the choice for the Staff member of which backend should be used a particular ILL request.
Created attachment 179720 [details] [review] Bug 35604: (QA follow-up): Remove occurrences of 'auto' and 'automatic' User testing reported that making a UI distinction between 'regular' and 'automatic' ILL requests is redundant. This patch reverts it.
Created attachment 179721 [details] [review] Bug 35604: Multiple QA followups Multiple fixes: - File permmissions - Capitalization - Translatability - Use of console.log - Atomic update description typo fix - Remove occurrences of 'auto' and 'automatic': User testing reported that making a UI distinction between 'regular' and 'automatic' ILL requests is redundant.
Created attachment 180335 [details] [review] Bug 35604: (QA follow-up): Remove remaining 'automatic' occurrences From Hackfest QA session
Created attachment 180451 [details] [review] Bug 35604: New Backends.pm class. installed_backends method Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180452 [details] [review] Bug 35604: New 'ill-backends' syspref type UI rendering of sortable ILL backends Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180453 [details] [review] Bug 35604: New AutoILLBackendPriority sys prefs Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180454 [details] [review] Bug 35604: atomicupdate Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180455 [details] [review] Bug 35604: ILL request creation with AutoILLBackendPriority enabled Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180456 [details] [review] Bug 35604: ConfirmAuto workflow stage Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180457 [details] [review] Bug 35604: Update 'Make an ILL request' link in OPAC search results page Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180458 [details] [review] Bug 35604: Add tests Test plan: Apply patches Staff: 1) Visit the ILL sysprefs page: /cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans 2) Enable ILLModule 3) Check the new AutoILLBackendPriority syspref. Notice it shows "No available backends" 4) Install 2 backend plugins from here: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 https://github.com/PTFS-Europe/koha-ill-reprintsdesk/releases/tag/rc-v3.0.0 5) Repeat 1. Notice the backends now show. 6) Enable both for AutoILLBackendPriority by ticking each respective checkbox. Save. 7) Visit ILL requests: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 8) Press "New auto ILL request" 9) You are presented the "Standard" create form. Pick type "journal" and put '123' on DOI. Enter '42' on cardnumber and pick a library. Click Create. 10) Notice you are now shown the "Confirm auto backend" stage screen. 11) PluginBackend has a 50% chance of being available or not for testing purposes. If it is, it'll suggest PluginBackend, if not, it'll default to Standard. ReprintsDesk will always error because it's missing credentials. 12) Pick any of the backends or accept the suggested one, click 'Create'. The request has been placed in that specific backend. OPAC: 1) Following from the previous test plan, visit OPAC ill requests: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl 2) Click "Create a new auto request" and repeat the same inputs as before. Click 'Create'. 3) Notice you're directed to "Confirming your request" screen. It's doing the same as it does in the Staff UI, except it does not allow the patron to pick a different backend, it'll always selected the first available backend in the priority. 4) Create more requests, you'll notice some of them will land on PluginBackend, others will land on 'Standard' because PluginBackend will sometimes be available and others not. Do more testing: - Reorder the backends in the AutoILLBackendPriority syspref and save, notice they're shown and suggested accordingly after creating a request. - Disable all backends from AutoILLBackendPriority. Notice the original behaviour of requiring the user to select a backend before creating the request is restored on both opac and staff. Run unit tests: prove t/db_dependent/Illrequest/ConfirmAuto.t prove t/db_dependent/Koha/ILL/Backends.t Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180459 [details] [review] Bug 35604: Display success message Consider success message from backend and present it. Add btn-primary class to auto ILL screen Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180460 [details] [review] Bug 35604: (QA follow-up): Cosmetic improvements These are improvements to the confirm auto backend stage screen on the Staff UI. Result of a chat with Tomas about this development. Sponsored-by: NHS England Sponsored-by: PTFS Europe Ltd Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180461 [details] [review] Bug 35604: (QA follow-up): Fix find-missing-op-in-forms.t prove xt/find-missing-op-in-forms.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180462 [details] [review] Bug 35604: (QA follow-up): Prevent more than one 'op' in form This may happen if for example getting to the automatic backend screen from the ILL availability screen. 'op' is already being added, skip it if already exists in ported metadata Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180463 [details] [review] Bug 35604: Multiple QA followups Multiple fixes: - File permmissions - Capitalization - Translatability - Use of console.log - Atomic update description typo fix - Remove occurrences of 'auto' and 'automatic': User testing reported that making a UI distinction between 'regular' and 'automatic' ILL requests is redundant. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180464 [details] [review] Bug 35604: (QA follow-up): Remove remaining 'automatic' occurrences From Hackfest QA session Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180465 [details] [review] Bug 35604: Move available_backends() to Koha::ILL::Request::Config This patch removes the the Koha::ILL::Backends class as it is not actually a Koha::Objects-based class as it claims, and also internally instantiates the config class, which could just hold the only defined method. This method is only used in the preferences.pl page, and its occurence is fixed by this patch. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Great job, team. Thanks for promptly following up on things we talked about! Hope you agree with the follow-up. Happy to discuss if required.
(In reply to Tomás Cohen Arazi (tcohen) from comment #138) > Hope you agree with the follow-up. Happy to discuss if required. Fully on board. Thank you very much!
1) I double checked the file, but this seems to be a false positive? Can you please check as well just in case? FAIL installer/data/mysql/mandatory/sysprefs.sql FAIL semicolon simicolon found instead of comma at line 881 (also spelling error: simicolon) 2) The new system preference now shows: "No available backends". I have been wondering if it would be clearer to say: Only one backend available. (when only standard is there) (not blocker)
Pushed for 25.05! Well done everyone, thank you!
(In reply to Katrin Fischer from comment #140) > 2) The new system preference now shows: "No available backends". I have been > wondering if it would be clearer to say: Only one backend available. (when > only standard is there) (not blocker) Only the Standard backend is available. Additional backends are required to use this feature. Would you agree to the above?
(In reply to Pedro Amorim from comment #142) > (In reply to Katrin Fischer from comment #140) > > 2) The new system preference now shows: "No available backends". I have been > > wondering if it would be clearer to say: Only one backend available. (when > > only standard is there) (not blocker) > > > Only the Standard backend is available. Additional backends are required to > use this feature. > > > Would you agree to the above? I think that would be a very helpful message :)
Created attachment 180956 [details] [review] Bug 35604: (QA follow-up): Update UI message if feature cannot be utilized
Another missed kitten, picked up for main now.