Bug 23482 - BakerTaylor images broken on OPAC lists
Summary: BakerTaylor images broken on OPAC lists
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 17896
Blocks:
  Show dependency treegraph
 
Reported: 2019-08-21 02:24 UTC by Nick Clemens
Modified: 2020-03-13 15:50 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00


Attachments
Bug 23482: Fix BakerTaylor cover images on lists (4.43 KB, patch)
2019-08-21 02:27 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23482: Fix BakerTaylor cover images on lists (4.50 KB, patch)
2019-11-21 14:53 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23482: Fix BakerTaylor cover images on lists (4.39 KB, patch)
2020-02-19 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23482: Remove conditional from BEGIN block (857 bytes, patch)
2020-02-19 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23482: Fix BakerTaylor cover images on lists (1.27 KB, patch)
2020-02-21 13:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23482: Fix BakerTaylor cover images on lists (4.01 KB, patch)
2020-02-21 13:17 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23482: Fix BakerTaylor cover images on lists (4.09 KB, patch)
2020-02-21 14:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23482: [19.11.x] Fix BakerTaylor cover images on lists (4.17 KB, patch)
2020-03-13 15:50 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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>