Bug 7264 - Display information about library on OPAC detail
Summary: Display information about library on OPAC detail
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Owen Leonard
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-25 02:54 UTC by Srdjan Jankovic
Modified: 2013-12-05 20:04 UTC (History)
8 users (show)

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


Attachments
patch (13.43 KB, patch)
2012-01-06 05:16 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Screenshot of library details displayed in the OPAC (42.08 KB, image/png)
2012-01-06 09:24 UTC, Magnus Enger
Details
patch (16.49 KB, patch)
2012-02-02 03:17 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 7264 - Followup: Add a black border to the popup (965 bytes, patch)
2012-02-03 11:37 UTC, Magnus Enger
Details | Diff | Splinter Review
bug_7264: Added opac_info field to branches table. (17.17 KB, patch)
2012-02-03 11:42 UTC, Magnus Enger
Details | Diff | Splinter Review
bug_7264: [SIGNED-OFF] Added opac_info field to branches table. (17.18 KB, patch)
2012-02-03 11:43 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7264 - Followup: Add a black border to the popup (1.05 KB, patch)
2012-02-08 21:17 UTC, Owen Leonard
Details | Diff | Splinter Review
patch (13.43 KB, patch)
2012-02-28 00:33 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 7264 - Followup (1.05 KB, patch)
2012-03-08 08:22 UTC, delaye
Details | Diff | Splinter Review
patch (15.85 KB, patch)
2012-03-09 01:30 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug_7264: [SIGNED-OFF] Branch popup on OPAC detail.Added opac_info field to branches table. (15.97 KB, patch)
2012-03-09 09:24 UTC, Paul Poulain
Details | Diff | Splinter Review
bug_7264: [SIGNED-OFF] Branch popup on OPAC detail.Added opac_info field to branches table. (15.97 KB, patch)
2012-03-09 09:33 UTC, Paul Poulain
Details | Diff | Splinter Review
bug7264-SIGNED-OFF-Branch-popup-on-OPAC-detailAdde.patch (15.97 KB, patch)
2012-03-09 09:36 UTC, delaye
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Srdjan Jankovic 2011-11-25 02:54:33 UTC
On opac-detail, there should be a JS popup or something like that that could display the library information(s): address, phone, email, and notes field - that could be used to specify opening hours or any other useful information.
Comment 1 Srdjan Jankovic 2012-01-06 05:16:24 UTC Comment hidden (obsolete)
Comment 2 Magnus Enger 2012-01-06 09:23:28 UTC
This looks like a nice enhancement, but I fear we will never agree on what should and should not be displayed in the popup... Would it be possible to add a syspref that

- disables the feature if it's empty
- takes a template with placeholders for the different fields that describe the libraries, so each library could format the contents of the popup to their heart's content? 

I'm attaching a screenshot of what the current patch looks like, to make it easier for others to add their opinion.
Comment 3 Magnus Enger 2012-01-06 09:24:10 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2012-01-06 09:28:21 UTC
I think i agree with Magnus - not every library will want all the data from the branches table to be displayed in the OPAC. 

I would really like to see that as an option, a syspref with a list of fields to display perhaps and leaving it empty when you don't want it to display. With TT such things have become a lot easier to do!

Is it possible to click on the mail adress? I can't see from the screenshot.
Comment 5 Magnus Enger 2012-01-06 09:32:38 UTC
(In reply to comment #4)
> Is it possible to click on the mail adress? I can't see from the screenshot.

Good question! No, it's not. (At least not in FF8 on Ubuntu). The popup follows the mouse pointer, and you can't moce the mouse pointer over the popup, if you try it disappears. So turning the address into a clickable link might not make too much sense.
Comment 6 David Schuster 2012-01-06 14:30:49 UTC
Please make this optional as schools generally the address of the school isn't important to our users they are just trying to locate a book at their school or can talk with the librarian about getting it.

my .0001 cent opinion.
Comment 7 claire.hernandez@biblibre.com 2012-01-09 16:38:57 UTC
We would prefer a sytem like in news ("koha-news.pl") where you can define html code you want to display.
Comment 8 Katrin Fischer 2012-01-09 16:54:36 UTC
I think for this bug a system preference allowing to specify the fields for display would be a nice addition and could be enhanced later on. 

In general I like the idea to use something like the news editor/notices editor for some of the things we have in system preferences now, like most of the OPAC html preferences.

- different entries for different languages
- html editor to make it easier for users without html knowledge
- display of available placeholders
Comment 9 Srdjan Jankovic 2012-02-02 03:17:23 UTC Comment hidden (obsolete)
Comment 10 Magnus Enger 2012-02-03 11:37:06 UTC Comment hidden (obsolete)
Comment 11 Magnus Enger 2012-02-03 11:42:36 UTC Comment hidden (obsolete)
Comment 12 Magnus Enger 2012-02-03 11:43:46 UTC Comment hidden (obsolete)
Comment 13 Magnus Enger 2012-02-03 11:46:31 UTC
I have signed off on attachment 7441 [details] [review], but my followup attachment 7439 [details] [review] needs signoff (or another patch, if someone comes up with other improvements), so I leave the status at "Needs Signoff".
Comment 14 Owen Leonard 2012-02-08 21:17:37 UTC Comment hidden (obsolete)
Comment 15 Marcel de Rooy 2012-02-27 08:06:34 UTC
Applying: bug_7264: [SIGNED-OFF] Added opac_info field to branches table.
/usr/share/koha/testclone/.git/rebase-apply/patch:201: trailing whitespace.
 *
/usr/share/koha/testclone/.git/rebase-apply/patch:203: trailing whitespace.
 *
/usr/share/koha/testclone/.git/rebase-apply/patch:205: trailing whitespace.
 *
/usr/share/koha/testclone/.git/rebase-apply/patch:207: trailing whitespace.
 *
error: patch failed: C4/Items.pm:1235
error: C4/Items.pm: patch does not apply
error: patch failed: installer/data/mysql/updatedatabase.pl:4663
error: installer/data/mysql/updatedatabase.pl: patch does not apply
warning: koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt has type 100644, expected 100755
Using index info to reconstruct a base tree...
<stdin>:201: trailing whitespace.
 *
<stdin>:203: trailing whitespace.
 *
<stdin>:205: trailing whitespace.
 *
<stdin>:207: trailing whitespace.
 *
warning: 4 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merged C4/Items.pm
CONFLICT (content): Merge conflict in C4/Items.pm
Auto-merged installer/data/mysql/kohastructure.sql
Auto-merged installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merged koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
Failed to merge in the changes.

Please rebase, fix whitespace warnings too and note file attribs. Thanks!
Comment 16 Srdjan Jankovic 2012-02-28 00:33:11 UTC Comment hidden (obsolete)
Comment 17 delaye 2012-03-07 10:50:59 UTC
i tried to test this patch with sandbox system, it does not work, here is the result:

Applying: Bug 7264 - Followup: Add a black border to the popup
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 7264 - Followup: Add a black border to the popup
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Bug 7264 - Display information about library on OPAC detail

[SIGNED-OFF] Bug 7264 - Followup: Add a black border to the popup
Apply? [yn]
Patch left in /tmp/SIGNED-OFF-Bug-7264---Followup-Add-a-black-border--6LJb4S.patch
Comment 18 Katrin Fischer 2012-03-07 10:53:58 UTC
I think the problem might be that the patches are attached in the wrong order - the second attachement should be the first patch to be applied.
Comment 19 Srdjan Jankovic 2012-03-07 23:14:47 UTC
Yes, that is an interesting situation. I should have noted that. So the latest patch is the original patch replacement and should be applied first. The signed off patch is a follow up that needs to be applied afterwards.
Now I can forsee some problems with that, so please give it a go and if it does not wotr I'll combine them into one patch.
Comment 20 delaye 2012-03-08 08:22:15 UTC Comment hidden (obsolete)
Comment 21 delaye 2012-03-08 08:34:15 UTC
I use a sandbox to test patches.
In 7264 i changed the order of the patch then I started applying the patches but I have this new message:

Applying: bug_7264: Branch popup
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging C4/Items.pm
CONFLICT (content): Merge conflict in C4/Items.pm
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
Auto-merging opac/opac-detail.pl
Failed to merge in the changes.
Patch failed at 0001 bug_7264: Branch popup
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Bug 7264 - Display information about library on OPAC detail

patch
Apply? [yn]
Patch left in /tmp/patch-IjIQ5k.patch
Comment 22 Srdjan Jankovic 2012-03-09 01:30:20 UTC Comment hidden (obsolete)
Comment 23 Paul Poulain 2012-03-09 09:24:19 UTC Comment hidden (obsolete)
Comment 24 delaye 2012-03-09 09:31:46 UTC
I sign this patch because it corresponds to customer requests. Since Koha Administration / branches I associated HTML (table) to a branch. Since the OPAC and data copies if I place the pointer over the title of the branch a pop up back in plain HTML (table)
Comment 25 Paul Poulain 2012-03-09 09:33:38 UTC Comment hidden (obsolete)
Comment 26 delaye 2012-03-09 09:36:34 UTC
Created attachment 8113 [details] [review]
bug7264-SIGNED-OFF-Branch-popup-on-OPAC-detailAdde.patch
Comment 27 Marcel de Rooy 2012-03-19 09:51:35 UTC
Question: Which patches should be looked at? There is only one left now (att 8113) and rest has been obsoleted (12 atts).
Please describe clearly what should be done to test this. Which field should be edited, where, etc. It is not clear to me now.
Changing status to reflect the need for clarification. Thanks!
Comment 28 delaye 2012-03-19 10:34:33 UTC
see the note 2012-03-09 09:31:46 UTC 

You go in Koha  Administration and  Libraries, branches and groups
you edit the branch
and in opac info you enter your html page.

you use bug7264-SIGNED-OFF-Branch-popup-on-OPAC-detailAdde.patch
Comment 29 Marcel de Rooy 2012-03-19 11:40:56 UTC
Thanks for responding.
But where do you add this field to the database? Back to Failed QA.

[Mon Mar 19 12:31:34 2012] [error] [client 192.87.126.61] [Mon Mar 19 12:31:34 2012] branches.pl: DBD::mysql::st execute failed: Unknown column 'opac_info' in 'field list' at /usr/share/koha/testclone/C4/Branch.pm line 234., referer: http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/admin/branches.pl?op=edit&branchcode=FVM
Comment 30 Chris Cormack 2012-03-19 16:43:07 UTC
Marcel its in the updatedatabse.pl in the patch. Did you not run that?
Comment 31 Marcel de Rooy 2012-03-21 12:22:12 UTC
(In reply to comment #30)
> Marcel its in the updatedatabse.pl in the patch. Did you not run that?

Good point. I apparently overlooked the dbrev. Back to Signed off for QA.
Comment 32 Paul Poulain 2012-03-22 11:00:55 UTC
QA comment:
* this code is clean, no specific problem with it (perlcritic branches.pl & branches.pm [1] OK, valid&translatable templates)
* it adds a jquery plugin (opac-tmpl/prog/en/lib/jquery/plugins/jquery.tools.min.js), I had a discussion about it with owen, he think this plugin will be obsoleted by jqueryUI, but until jqueryUI is not here, this plugin can be included
*

[1] well, Branches.pm is not OK for perlcritic after the patch, but it wasn't before too, so, this patch doesn't break of fix here.
Comment 33 Owen Leonard 2012-11-26 20:35:49 UTC
This feature was broken by the upgrade of to jQueryUI tabs (Bug 8143).
Comment 34 Owen Leonard 2013-03-19 15:46:09 UTC
I'm marking this fixed because the original feature was pushed and was working. A new bug was created which addresses the subsequent breakage (Bug 9572).