Bug 14376 - Allow the librarian to choose a patron when entering a purchase suggestion
Summary: Allow the librarian to choose a patron when entering a purchase suggestion
Status: RESOLVED DUPLICATE of bug 24819
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Matthias Meusburger
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-11 14:42 UTC by Matthias Meusburger
Modified: 2020-03-06 09:19 UTC (History)
10 users (show)

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


Attachments
Bug 14376: Allow the librarian to choose a patron when entering a purchase suggestion. (4.21 KB, patch)
2015-06-11 15:02 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 14376: Allow the librarian to choose a patron when entering a purchase suggestion. (4.20 KB, patch)
2015-06-15 07:53 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 14376: Allow the librarian to choose a patron when entering a purchase suggestion. (5.94 KB, patch)
2015-06-16 13:35 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 14376: Allow the librarian to choose a patron when entering a purchase suggestion. (6.01 KB, patch)
2015-06-16 22:46 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
(qa fix) fix tt_valid fail on 419 (2.70 KB, patch)
2015-06-16 22:54 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 14376: Fix QA (4.20 KB, patch)
2015-06-23 09:45 UTC, Matthias Meusburger
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Meusburger 2015-06-11 14:42:16 UTC

    
Comment 1 Matthias Meusburger 2015-06-11 15:02:46 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2015-06-14 03:29:22 UTC
Comment on attachment 40095 [details] [review]
Bug 14376: Allow the librarian to choose a patron when entering a purchase suggestion.

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

::: koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
@@ +416,5 @@
>                  <td><input type="text" id="suggesteddate" name="suggesteddate" size="10" maxlength="10" value="[% suggesteddate | $KohaDates %]"/></td>
> +                <td>
> +                  <div class="autocomplete">
> +                    <input autocomplete="on" id="findborrowersuggestion" name="findborrowersuggestion" size="40" class="focus" type="text" [% IF ( suggestedby_borrowernumber ) %] value="[% suggestedby_surname %], [% suggestedby_firstname %] [% Branches.GetName( suggestedby_branchcode ) %] ([% suggestedby_description %])" [% END %]
> +value="" />

Shouldn't there be an [% ELSE %]?
Comment 3 Matthias Meusburger 2015-06-15 07:52:53 UTC
In fact, the ' value="" 'part can be removed. Updating proposed patch.
Comment 4 Matthias Meusburger 2015-06-15 07:53:52 UTC Comment hidden (obsolete)
Comment 5 Indranil Das Gupta 2015-06-15 09:54:05 UTC
Hi Matthias,

(In reply to Matthias Meusburger from comment #4)
> Created attachment 40131 [details] [review] [review]
> Bug 14376: Allow the librarian to choose a patron when entering a purchase
> suggestion.
> 
> Sometimes librarians are creating purchase suggestions that came from patrons
>  which didn't use the opac (but sent an email, or told the librarian
> verbally...)
> 
>  This patch allows the librarian to change the creator of the purchase
>  suggestion when entering it.

The fact that "Suggestion creation" was pre-filled and not editable, served an important function. It created a de-facto audit trail and helped maintain accountability. By making it editable, this patch is taking that function away, without something to replace it. So I disagree with this patch in its present shape.

This bug does have merit though and does address a valid concern. My observations:

1/ Needs additional column in 'suggestions' say 'source' (i.e. of suggestion) with options - 'OPAC|STAFF CLIENT|EMAIL|PHONE|LETTER|OTHER'

2/ Keep the field locked with pre-filled values (current behaviour) with say a check box to edit/change it

3/ Log the edit.

Setting this to 'In Discussion'
Comment 6 Katrin Fischer 2015-06-16 09:34:55 UTC
Hi Indranil,

I think I don't fully understand the problem. 

It is already possible to add suggestions for a patron from their patron account in the staff client - this would just make it a bit more convenient I guess, as yu don't have to search for the patron first (less clicks). 

Maybe logging it would be enough?

I feel like adding a column for 'how the suggestion was made' should be a separate enhancement request.
Comment 7 Indranil Das Gupta 2015-06-16 10:11:58 UTC
Hi Katrin,

(In reply to Katrin Fischer from comment #6)
> Hi Indranil,
> 
> I think I don't fully understand the problem. 
> 
> It is already possible to add suggestions for a patron from their patron
> account in the staff client - this would just make it a bit more convenient
> I guess, as yu don't have to search for the patron first (less clicks). 
> 
> Maybe logging it would be enough?

yep! if logging is added, I can certainly test and sign off this one.


> I feel like adding a column for 'how the suggestion was made' should be a
> separate enhancement request.

Fair observation. I agree.
Comment 8 Matthias Meusburger 2015-06-16 13:35:03 UTC Comment hidden (obsolete)
Comment 9 Matthias Meusburger 2015-06-16 13:35:28 UTC
This patch adds logging support.
Comment 10 Indranil Das Gupta 2015-06-16 22:46:26 UTC
Created attachment 40202 [details] [review]
Bug 14376: Allow the librarian to choose a patron when entering a purchase suggestion.

Sometimes librarians are creating purchase suggestions that came from patrons
 which didn't use the opac (but sent an email, or told the librarian verbally...)

 This patch allows the librarian to change the creator of the purchase suggestion
 when entering it.

 This way, the patron will be able to receive notifications during the purchase
 suggestion workflow.

 Test plan:
  - Create a new suggestion
  - Edit the input "By" from "Suggestion creation"
  - Check that autocomplete works.
  - Submit the suggestion with a user different than the logged-in librarian.
  - Check that the correct user is recorded for this suggestion.

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Comment 11 Indranil Das Gupta 2015-06-16 22:54:43 UTC
Created attachment 40203 [details] [review]
(qa fix) fix tt_valid fail on 419

Line 419 in attachment 40181 [details] [review] introduced TT code into <input>
causing tt_valid qa tools fail. this patch fixes the issue.

Test plan
=========

1/ run koha qa test tools (with the first patch applies)
   it should fails on suggestion.tt

   FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
   OK	  forbidden patterns
   FAIL	  tt_valid
		lines 419
   OK	  valid_template

2/ apply this qa fix patch

3/ run koha qa tools again, this time the tt_valid test should clear

   OK	koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
   OK	  forbidden patterns
   OK	  tt_valid

   OK	  valid_template
Comment 12 Jonathan Druart 2015-06-18 11:04:58 UTC
This is not really user friendly.
1/ The input is too short and does not contain all the data (at least in my case).
2/ The text displayed after you have chosen a patron is not the same as when you arrive on the page.
3/ There are already several way to search for a patron, no need to introduce a new one.

You could add an edit button and use it the same way as when we add users to a basket (popup), or the button will display the input with the autocompletion.
Comment 13 Matthias Meusburger 2015-06-23 09:45:02 UTC
Created attachment 40514 [details] [review]
Bug 14376: Fix QA

 - Add an 'edit button'
 - Make the dynamic patron information more consistent with the original one
 - Make the input field wider
Comment 14 Matthias Meusburger 2015-06-23 09:48:12 UTC
About your second point Jonathan, this new patch make the dynamic patron information more consistent with the original one. 

However, I did not add the category description in ysearch.pl on purpose, because I have concerns about performances issues if added. Please tell me how you feel about that.
Comment 15 Jonathan Druart 2015-06-24 11:10:13 UTC
I am still not comfortable with what you propose.
I will let another QAer to have a look.

Did you look at the way a guarantor is added to a patron? (members/memberentry.pl, button "Set to patron")
Comment 16 Vinod 2015-06-30 20:30:42 UTC
So whats final decision? i think there must be option to choose at least suggester name by searching their ID, so the order reports must also include suggester name along with their ID as well as other titles details. It has been added in 3.20 to notify the multiple patron on receiving the book so i thing may serve the purpose or in same way it can be made. In purchase suggestion the default (staff who enters the data) must be lifted and option must be given to choose the requester by name/ID.
Comment 17 Katrin Fischer 2015-10-25 14:17:20 UTC
Hi Matthias, 

I've taken a look at the code and the GUI, but there are some small and some bigger things bothering me:

1) Display of patron names (trivial)

+            .append( "<a>" + item.surname + ", " + item.firstname + ", " + item.branch + "</a>" )
Please also think about the case where the patron has no firstname (organisations for example)

2) Logging

+        my $member = GetMember(borrowernumber => $$suggestion_only{'suggestedby'});
+        logaction('ACQUISITIONS', 'MODIFY', undef, 'Suggestion title: ' . $$suggestion_only{'title'} . ", creator: " .
+                                               $member->{'surname'} . ', ' .
+                                               $member->{'firstname'} . ' (' .
+                                               $member->{'cardnumber'} . ')' );

a) I think we need to make more clear about which action in acquisitions this
entry is about. I'd suggest to either add a new module 'SUGGESTIONS' 
or to be more clear about the action: 'MODIFY SUGG' or similar.

b) For privacy reasons I am also quite uncomfortable about adding the name 
in plain text to the logs. We don't need this information if we reference
the borrowernumber instead and it will also be better for privacy.
For me this is a (blocker).

c) Why are you leaving the object undef? I feel like this should be the
suggestion id. Then we could just have the borrowernumber in the
description. All other information can easily be found.

d) I think the new logging needs to be covered by a system preference 
to be consistent with current practice.

-- 
Don't misunderstand me - I really think we should do more nice logging!
But I think we should try to avoid some of the problems we had with it
in the past. One idea might even be to add a new column to action_logs to
be able to store another id in a better way (user, object, affected_user?)

3) GUI

I'd prefer the way patron search is done in other places taking place
in a separate pop-up or modal. Because it offers a better overview
for libraries with a lot of patrons and better options to limit search.
An example would be the way it's done for searching patrons to be
notified about an order (when creating an order line).
Comment 18 Liz Rea 2016-02-15 00:23:34 UTC
Hi,

I just noticed this bug, wanted to say that I think this is a great idea and I hope you come back to it.

Cheers,
Liz
Comment 19 Katrin Fischer 2016-02-15 06:33:05 UTC
Hi Amit, why did you change the status? I think Failed QA is correct, as there are patches to work with.
Comment 20 Fridolin Somers 2019-05-07 16:05:42 UTC
> 3) GUI
> I'd prefer the way patron search is done in other places taking place
in a separate pop-up or modal.
Is it already existing somewhere is actual code ?
Comment 21 Katrin Fischer 2019-05-07 17:22:23 UTC
(In reply to Fridolin SOMERS from comment #20)
> > 3) GUI
> > I'd prefer the way patron search is done in other places taking place
> in a separate pop-up or modal.
> Is it already existing somewhere is actual code ?

Take a look at how patrons can be added as:
- guarantors
- to be notified when an order is received (edit order screen)

For examples.
Comment 22 Fridolin Somers 2019-05-14 10:05:15 UTC
Thanks a lot Katrin :D
Comment 23 Matthias Meusburger 2020-03-06 09:19:28 UTC

*** This bug has been marked as a duplicate of bug 24819 ***