Bug 11982 - Show local cover images in the biblio details page in the intranet.
Summary: Show local cover images in the biblio details page in the intranet.
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Maxime Beaulieu
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-21 15:56 UTC by Frédérick Capovilla
Modified: 2016-12-05 21:23 UTC (History)
14 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Show local cover image in the intranet biblio details page. (5.64 KB, patch)
2014-03-21 15:56 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
2 images (60.42 KB, image/png)
2014-03-22 02:49 UTC, Nicole C. Engard
Details
Show local cover image in the intranet biblio details page. Also, fix useless "No cover image" block when using Amazon and local cover images at the same time. (5.77 KB, patch)
2014-08-08 13:55 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11982 - Fixed the amazon no image icon. (6.53 KB, patch)
2014-08-08 13:55 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11982 - Images now display properly in the details section. (7.46 KB, patch)
2014-08-08 13:55 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Details-Both_Images (181.61 KB, image/png)
2014-08-11 13:16 UTC, Maxime Beaulieu
Details
Details-One_Image (180.21 KB, image/png)
2014-08-11 13:17 UTC, Maxime Beaulieu
Details
[SIGNED-OFF] Bug 11982 - Show local cover image in the intranet biblio details page (5.89 KB, patch)
2014-10-23 17:52 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11982 - Fixed the amazon no image icon. (6.64 KB, patch)
2014-10-23 17:53 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11982 - Images now display properly in the details section. (7.56 KB, patch)
2014-10-23 17:53 UTC, Owen Leonard
Details | Diff | Splinter Review
[PASSED QA] Bug 11982 - Show local cover image in the intranet biblio details page (5.89 KB, patch)
2014-10-25 20:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11982 - Fixed the amazon no image icon. (6.65 KB, patch)
2014-10-25 20:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11982 - Images now display properly in the details section. (7.71 KB, patch)
2014-10-25 20:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11982 - Images now display properly in the details section. (7.71 KB, patch)
2014-12-29 13:33 UTC, simith.doliveira
Details | Diff | Splinter Review
Bug 11982 - Show local cover image in the intranet biblio details page (6.05 KB, patch)
2014-12-29 13:55 UTC, simith.doliveira
Details | Diff | Splinter Review
Bug 11982 - Show local cover image in the intranet biblio details page - conflit fixed (6.05 KB, patch)
2014-12-29 13:56 UTC, simith.doliveira
Details | Diff | Splinter Review
Bug 11982 - Fixed the amazon no image icon. (7.11 KB, patch)
2015-03-05 16:36 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11982 - Images now display properly in the details (8.32 KB, patch)
2015-03-05 16:39 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
[PASSED QA] Bug 11982 - Show local cover image in the intranet biblio details page (6.12 KB, patch)
2015-03-13 12:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11982 - Fixed the amazon no image icon. (7.18 KB, patch)
2015-03-13 12:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11982 - Images now display properly in the details section. (8.38 KB, patch)
2015-03-13 12:52 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frédérick Capovilla 2014-03-21 15:56:20 UTC Comment hidden (obsolete)
Comment 1 Nicole C. Engard 2014-03-22 02:49:45 UTC
Created attachment 26515 [details]
2 images

After applying the patch I uploaded a local cover image.  It does show on the detail page, but it still shows the 'no image' icon from amazon as well.

Normally, I'd be fine with that, but since you mentioned that this patch fixed that I thought I should point it out.
Comment 2 Maxime Beaulieu 2014-08-08 13:55:04 UTC Comment hidden (obsolete)
Comment 3 Maxime Beaulieu 2014-08-08 13:55:12 UTC Comment hidden (obsolete)
Comment 4 Maxime Beaulieu 2014-08-08 13:55:17 UTC Comment hidden (obsolete)
Comment 5 HB-NEKLS 2014-08-08 22:34:06 UTC
I tried testing this patch, and got the following message when applying the patch:

Applying: Show local cover image in the intranet biblio details page. Also, fix useless "No cover image" block when using Amazon and local cover images at the same time.
fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Show local cover image in the intranet biblio details page. Also, fix useless "No cover image" block when using Amazon and local cover images at the same time.

It's my first time using git to test patches, so it may be me.
Comment 6 Katrin Fischer 2014-08-09 08:57:29 UTC
Hi Heather, patches apply for me after doing a git pull on master this morning - could you maybe try again? I think maybe your master was a little bit behind and Maxime had just rebased the patches.
Comment 7 HB-NEKLS 2014-08-09 15:47:55 UTC
Katrin, 

I tried again this morning, and the patches applied successfully (yay!); thanks for the encouragement. However, bear with the testing newbie, I have two questions: 

1) Should I even sign off on this patch if it doesn't have a test plan? 
2) The uploaded image is displaying on my test system in an odd place. Is that a problem within the scope of this patch (see linked screenshot: http://goo.gl/ZveVYw), my setup, or a separate issue?
Comment 8 Katrin Fischer 2014-08-09 16:07:57 UTC
Hi Heather, thanks for trying again :) I think you are right - it doesn't look quite right at the moment. Maybe you could attach your screenshot here and ask Maxime to take a look? 
If you are unsure about how something is supposed to work, asking for a test plan is always ok.
Comment 9 Maxime Beaulieu 2014-08-11 13:16:40 UTC
Created attachment 30689 [details]
Details-Both_Images
Comment 10 Maxime Beaulieu 2014-08-11 13:17:07 UTC
Created attachment 30690 [details]
Details-One_Image
Comment 11 Maxime Beaulieu 2014-08-11 13:27:34 UTC
> 2) The uploaded image is displaying on my test system in an odd place. Is
> that a problem within the scope of this patch (see linked screenshot:
> http://goo.gl/ZveVYw), my setup, or a separate issue?

I would like to know more about your test browser.

Images are displayed correctly for me in Firefox 29. (See attachements)

Here is a test plan.
1) Enable both amazon and local cover images in sysprefs
2) Create 4 test records
    1) Local image only
    2) Amazon image only
    3) Both images
    4) No image
3) Display all records in a search result.
4) Check the tumbnails. The only record with "No cover image available" thumbnail should be item 4
5) Go to the details page for each item. The "No cover image available" thumbnail should not appear for any record, only the images that were set / found on amazon are displayed on this page.
Comment 12 Owen Leonard 2014-10-23 17:52:55 UTC Comment hidden (obsolete)
Comment 13 Owen Leonard 2014-10-23 17:53:46 UTC Comment hidden (obsolete)
Comment 14 Owen Leonard 2014-10-23 17:53:57 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2014-10-24 06:39:32 UTC
Working on this!

I noticed that the "IE Hack" was changed, could someone verify this displays ok in IE?
Comment 16 Owen Leonard 2014-10-24 12:43:33 UTC
(In reply to Katrin Fischer from comment #15)
> I noticed that the "IE Hack" was changed, could someone verify this displays
> ok in IE?

IE11 - Works as expected
IE9 - Works as expected
IE8 - JavaScript error on search results, but it's unrelated
      to this patch
    - Local covers don't display on search results, but it could
      be because script execution was interrupted by the above error
    - Local covers don't display on detail pages. "No cover
      available" box is seen alongside the Amazon cover image for the
      record which has both. The browser does not report a JS error.

In my opinion the IE8 issues could be addressed in a follow-up--or ignored, as other IE8 JavaScript errors have been.
Comment 17 Katrin Fischer 2014-10-25 20:35:00 UTC Comment hidden (obsolete)
Comment 18 Katrin Fischer 2014-10-25 20:35:05 UTC Comment hidden (obsolete)
Comment 19 Katrin Fischer 2014-10-25 20:35:08 UTC Comment hidden (obsolete)
Comment 20 Tomás Cohen Arazi 2014-12-28 23:00:26 UTC
Hi, it seems some rebasing is needed. Will be watghing this one to push it ASAP.
Comment 21 simith.doliveira 2014-12-29 13:33:22 UTC Comment hidden (obsolete)
Comment 22 simith.doliveira 2014-12-29 13:55:38 UTC Comment hidden (obsolete)
Comment 23 simith.doliveira 2014-12-29 13:56:51 UTC Comment hidden (obsolete)
Comment 24 Nick Clemens 2015-02-07 15:32:58 UTC
I tried looking at this, I think that two patches were obsoleted that shouldn't have been.  If I am right 32747 and 34813 should be un-obsoleted and the patch can be moved to passed-qa as I think a small rebase doesn't need a new signoff.
Comment 25 Katrin Fischer 2015-02-08 21:31:07 UTC
I tested with one patch visible on the bug report right now and found a small regression:

Problem:
Amazon cover exists, no local cover 
With patch: no cover is shown in detail page
Without patch: Amazon cover shows

Problem?:
Amazon cover and local cover exist
With patch: local cover is shown
Without patch: Amazon cover is shown

OK:
Only local cover exists
With patch: local cover is shown
Without patch: no cover is shown


Simith or Maxime, can you please take a look?
Comment 26 Maxime Beaulieu 2015-03-05 16:36:24 UTC Comment hidden (obsolete)
Comment 27 Maxime Beaulieu 2015-03-05 16:39:16 UTC Comment hidden (obsolete)
Comment 28 Katrin Fischer 2015-03-05 16:42:44 UTC
Plase take a look at comment 25 too! Thx!
Comment 29 Maxime Beaulieu 2015-03-05 16:44:13 UTC
(In reply to Katrin Fischer from comment #28)
> Plase take a look at comment 25 too! Thx!

The regression was caused by the missing patches.
If you apply all three you should not see any problems.
Comment 30 Kyle M Hall 2015-03-13 12:51:42 UTC
(In reply to Maxime Beaulieu from comment #29)
> (In reply to Katrin Fischer from comment #28)
> > Plase take a look at comment 25 too! Thx!
> 
> The regression was caused by the missing patches.
> If you apply all three you should not see any problems.

With all three patches, I'm not seeing those problems!
Comment 31 Kyle M Hall 2015-03-13 12:52:03 UTC
Created attachment 36852 [details] [review]
[PASSED QA] Bug 11982 - Show local cover image in the intranet biblio details page

Also, fix useless "No cover image" block when using Amazon and local
cover images at the same time.

http://bugs.koha-community.org/show_bug.cgi?id=11982
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 32 Kyle M Hall 2015-03-13 12:52:11 UTC
Created attachment 36853 [details] [review]
[PASSED QA] Bug 11982 - Fixed the amazon no image icon.

A no image icon will only be displayed if both the amazon and local images are missing.
Added hyperlink to details on the local image in the search result page.
    modified:   koha-tmpl/intranet-tmpl/prog/en/js/localcovers.js
    modified:   koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
    modified:   koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 33 Kyle M Hall 2015-03-13 12:52:14 UTC
Created attachment 36854 [details] [review]
[PASSED QA] Bug 11982 - Images now display properly in the details section.

If the a cover image exists, it will be displayed on the details page.
Nothing is displayed of there is no cover available.
    modified:   koha-tmpl/intranet-tmpl/prog/en/js/localcovers.js
    modified:   koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Works as described, passes tests and QA script.
Checked Amazon covers and local covers display correctly
in staff on results and detail pages.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Tomás Cohen Arazi 2015-03-30 16:51:13 UTC
Patches pushed to master.

Thanks Frederick!
Comment 35 Chris Cormack 2015-03-31 05:42:19 UTC
Pushed to 3.18.x will be in 3.18.6