Bug 11982

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.

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.

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.

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?

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.

Created attachment 30689 [details]
Details-Both_Images

Created attachment 30690 [details]
Details-One_Image

> 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.

Working on this!

I noticed that the "IE Hack" was changed, could someone verify this displays ok in IE?

(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.

Hi, it seems some rebasing is needed. Will be watghing this one to push it ASAP.

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.

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?

Plase take a look at comment 25 too! Thx!

(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.

(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!

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>

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>

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>

Patches pushed to master.

Thanks Frederick!

Pushed to 3.18.x will be in 3.18.6

Created attachment 114729 [details]
Bug 11983: Centralised Koha::Patrons method OPTION 2