Bug 7674 - Separate tabs for my holdings and others holdings
Summary: Separate tabs for my holdings and others holdings
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Julian Maurice
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 10257 9875 10283 11255
  Show dependency treegraph
 
Reported: 2012-03-08 13:37 UTC by delaye
Modified: 2014-05-26 21:04 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7674: Separate items in two tabs (opac/opac-detail.pl) (6.61 KB, patch)
2012-09-21 12:47 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Separate items in two tabs (catalogue/detail.pl) (20.80 KB, patch)
2012-09-21 12:47 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Add menu entry to delete items in batch (4.11 KB, patch)
2012-09-21 12:47 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Separate items in two tabs on biblio detail page (staff+opac) (30.60 KB, patch)
2012-09-21 14:03 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Separate items in two tabs on biblio detail page (staff+opac) (30.62 KB, patch)
2012-10-09 15:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Open shelf browser in appropriate tab (27.33 KB, patch)
2012-10-09 15:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Separate items in two tabs on biblio detail page (staff+opac) (30.58 KB, patch)
2012-10-23 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7674: Open shelf browser in appropriate tab (27.33 KB, patch)
2012-10-23 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7674: Separate items in two tabs on biblio detail page (staff+opac) (30.50 KB, patch)
2012-11-02 10:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Open shelf browser in appropriate tab (27.33 KB, patch)
2012-11-02 10:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Add 2 sysprefs to indicate the branch we want to separate on (9.35 KB, patch)
2012-11-02 10:10 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674: Separate items in two tabs on biblio detail page (staff+opac) (30.59 KB, patch)
2012-11-02 12:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674: Open shelf browser in appropriate tab (27.43 KB, patch)
2012-11-02 12:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674: Add 2 sysprefs to indicate the branch we want to separate on (9.44 KB, patch)
2012-11-02 12:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7674 - Followup - Add missing right-hand curly bracket (1.30 KB, patch)
2012-11-02 12:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-0FF] Bug 7674 - Followup - Add missing right-hand curly bracket (1.32 KB, patch)
2012-11-02 12:40 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674: Separate items in two tabs on biblio detail page (staff+opac) (30.58 KB, patch)
2012-12-03 08:14 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674: Open shelf browser in appropriate tab (27.31 KB, patch)
2012-12-03 08:14 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674: Add 2 sysprefs to indicate the branch we want to separate on (9.45 KB, patch)
2012-12-03 08:15 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674 - Followup - Add missing right-hand curly bracket (1.32 KB, patch)
2012-12-03 08:15 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674: Separate items in two tabs on biblio detail page (staff+opac) (30.65 KB, patch)
2012-12-11 07:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674: Open shelf browser in appropriate tab (27.38 KB, patch)
2012-12-11 07:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674: Add 2 sysprefs to indicate the branch we want to separate on (10.08 KB, patch)
2012-12-11 07:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7674 - Followup - Add missing right-hand curly bracket (3.34 KB, patch)
2012-12-11 07:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Picture of the problem (144.00 KB, image/png)
2012-12-13 15:50 UTC, Jared Camins-Esakov
Details
Bug 7674: Remove some debug code (1005 bytes, patch)
2012-12-14 08:58 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Hide empty items tab (1.44 KB, patch)
2013-01-10 16:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7674: Hide empty items tab (1.49 KB, patch)
2013-03-20 11:13 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 7674: Hide empty items tab (1.49 KB, patch)
2013-03-20 11:25 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description delaye 2012-03-08 13:37:12 UTC
From screen ../cgi-bin/koha/catalogue/detail.pl?biblionumber=
Different lists(tabs) for my holdings(items with branch connexion) and other items(item with branch different of branch connexion).

Add filter at the top right of the item list. Filter that points to the enumchron
4 / Add a column "Selection" check at the table of results
5/At the bottom add two buttons: "Remove items Selected" and "Modify items Selected" 
Buttons that copy / paste barcodes on the screens: batchMod.pl or batchMod.pl? Del = 1
6/Buttons activating a pop up screen avoiding leaving detail.pl

From items page, reach in 1 click the "batch del item" (usefull when you trash a year of a daily serial)

Notes : new permissions  associated with these functions see ticket 7673
Comment 1 delaye 2012-03-08 13:41:28 UTC
Notes : the two tabs My Holdings et others Holdings must also be activated in the OPAC : ..//cgi-bin/koha/opac-detail.pl
Comment 2 Matthias Meusburger 2012-05-31 13:36:14 UTC
For reference, this bug is linked to biblibre's tickets 4656 and 4655
Comment 3 Julian Maurice 2012-09-21 12:47:18 UTC Comment hidden (obsolete)
Comment 4 Julian Maurice 2012-09-21 12:47:21 UTC Comment hidden (obsolete)
Comment 5 Julian Maurice 2012-09-21 12:47:24 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2012-09-21 12:56:40 UTC
This sounds like it should be two bug reports: One for the separate holdings tabs, one for the batch operations.

The one for the separate holdings tabs needs to be controlled by a system preference, one for the OPAC and one for the staff client.

I would suggest that instead of "My holdings" the tab use the library name: "Athens holdings."
Comment 7 Julian Maurice 2012-09-21 13:25:11 UTC
New bug for the batch operation: Bug 8801
Comment 8 Julian Maurice 2012-09-21 14:03:10 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2012-09-21 18:30:47 UTC
Thanks for the improvements. I see another issue: Clicking a "browse shelf" link in the "other holdings" tab opens the shelf browser in the "<branch> holdings" tab. It would be easy enough to do something like this:

[% PROCESS items_table items=otheritemloop tab="otherholdings" %]

...and modify the link to the shelf browser:

<a href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% ITEM_RESULT.biblionumber %]&amp;shelfbrowse_itemnumber=[% ITEM_RESULT.itemnumber %]#[% tab %]">Browse shelf</a>

...but I'm not sure how you get the shelf browser to appear under the correct tab.
Comment 10 delaye 2012-09-24 15:11:55 UTC
i have this message when i want to test in a sandbox

omething went wrong !
Applying: Bug 7674: Separate items in two tabs on biblio detail page (staff+opac)
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Auto-merging installer/data/mysql/updatedatabase.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 7674: Separate items in two tabs on biblio detail page (staff+opac)
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 7674 - Separate tabs for my holdings and others holdings

Bug 7674: Separate items in two tabs on biblio detail page (staff+opac)
Apply? [yn]
Patch left in /tmp/Bug-7674-Separate-items-in-two-tabs-on-biblio-deta-iPrfAr.patch
Comment 11 Julian Maurice 2012-09-25 06:41:54 UTC
Marking as Failed QA for comment 9.

Owen, how can I get this "browse shelf" link appearing ? When I made this patch I didn't notice it.
Comment 12 Katrin Fischer 2012-09-25 07:37:37 UTC
Hi Julian,
there are system preferences to control the shelf browser and your items need to have callnumbers. I hope there is no UNIMARC/MARC21 problem involved here. 
Hope that helps!
Comment 13 Julian Maurice 2012-10-09 15:05:19 UTC Comment hidden (obsolete)
Comment 14 Julian Maurice 2012-10-09 15:05:27 UTC Comment hidden (obsolete)
Comment 15 Julian Maurice 2012-10-09 15:07:27 UTC
Thanks Katrin, in fact the link didn't show up because my items have not a callnumber.
Patch should fix the problem reported by Owen.
Comment 16 delaye 2012-10-15 09:52:24 UTC
i have this message when i want to test in a sandbox(biblibre)

CONFLICT (content): Merge conflict in catalogue/detail.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
Comment 17 Jonathan Druart 2012-10-23 09:05:51 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2012-10-23 09:06:43 UTC Comment hidden (obsolete)
Comment 19 Kyle M Hall 2012-10-30 13:43:18 UTC
Question, it looks like this splits the items based on the homebranch. Shouldn't it be split by the holdingbranch? This seems like the sensible way to do it, if this is a 'show items in my library' feature. It especially makes more sense considering the tabs are labeled 'holdings' and not 'owned by'.
Comment 20 Julian Maurice 2012-10-31 16:07:03 UTC
Yes you are probably right.
I reset status to Assigned and will send a follow up soon.
Comment 21 Katrin Fischer 2012-11-01 10:32:55 UTC
Hi Kyle and Jonathan,

I think it's debatable. I have been thinking about is since I saw this bug a whlie ago and I am not sure which is the right field to display and if every library wants it to be the same.

When I am working in a library where the books are always sent back after being sent to another library for being checked out there and I want to see how many items my library owns - home branch is right.

If I want to see what is currently on shelf for a borrower standing at the circulation desk and asking about the book, I would want the holdingbranch.

These are 2 examples I came up with, but I think we had this discussion before about the branch to show with the item in OPAC and staff. I am not sure which bug that was, but before changing the patch it's maybe worth trying to figure out what exactly libraries want and if we need to make this an option maybe to be flexible enough.
Comment 22 Kyle M Hall 2012-11-01 13:15:41 UTC
Making this choice a system pref is the easy solution. There is a lot of conflict over where to show holding vs home branch. For example, in search results I believe we see an items home branch, but in the record details, we see the holding branch.

Kyle

(In reply to comment #21)
> Hi Kyle and Jonathan,
> 
> I think it's debatable. I have been thinking about is since I saw this bug a
> whlie ago and I am not sure which is the right field to display and if every
> library wants it to be the same.
> 
> When I am working in a library where the books are always sent back after
> being sent to another library for being checked out there and I want to see
> how many items my library owns - home branch is right.
> 
> If I want to see what is currently on shelf for a borrower standing at the
> circulation desk and asking about the book, I would want the holdingbranch.
> 
> These are 2 examples I came up with, but I think we had this discussion
> before about the branch to show with the item in OPAC and staff. I am not
> sure which bug that was, but before changing the patch it's maybe worth
> trying to figure out what exactly libraries want and if we need to make this
> an option maybe to be flexible enough.
Comment 23 Julian Maurice 2012-11-02 10:10:08 UTC Comment hidden (obsolete)
Comment 24 Julian Maurice 2012-11-02 10:10:19 UTC Comment hidden (obsolete)
Comment 25 Julian Maurice 2012-11-02 10:10:28 UTC Comment hidden (obsolete)
Comment 26 Julian Maurice 2012-11-02 10:12:15 UTC
2 previous patches rebased on master + additional patch to choose whether we want to split items by home branch or by holding branch
Comment 27 Kyle M Hall 2012-11-02 12:13:50 UTC Comment hidden (obsolete)
Comment 28 Kyle M Hall 2012-11-02 12:14:07 UTC Comment hidden (obsolete)
Comment 29 Kyle M Hall 2012-11-02 12:14:22 UTC Comment hidden (obsolete)
Comment 30 Kyle M Hall 2012-11-02 12:14:32 UTC Comment hidden (obsolete)
Comment 31 Kyle M Hall 2012-11-02 12:17:25 UTC
Tested in both OPAC and staff with both settings. Everything appears to be working great! I did get a syntax error testing the staff side, but it was only a missing curly bracket. I've attached a followup. It is so small that I don't think it justifies holding back a signed-off status.

Great work Paul & Julian!
Comment 32 Julian Maurice 2012-11-02 12:40:25 UTC Comment hidden (obsolete)
Comment 33 Julian Maurice 2012-11-02 12:42:03 UTC
Thanks Kyle for the follow-up (next time I will test *after* doing the rebase...)
Comment 34 Katrin Fischer 2012-12-02 11:00:23 UTC
Hi Julian and others, 

can someone please rebase those patches? I fixed a conflict in sysprefs.sql, but was not sure how to resolve the next in opac-detail. Please set back to 'signed off' if the conflicts are not severe.
Comment 35 Julian Maurice 2012-12-03 08:14:38 UTC Comment hidden (obsolete)
Comment 36 Julian Maurice 2012-12-03 08:14:53 UTC Comment hidden (obsolete)
Comment 37 Julian Maurice 2012-12-03 08:15:07 UTC Comment hidden (obsolete)
Comment 38 Julian Maurice 2012-12-03 08:15:20 UTC Comment hidden (obsolete)
Comment 39 Julian Maurice 2012-12-03 08:15:51 UTC
All patches rebased on master.
Comment 40 Katrin Fischer 2012-12-11 06:24:58 UTC
Working on this now.
Comment 41 Katrin Fischer 2012-12-11 07:36:01 UTC Comment hidden (obsolete)
Comment 42 Katrin Fischer 2012-12-11 07:36:14 UTC Comment hidden (obsolete)
Comment 43 Katrin Fischer 2012-12-11 07:36:22 UTC Comment hidden (obsolete)
Comment 44 Katrin Fischer 2012-12-11 07:36:33 UTC Comment hidden (obsolete)
Comment 45 Jared Camins-Esakov 2012-12-13 15:50:40 UTC
Created attachment 14082 [details]
Picture of the problem

An extra (and ugly) "%]" shows up on the OPAC in the holdings tab if I do not change any sysprefs at all after applying this patch.
Comment 46 Julian Maurice 2012-12-14 08:57:22 UTC
I'm unable to reproduce this problem.
I think I've tried any combination of the 2 sysprefs involved here (OpacSeparateHoldings, OpacSeparateHoldingsBranch), logged in status, shelf browser displayed or not, and branch of items.
I recreated the git branch like this
  git fetch origin
  git checkout -b bug/7674 origin/master
  git bz apply 7674 # applied all patches

I found some debug text still present in the browser shelf so I will submit a patch for that.
Jared if you are able to reproduce, could you investigate to find the line of code that produce this extra '%]' ?
Comment 47 Julian Maurice 2012-12-14 08:58:58 UTC Comment hidden (obsolete)
Comment 48 Jared Camins-Esakov 2012-12-17 14:58:08 UTC
It turns out the merge problem was caused trailing whitespace. I am not sure why. However, disabling the whitespace error squelching fixed it.

I am going to push this patchset, with the following complaints:
1) If you choose to separate based on homebranch, the display makes no sense at all... the tab says "Centerville holdings ( 1 )" and the items table says "Fairfield" in the location column. However, the display makes no sense anyway since apparently at some point we removed the possibility of displaying the home branch at all.
2) The two holdings tabs display whether or not there is anything to go in them.
Comment 49 Julian Maurice 2013-01-10 16:05:27 UTC Comment hidden (obsolete)
Comment 50 Julian Maurice 2013-01-10 16:08:53 UTC
I obsoleted all previous patches that are in master and reset the status to Needs signoff.
This patch is for the 2nd Jared's complaint.
I don't think there is something to do for the first (in this bug).
Comment 51 Brendan Gallagher 2013-03-20 11:13:09 UTC Comment hidden (obsolete)
Comment 52 Brendan Gallagher 2013-03-20 11:14:22 UTC
Worked as expected and you have meet all of the other thoughts and options presented.  Great.  I like this feature.
Comment 53 Paul Poulain 2013-03-20 11:25:00 UTC
Patch tested with a sandbox, by Corinne Bulac <corinne.hayet@bulac.fr>
Comment 54 Paul Poulain 2013-03-20 11:25:29 UTC Comment hidden (obsolete)
Comment 55 Jared Camins-Esakov 2013-03-20 11:39:51 UTC
Perhaps this latest patch could be moved to a separate bug?
Comment 56 Katrin Fischer 2013-03-20 16:38:56 UTC
Moved to 9875