Bug 14039 - Add patron title to checkout screen
Summary: Add patron title to checkout screen
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: David Roberts
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 19456
  Show dependency treegraph
 
Reported: 2015-04-22 09:21 UTC by David Roberts
Modified: 2017-11-02 11:24 UTC (History)
7 users (show)

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


Attachments
Proposed patch (1.28 KB, patch)
2015-04-22 09:26 UTC, David Roberts
Details | Diff | Splinter Review
Add patron salutation to checkout screen (1.22 KB, patch)
2015-04-24 15:30 UTC, David Roberts
Details | Diff | Splinter Review
[Signed-off] Add patron salutation to checkout screen (1.63 KB, patch)
2015-04-24 18:19 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 14039 - Add patron salutation to checkout screen (1.55 KB, patch)
2015-05-01 12:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Add patron salutation to checkout screen (1.35 KB, patch)
2017-06-06 23:13 UTC, David Roberts
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14039 - Add patron salutation to checkout screen (1.60 KB, patch)
2017-06-07 12:41 UTC, Owen Leonard
Details | Diff | Splinter Review
Add patron salutation to checkout screen (1.35 KB, patch)
2017-08-03 22:26 UTC, David Roberts
Details | Diff | Splinter Review
[SIGNED-OFF] Add patron salutation to checkout screen (1.41 KB, patch)
2017-08-04 12:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 14039: Add the title to all places (4.75 KB, patch)
2017-08-09 19:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Add patron salutation to checkout screen (1.46 KB, patch)
2017-10-07 00:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14039: Add the title to all places (4.82 KB, patch)
2017-10-07 00:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14039: Fix patron search (2.95 KB, patch)
2017-10-10 13:16 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Roberts 2015-04-22 09:21:32 UTC
When checking out an item to a user, it sounds more professional if the patron can be addressed as Mr / Mrs / Miss etc, but this information doesn't display to the staff member, who has to click on the details tab in order to find out how the patron should be addressed.
Comment 1 David Roberts 2015-04-22 09:26:42 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2015-04-23 08:28:41 UTC
Wanted to test, does not apply:

CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/patron-title.inc
Failed to merge in the changes.
Comment 3 David Roberts 2015-04-24 15:30:50 UTC Comment hidden (obsolete)
Comment 4 David Roberts 2015-04-24 15:33:21 UTC
Sorry, my mistake - I was tracking the wrong version! A new patch has now been submitted so hopefully it will work this time.
Comment 5 Marc Véron 2015-04-24 18:19:10 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2015-05-01 12:38:41 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2015-06-05 15:32:04 UTC
1) I think this patch introduces a change in much more places than expected:

$ git grep patron-title.inc | wc -l
1491

I suggest that such a change should be optional, and could be done like this:

-            [%- borrower.firstname %] [% IF borrower.othernames %] ([% borrower.othernames %]) [% END %] [% borrower.surname %]
+            <span id="patron-title">[%- borrower.title %] </span>[% borrower.firstname %] [% IF borrower.othernames %] ([% borrower.othernames %]) [% END %] [% borrower.surname %]

i.e. putting the title inside a span with an id, and change CSS to hide it by default. So you keep the enhancement you want, but people can opt-in instead of making it mandatory (e.g. #patron-title { display: none; } )

2) Please make it check for borrower.title to avoid warnings? [%- IF borrower.title -%] etc
Comment 8 David Roberts 2017-06-06 23:13:40 UTC
Created attachment 64069 [details] [review]
Add patron salutation to checkout screen
Comment 9 David Roberts 2017-06-06 23:14:11 UTC
Belatedly revisiting this. This patch was intended to only apply to the checkout screen as this is where the customer wanted it to display, but if it appears elsewhere then that isn't the end of the world. 

I've added a span to the title so that it can be hidden in CSS - I believe it is better to deliver this with it enabled so that the user knows the functionality is there, and can be easily hidden. It is much harder to inform the user that it is possible to enable a function they don't know exists.

I've added the test to prevent errors it there is no borrower.title present in the user record. 

Amended test plan:

1) Install this patch.
2) Ensure a user has a salutation (Mr, Dr, Miss etc) set, and checkout an item to this user.
In the area above the barcode box, the patron's title should display, e.g:

Checking out to Mr John Doe (0001).

3) Add #patron-title { display: none; } to IntranetUserCSS
Check that the patron title no longer displays

4) With a user without a salutation, check that no errors display on checkout.
Comment 10 David Roberts 2017-06-06 23:16:36 UTC
Sorry, I don't know why the original patch (38734) wasn't marked as obsolete
Comment 11 Owen Leonard 2017-06-07 12:41:50 UTC
Created attachment 64077 [details] [review]
[SIGNED-OFF] Bug 14039 - Add patron salutation to checkout screen

To test:

Install this patch.
Ensure a user has a salutation (Mr, Dr, Miss etc) set, and checkout an
item to this user.
In the area above the barcode box, the patron's title should display,
e.g:

Checking out to Mr John Doe (0001).

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 12 Owen Leonard 2017-06-07 12:44:14 UTC
(In reply to Tomás Cohen Arazi from comment #7)
> I suggest that such a change should be optional

This patch does not make the change hidden by default, but I have signed off because I think it's okay the way it is. I believe that if libraries are collecting this information they most likely would like to see it.
Comment 13 David Roberts 2017-07-07 14:43:22 UTC
Sorry, edited the wrong bug. Changing status back to signed off.
Comment 14 Jonathan Druart 2017-07-13 17:43:58 UTC
Comment on attachment 64077 [details] [review]
[SIGNED-OFF] Bug 14039 - Add patron salutation to checkout screen

Review of attachment 64077 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/includes/patron-title.inc
@@ +5,4 @@
>          [%- IF invert_name %]
>              [%- borrower.surname | html %], [% borrower.firstname | html %] [% IF borrower.othernames %] ([% borrower.othernames | html %]) [% END %]
>          [%- ELSE %]
> +            [% IF borrower.title %] <span id="patron-title">[%- borrower.title | html %] </span>[% END %][%- borrower.firstname | html %] [% IF borrower.othernames %] ([% borrower.othernames | html %]) [% END %] [% borrower.surname | html %]

I do not think it's a good idea to have an id here, there are certainly places where this include file is used several times.
Comment 15 David Roberts 2017-07-14 09:49:16 UTC
The only reason I added an id was because it failed QA without one. Now it's failed QA with one! Can't win.
Comment 16 Katrin Fischer 2017-07-14 11:13:53 UTC
Hi David, 
I think you can fix it easily - just make the id a class. If you have multiple elements with the same id on a page, this will cause problems, for example you will not be able to target them with jQuery in my experience. So Jonathan is right that we might want to be careful here.
class attributes can appear many times and will work nicely here to be on the safe side.
Comment 17 David Roberts 2017-08-03 22:26:12 UTC
Created attachment 65482 [details] [review]
Add patron salutation to checkout screen
Comment 18 David Roberts 2017-08-03 22:28:49 UTC
Thanks Katrin,

I've made this change. The test plan is now:

1) Install this patch.
2) Ensure a user has a salutation (Mr, Dr, Miss etc) set, and checkout an item to this user.
In the area above the barcode box, the patron's title should display, e.g:

Checking out to Mr John Doe (0001).

3) Add .patron-title { display: none; } to IntranetUserCSS
Check that the patron title no longer displays

4) With a user without a salutation, check that no errors display on checkout.
Comment 19 Owen Leonard 2017-08-04 12:23:50 UTC
Created attachment 65496 [details] [review]
[SIGNED-OFF] Add patron salutation to checkout screen

https://bugs.koha-community.org/show_bug.cgi?id=14039

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 20 Jonathan Druart 2017-08-09 19:49:14 UTC
Created attachment 65777 [details] [review]
Bug 14039: Add the title to all places

There are other scripts where the borrower variable is not defined and
the fields are passed one by one.
To have a consistent behaviour we should add the title at the different
places.

Note that this script also add the use of the include file for
statistics.tt and remove the pass of parameters to the template, already
done later:
 99 $template->param(%$borrower);
Comment 21 Nick Clemens 2017-10-07 00:51:46 UTC
Created attachment 67766 [details] [review]
Add patron salutation to checkout screen

https://bugs.koha-community.org/show_bug.cgi?id=14039

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Nick Clemens 2017-10-07 00:51:49 UTC
Created attachment 67767 [details] [review]
Bug 14039: Add the title to all places

There are other scripts where the borrower variable is not defined and
the fields are passed one by one.
To have a consistent behaviour we should add the title at the different
places.

Note that this script also add the use of the include file for
statistics.tt and remove the pass of parameters to the template, already
done later:
 99 $template->param(%$borrower);

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Jonathan Druart 2017-10-09 19:17:19 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 24 Jonathan Druart 2017-10-10 13:16:24 UTC
Created attachment 67870 [details] [review]
Bug 14039: Fix patron search

The JSON was malformed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Jonathan Druart 2017-10-10 13:20:23 UTC
Last patch pushed to master.