Bug 23482

Summary: BakerTaylor images broken on OPAC lists
Product: Koha Reporter: Nick Clemens <nick>
Component: OPACAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: normal    
Priority: P5 - low CC: 1joynelson, jonathan.druart, katrin.fischer, kelly
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00
Bug Depends on: 17896    
Bug Blocks:    
Attachments: Bug 23482: Fix BakerTaylor cover images on lists
Bug 23482: Fix BakerTaylor cover images on lists
Bug 23482: Fix BakerTaylor cover images on lists
Bug 23482: Remove conditional from BEGIN block
Bug 23482: Fix BakerTaylor cover images on lists
Bug 23482: Fix BakerTaylor cover images on lists
Bug 23482: Fix BakerTaylor cover images on lists
Bug 23482: [19.11.x] Fix BakerTaylor cover images on lists

Description Nick Clemens 2019-08-21 02:24:00 UTC
The variables for BakerTaylor covers are not passed to the template and the implementation differs from that of results page
Comment 1 Nick Clemens 2019-08-21 02:27:19 UTC
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
Comment 2 ByWater Sandboxes 2019-11-21 14:53:22 UTC
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>
Comment 3 Kelly McElligott 2019-11-21 14:55:04 UTC
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 4 Katrin Fischer 2020-02-17 21:52:47 UTC
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?
Comment 5 Nick Clemens 2020-02-18 13:20:28 UTC
(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')
Comment 6 Katrin Fischer 2020-02-18 13:30:10 UTC
Argh, thx!
Comment 7 Jonathan Druart 2020-02-18 16:04:39 UTC
The BEGIN block is wrong, see bug 17896.
Comment 8 Nick Clemens 2020-02-19 12:58:19 UTC
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>
Comment 9 Nick Clemens 2020-02-19 12:58:23 UTC
Created attachment 99248 [details] [review]
Bug 23482: Remove conditional from BEGIN block
Comment 10 Jonathan Druart 2020-02-21 09:54:05 UTC
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.
Comment 11 Nick Clemens 2020-02-21 13:04:34 UTC
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>
Comment 12 Nick Clemens 2020-02-21 13:17:04 UTC
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>
Comment 13 Jonathan Druart 2020-02-21 14:17:00 UTC
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>
Comment 14 Jonathan Druart 2020-02-21 14:17:34 UTC
I have tried to put this code in an include file but failed. They are all different!
Comment 15 Martin Renvoize 2020-02-21 15:45:09 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 16 Joy Nelson 2020-03-13 01:23:43 UTC
does not apply to 19.11.x branch
please rebase if needed
Comment 17 Nick Clemens 2020-03-13 15:50:30 UTC
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>