Bug 7480 - The Basket manager is always the logged in user
Summary: The Basket manager is always the logged in user
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-31 13:55 UTC by Adrien SAURAT
Modified: 2015-12-03 22:03 UTC (History)
5 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
Bug 7480: Don't display current logged in user as basket manager (1.30 KB, patch)
2015-03-27 16:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
[Signed-off] Bug 7480: Don't display current logged in user as basket manager (1.49 KB, patch)
2015-03-27 17:23 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 7480: Don't display current logged in user as basket manager (1.54 KB, patch)
2015-04-05 21:03 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Adrien SAURAT 2012-01-31 13:55:20 UTC
In the "neworderempty.pl" file, as we are filling the template->param(*) info beginning on line 346, we have a value appearing twice:

- in line 359,
authorisedbyname     => $basket->{'authorisedbyname'},

- in line 365,
authorisedbyname => $borrower->{'firstname'} . " " . $borrower->{'surname'},

Moreover, this line 365 generates log lines like the following:
[Tue Jan 31 14:39:41 2012] neworderempty.pl: Use of uninitialized value in concatenation (.) or string at /home/asaurat/workspace/versions/community/acqui/neworderempty.pl line 346.
[Tue Jan 31 14:39:41 2012] neworderempty.pl: Use of uninitialized value in concatenation (.) or string at /home/asaurat/workspace/versions/community/acqui/neworderempty.pl line 346.

These log entries are easy to correct if we keep the $borrower version, but that's my question: which line should we keep? Or maybe both values can be ok but one has priority over the other one?

This authorisedbyname variable is linked to a "Managed by:" label in the template.
Comment 1 Adrien SAURAT 2012-01-31 14:40:11 UTC
As seen with kf,
getBasket builds authorisedbyname using  concat( b.firstname,' ',b.surname) AS authorisedbyname, from borrowers
So in both cases we have the same information.

But maybe can problems arise if the borrower has been deleted -> to be handled.
Comment 2 Katrin Fischer 2014-11-09 18:08:55 UTC
Still valid, something for a small clean-up patch:

    authorisedbyname     => $basket->{'authorisedbyname'},
    closedate            => $basket->{'closedate'},
    # order details
    suggestionid         => $suggestion->{suggestionid},
    surnamesuggestedby   => $suggestion->{surnamesuggestedby},
    firstnamesuggestedby => $suggestion->{firstnamesuggestedby},
    biblionumber         => $biblionumber,
    uncertainprice       => $data->{'uncertainprice'},
    authorisedbyname     => $borrower->{'firstname'} . " " . $borrower->{'surname'},
Comment 3 Jonathan Druart 2015-03-27 16:32:26 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2015-03-27 17:23:56 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2015-04-05 21:03:41 UTC
Created attachment 37511 [details] [review]
[PASSED QA] Bug 7480: Don't display current logged in user as basket manager

The "Managed by" field displayed on creating/editing an order is always
the name of the logged in user.

To reproduce:
1/ Log in with patron A
2/ Create a basket
3/ Create an order
4/ Log in with patron B
5/ Edit the order
6/ The "Managed by" line is "patron B"

Test plan:
Apply the patch and confirm that the "patron A" is always the basket
manager.

Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2015-04-05 21:04:40 UTC
Funny noone ever noticed the visible effect of this bug.
Comment 7 Tomás Cohen Arazi 2015-04-08 14:36:43 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 8 Chris Cormack 2015-04-11 05:25:33 UTC
Pushed to 3.18.x will be in 3.18.6