Bug 34377 - Accounting transactions should show managing librarian info for credits/debits
Summary: Accounting transactions should show managing librarian info for credits/debits
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Lucas Gass (lukeg)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-25 16:24 UTC by Lucas Gass (lukeg)
Modified: 2024-07-04 20:37 UTC (History)
2 users (show)

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


Attachments
Bug 34377: Add managing librarain (2.35 KB, patch)
2023-09-07 22:57 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 34377: Add managing librarain (2.41 KB, patch)
2023-09-08 09:11 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 34377: (follow-up) Use patron-title include (2.96 KB, patch)
2023-09-08 09:11 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 34377: Add managing librarain (2.42 KB, patch)
2023-10-17 12:28 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 34377: (follow-up) Use patron-title include (3.09 KB, patch)
2023-10-17 12:28 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass (lukeg) 2023-07-25 16:24:31 UTC
To recreate:

1. From the patron Accounting tab > "Create manual credit"
2. After doing so visit accountline-details.pl for that particular transaction.
3. Notice that there is no related transaction information. 
4. It would be nice if this displayed, especially the managing Librarian bit.
Comment 1 Lucas Gass (lukeg) 2023-09-07 22:49:08 UTC
My mistake. Where the managing librarian is needed is in the Details of debit/credit table.
Comment 2 Lucas Gass (lukeg) 2023-09-07 22:57:12 UTC
Created attachment 155345 [details] [review]
Bug 34377: Add managing librarain

1. From the patron Accounting tab > "Create manual credit".
2. From the patron Accounting tab > "Create manua
3. Go to the Transaction tab (members/boraccount.pl) and click the 'Details' button.
4. Do this for  both a debit and a credit.
5. No info. about which librarain administered the credit/debit.
6. Apply patch
7. Try 3 -6 again, notice the table now shows which librarian administed the credit/debit.
Comment 3 Martin Renvoize (ashimema) 2023-09-08 01:51:52 UTC
You construct the name display here manually.. don't we have an include for that to ensure consistency?
Comment 4 Martin Renvoize (ashimema) 2023-09-08 09:11:34 UTC
Created attachment 155379 [details] [review]
Bug 34377: Add managing librarain

1. From the patron Accounting tab > "Create manual credit".
2. From the patron Accounting tab > "Create manua
3. Go to the Transaction tab (members/boraccount.pl) and click the 'Details' button.
4. Do this for  both a debit and a credit.
5. No info. about which librarain administered the credit/debit.
6. Apply patch
7. Try 3 -6 again, notice the table now shows which librarian administed the credit/debit.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize (ashimema) 2023-09-08 09:11:36 UTC
Created attachment 155380 [details] [review]
Bug 34377: (follow-up) Use patron-title include

This switched the existing manual handling of patron title construction
to use the accepted include method.
Comment 6 Martin Renvoize (ashimema) 2023-09-08 09:14:03 UTC
I signed off and added a follow-up to convert to using the aforementioned include.

I'm confused by that include.. it requires `hide_patron_infos_if_needed => 1` is passed to allow a link to be created. In this case, we're looking at a librarian, I sort of feel like we should be able to override the hide but that would nuke the link which would be a regression I believe.

Anywho.. it's looking good to me now.
Comment 7 Lucas Gass (lukeg) 2023-09-15 15:21:58 UTC
(In reply to Martin Renvoize from comment #6)
> I signed off and added a follow-up to convert to using the aforementioned
> include.
> 
> I'm confused by that include.. it requires `hide_patron_infos_if_needed =>
> 1` is passed to allow a link to be created. In this case, we're looking at a
> librarian, I sort of feel like we should be able to override the hide but
> that would nuke the link which would be a regression I believe.
> 
> Anywho.. it's looking good to me now.

Thank you, Martin!
Comment 8 Martin Renvoize (ashimema) 2023-10-17 12:28:45 UTC
Created attachment 157234 [details] [review]
Bug 34377: Add managing librarain

1. From the patron Accounting tab > "Create manual credit".
2. From the patron Accounting tab > "Create manua
3. Go to the Transaction tab (members/boraccount.pl) and click the 'Details' button.
4. Do this for  both a debit and a credit.
5. No info. about which librarain administered the credit/debit.
6. Apply patch
7. Try 3 -6 again, notice the table now shows which librarian administed the credit/debit.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize (ashimema) 2023-10-17 12:28:47 UTC
Created attachment 157235 [details] [review]
Bug 34377: (follow-up) Use patron-title include

This switched the existing manual handling of patron title construction
to use the accepted include method.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 10 Martin Renvoize (ashimema) 2023-10-17 12:29:20 UTC
Tiny rebase.. deciding to upgrade my SO to a PQA as the patch is trivial, clear and QA scripts are happy.
Comment 11 Lucas Gass (lukeg) 2023-10-17 14:50:55 UTC
Thank you, Martin!
Comment 12 Tomás Cohen Arazi (tcohen) 2023-10-25 14:09:56 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 13 Fridolin Somers 2023-10-26 08:21:49 UTC
Enhancement not pushed to 23.05.x