The variables for BakerTaylor covers are not passed to the template and the implementation differs from that of results page
Created attachment 92390 [details] [review] Bug 23482: Fix BakerTaylor cover images on lists While this needs a username/pass to fully test, it should be possible to verify the code changes by comparing to opac-results code To test: 1 - Enable BakerTaylor images 2 - Enter your usernme and password 3 - Do not fill the BookStore URL 4 - Verify OPAC covers are working 5 - Save some times with covers to a public list 6 - Verify they do not display in list 7 - Apply patch 8 - Verify images now work
Created attachment 95661 [details] [review] Bug 23482: Fix BakerTaylor cover images on lists While this needs a username/pass to fully test, it should be possible to verify the code changes by comparing to opac-results code To test: 1 - Enable BakerTaylor images 2 - Enter your usernme and password 3 - Do not fill the BookStore URL 4 - Verify OPAC covers are working 5 - Save some times with covers to a public list 6 - Verify they do not display in list 7 - Apply patch 8 - Verify images now work Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
I contacted the library that submitted this bug to us and they provided me with their B&T user name and password, this patch works as expected.
Comment on attachment 95661 [details] [review] Bug 23482: Fix BakerTaylor cover images on lists Review of attachment 95661 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt @@ +360,5 @@ > + <span class="no-image">No cover image available</span> > + [% END %] > + [% END %] > + > + [% IF ( Koha.Preference('BakerTaylorEnabled') && Koha.Preference('BakerTaylorBookstoreURL') ) %] Not sure why you start a new IF block here - conditions and variables appear to be the same as on the check right before it?
(In reply to Katrin Fischer from comment #4) > Comment on attachment 95661 [details] [review] [review] > Bug 23482: Fix BakerTaylor cover images on lists > > Review of attachment 95661 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt > @@ +360,5 @@ > > + <span class="no-image">No cover image available</span> > > + [% END %] > > + [% END %] > > + > > + [% IF ( Koha.Preference('BakerTaylorEnabled') && Koha.Preference('BakerTaylorBookstoreURL') ) %] > > Not sure why you start a new IF block here - conditions and variables appear > to be the same as on the check right before it? One is: !Koha.Preference('BakerTaylorBookstoreURL') The other is: Koha.Preference('BakerTaylorBookstoreURL')
Argh, thx!
The BEGIN block is wrong, see bug 17896.
Created attachment 99247 [details] [review] Bug 23482: Fix BakerTaylor cover images on lists While this needs a username/pass to fully test, it should be possible to verify the code changes by comparing to opac-results code To test: 1 - Enable BakerTaylor images 2 - Enter your usernme and password 3 - Do not fill the BookStore URL 4 - Verify OPAC covers are working 5 - Save some times with covers to a public list 6 - Verify they do not display in list 7 - Apply patch 8 - Verify images now work Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Created attachment 99248 [details] [review] Bug 23482: Remove conditional from BEGIN block
This is not enough for me to PQA. The original code is wrong (see bug 17896), you copy it, then modify it. I would prefer to fix the original issue, then reuse it. Here we are going to end up with 2 different versions.
Created attachment 99372 [details] [review] Bug 23482: Fix BakerTaylor cover images on lists While this needs a username/pass to fully test, it should be possible to verify the code changes by comparing to opac-results code To test: 1 - Enable BakerTaylor images 2 - Enter your usernme and password 3 - Do not fill the BookStore URL 4 - Verify OPAC covers are working 5 - Save some times with covers to a public list 6 - Verify they do not display in list 7 - Apply patch 8 - Verify images now work Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Created attachment 99374 [details] [review] Bug 23482: Fix BakerTaylor cover images on lists While this needs a username/pass to fully test, it should be possible to verify the code changes by comparing to opac-results code To test: 1 - Enable BakerTaylor images 2 - Enter your usernme and password 3 - Do not fill the BookStore URL 4 - Verify OPAC covers are working 5 - Save some times with covers to a public list 6 - Verify they do not display in list 7 - Apply patch 8 - Verify images now work Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Created attachment 99383 [details] [review] Bug 23482: Fix BakerTaylor cover images on lists While this needs a username/pass to fully test, it should be possible to verify the code changes by comparing to opac-results code To test: 1 - Enable BakerTaylor images 2 - Enter your usernme and password 3 - Do not fill the BookStore URL 4 - Verify OPAC covers are working 5 - Save some times with covers to a public list 6 - Verify they do not display in list 7 - Apply patch 8 - Verify images now work Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I have tried to put this code in an include file but failed. They are all different!
Nice work everyone! Pushed to master for 20.05
does not apply to 19.11.x branch please rebase if needed
Created attachment 100686 [details] [review] Bug 23482: [19.11.x] Fix BakerTaylor cover images on lists While this needs a username/pass to fully test, it should be possible to verify the code changes by comparing to opac-results code To test: 1 - Enable BakerTaylor images 2 - Enter your usernme and password 3 - Do not fill the BookStore URL 4 - Verify OPAC covers are working 5 - Save some times with covers to a public list 6 - Verify they do not display in list 7 - Apply patch 8 - Verify images now work Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>