Created attachment 8032 [details] [review] Add location an callnumber at opac subscription view This adds the location and the callnumber after the branch/library name on each subscription header of a subscription tab, in a serial detailed record at the OPAC. Tied with bug 7653
l'application du patch dans une sandbox a echoué Applying: Add location and callnumber at OPAC subscription view Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Add location and callnumber at OPAC subscription view
Created attachment 8200 [details] [review] Add location and callnumber to each subscription in opac subscription tab sorry, the previous patch was uncluding a modification of another local patch
Created attachment 8306 [details] Commas showing when no data There are some stray commas showing up in this patch if you don't have data in all of the fields.
When it comes to Item type it's showing the item type Code and not the description. For this and the previous comment I'm marking failed QA. Nicole
No new patch attached for signoff, I assume the status change was a mistake, changing back.
Could you please attach your updated patch to this?
Created attachment 14681 [details] [review] Bug 7654: Add location and callnumber at opac subscription view I corrected the comma's problem. As for the itemtype one, I could see the description in the itemtype column, holding tab. Maybe this problem has been corrected by another patch. However, I could see a new part of code that is seamingly made to display the callnumber but I don't understand under which conditions : <span style="display:none;"> [% IF ( subscription.callnumber ) %]<p>Callnumber: [% subscription.callnumber %]</p>[% END %]</span> I did a new patch nevertheless, but I suppose it 's necessary to harmonize the two callnumber displaying devices.
Patch applies
QA comment: * small patch, passes koha-qa.pl * i've added a tiny follow-up to remove 2 trailing spaces Passed QA
Created attachment 16677 [details] [review] [PASSED QA] Bug 7654:Add location and callnumber at opac subscription view Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Created attachment 16678 [details] [review] [PASSED QA] Bug 7654 follow-up: reindent Before this patch, there was a trailing space between branch, location and callnumber After this patch it's written branch, location, callnumber as expected Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
(In reply to comment #8) > Patch applies Pascale, Did you test this patch and sign off on it?
Bonjour, Yes, I signed off this patch during the hackfest in Marseille, last week.
Hello, I don't understand why this bug is "in discussion" after being signed-off et passed QA. I don't see in the comments an explanation about this status. Thanks, Sonia
I think we can fix the sign-off line in QA - switching back to signed off.
QA comment: Why do you add a br tag in the h3 title? The callnumber is hidden 2 line bellow, I think you can remove it. Maybe it is better to keep the existing one (callnumber). If all values are there, the h3 title will be too long, don't you think? Marked as Failed QA.
I suppose this br was meant to separate more clearly the "This is a serial" block from the localisations (if I had put it at the right place, after the h2 element). Ok for removing the hidden callnumber (didn't really understood what it does here) As for the length of h3, for us it's ok, may be some will find it too long. I joined a capture of an example in Lyon3. May be the closed mention ('This subscription is closed.') ,if present, would have to be move beneath the h3. Olivier Crouzet
Still valid?
No,it's no more valid Olivier Crouzet
Callnumber is already displayed in master, but we can still add the location.
Created attachment 58869 [details] [review] Bug 7654: Add location at opac subscription view Also fix a bug that prevent branch name to be displayed
Comment on attachment 58869 [details] [review] Bug 7654: Add location at opac subscription view Review of attachment 58869 [details] [review]: ----------------------------------------------------------------- Here are some untested thoughts. :) ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt @@ +796,4 @@ > <p>There are [% subscriptionsnumber %] subscription(s) associated with this title.</p> > [% FOREACH subscription IN subscriptions %] > [% IF ( subscription.branchcode ) %] > + <h3>At library: [% Branches.GetName( subscription.branchcode ) %]</h3> What if it gets a branch, but someone failed to enter a description or it is blank? Displaying the branchcode in that case would make sense. I do not think this correction is correct. [% at_library = Branches.GetName( subscription.branchcode ) %] [% SET at_library = subscription.branchcode IF at_library.length=0 %] [% IF at_library %] <h3>At library: [% at_library ) %]</h3> [% END %] @@ -799,5 @@ > - <h3>At library: [% Branches.GetName( subscription.branchname ) %]</h3> > - [% ELSE %] > - [% IF ( subscription.branchcode ) %] > - <h3>At library: [% subscription.branchcode %]</h3> > - [% END %] I'm pretty sure this code was intending to display a branch name if possible, but if not just the branch code. And clearly, it is wrong about that. @@ +813,5 @@ > <p>[% subscription.opacnote FILTER html_line_break %]</p> > [% END %] > + [% IF subscription.location_description || subscription.location %] > + <p>Location: [% IF subscription.location_description %][% subscription.location_description %][% ELSE %][% subscription.location %][% END %]</p> > + [% END %] [% location_description = subscription.location_description %] [% SET location_description = subscription.location IF ! defined location_description || location_description.length=0 %] [% IF location_description %] <p>Location: [% location_description %]</p> [% END %]
(In reply to M. Tompsett from comment #22) > Comment on attachment 58869 [details] [review] [review] > Bug 7654: Add location at opac subscription view > > Review of attachment 58869 [details] [review] [review]: > ----------------------------------------------------------------- > > Here are some untested thoughts. :) > > ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt > @@ +796,4 @@ > > <p>There are [% subscriptionsnumber %] subscription(s) associated with this title.</p> > > [% FOREACH subscription IN subscriptions %] > > [% IF ( subscription.branchcode ) %] > > + <h3>At library: [% Branches.GetName( subscription.branchcode ) %]</h3> > > What if it gets a branch, but someone failed to enter a description or it is > blank? Displaying the branchcode in that case would make sense. I do not > think this correction is correct. That's actually not a problem. The branchcode and branchname are both mandatory. It's not possible to have a branch without a branchname in Koha. Using the TT plugin like this is common in Koha.
Merge conflict in opac-detail.tt BTW: Bug 18388 - Standardize serials volume information displaying will touch the same file
I believe this was fixed by bug 12318 now. *** This bug has been marked as a duplicate of bug 12318 ***