|
Description
Kyle M Hall (khall)
2017-11-30 11:28:29 UTC
Created attachment 72773 [details] [review] Bug 19719 - Add a ‘Collection’ column in the patron loans data table. TEST: 1. Check out a few items to patron. 2. View check outs on patron account in staff client. 3. Collection code is not present. 4. Apply patch 5. Notice that not Collection code is visible while viewing check outs. Created attachment 72829 [details] [review] Bug 19719 - Add a Collection column in the patron loans data table. Signed-off-by: Severine QUEUNE <severine.queune@bulac.fr> Column correctly appears on both "Checkout" and "Details" tabs on patron account. It is correctly hidden if "collection_code" box are checked on Column configuration > Circulation > id=issues-table and Column configuration > Patrons > id=issues-table. Thanks Kyle and Jesse ! The patch works correctly but when I try to sign off the patch I get this error message.
/usr/local/bin/git-bz:1834: UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal
obsoleted = (initial_description == patch.description)
Traceback (most recent call last):
File "/usr/local/bin/git-bz", line 2632, in <module>
do_attach(*args)
File "/usr/local/bin/git-bz", line 1998, in do_attach
attach_commits(bug, commits, edit_comments=global_options.edit)
File "/usr/local/bin/git-bz", line 1913, in attach_commits
description, body, obsoletes, statuses, patch_complexities, depends = edit_attachment_comment(bug, commit.subject, body)
File "/usr/local/bin/git-bz", line 1866, in edit_attachment_comment
lines = edit_template(template.getvalue())
File "/usr/lib/python2.7/StringIO.py", line 271, in getvalue
self.buf += ''.join(self.buflist)
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 18: ordinal not in range(128)
I didn't get this message when signing off other bugs.
I think the issue is weird characters (quote in this case) in the patch message. UnicodeEncodeError: 'latin-1' codec can't encode character u'\u2018' in position 26: ordinal not in range(256) Created attachment 73510 [details] [review] Bug 19719: Add new column for collection in the patron checkouts data tables This makes the collection visible in the checkouts table on the details and issues tab in the patron account in staff. TEST: 1. Check out a few items to patron. 2. View check outs on patron account in staff client. 3. Collection code is not present. 4. Apply patch 5. Notice that not Collection code is visible while viewing check outs. Signed-off-by: Severine QUEUNE <severine.queune@bulac.fr> Signed-off by: Maksim Sen <maksim.sen@inlibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 73511 [details] [review] Bug 19719: (follow-up) Show description instead of code This follow-up changes the display from collection code to description, following the existing pattern for location. To test: - Repeat test plan from first patch - Verfiy now the collection descrption shows - Change column setting in administration - Verify they work as expected Hi Jesse, a few small things: - please include a description and the test plan in your commit message - please stick to latin-1 :) I've added a follow-up changing the display from collection code to collection name to keep things consistent. Asking for a sign-off for the follow-up. Created attachment 74538 [details] [review] Bug 19719: Add new column for collection in the patron checkouts data tables This makes the collection visible in the checkouts table on the details and issues tab in the patron account in staff. TEST: 1. Check out a few items to patron. 2. View check outs on patron account in staff client. 3. Collection code is not present. 4. Apply patch 5. Notice that not Collection code is visible while viewing check outs. Signed-off-by: Severine QUEUNE <severine.queune@bulac.fr> Signed-off by: Maksim Sen <maksim.sen@inlibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Created attachment 74539 [details] [review] Bug 19719: (follow-up) Show description instead of code This follow-up changes the display from collection code to description, following the existing pattern for location. To test: - Repeat test plan from first patch - Verfiy now the collection descrption shows - Change column setting in administration - Verify they work as expected Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Displaying is correct, depending on the Column settings. Ok for me ! Do we really want to display this column for all installations? We could display for new installations and hide on update? Not sure about the db structure there, have to take a closer look. Update the table's column configuration to initially hide the column? Created attachment 76199 [details] [review] Bug 19719: (follow-up) Add database update to keep collection column hidden For existing installations we want to keep the current behaviour. This patch adds the column configuration to hide the collection column from the details and checkouts tabs until toggled. Proposed a database update :) Comment on attachment 74539 [details] [review] Bug 19719: (follow-up) Show description instead of code Review of attachment 74539 [details] [review]: ----------------------------------------------------------------- ::: svc/checkouts @@ +158,5 @@ > } > + my $collection; > + if ( $c->{collection} ) { > + my $av = Koha::AuthorisedValues->search({ category => 'CCODE', authorised_value => $c->{collection} }); > + $collection = $av->count ? $av->next->lib : ''; Shouldn't we check that items.ccode is linked to the authorized value list CCODE ? We could imagine that in the MARC framework, the item subfield linked to items.ccode is configured to use another AV list (or no AV list at all). If the item subfield is not linked to an AV list and contains free text, it is not displayed. I think that's a problem. (In reply to Julian Maurice from comment #18) > If the item subfield is not linked to an AV list and contains free text, it > is not displayed. I think that's a problem. The same thing is done with items.location (AV list LOC), so passing QA for now to keep consistency. If it has to be fixed, we can fix them both in another bug. Created attachment 77559 [details] [review] Bug 19719: Add new column for collection in the patron checkouts data tables This makes the collection visible in the checkouts table on the details and issues tab in the patron account in staff. TEST: 1. Check out a few items to patron. 2. View check outs on patron account in staff client. 3. Collection code is not present. 4. Apply patch 5. Notice that not Collection code is visible while viewing check outs. Signed-off-by: Severine QUEUNE <severine.queune@bulac.fr> Signed-off by: Maksim Sen <maksim.sen@inlibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 77560 [details] [review] Bug 19719: (follow-up) Show description instead of code This follow-up changes the display from collection code to description, following the existing pattern for location. To test: - Repeat test plan from first patch - Verfiy now the collection descrption shows - Change column setting in administration - Verify they work as expected Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 77561 [details] [review] Bug 19719: (follow-up) Add database update to keep collection column hidden For existing installations we want to keep the current behaviour. This patch adds the column configuration to hide the collection column from the details and checkouts tabs until toggled. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Awesome work all! Pushed to master for 18.11 Enhancement, this will not be backported to 18.05.x series. |