Bug 37211 - All notes in the patron account transactions table should be editable
Summary: All notes in the patron account transactions table should be editable
Status: Pushed to main
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: t
Keywords: release-notes-needed
Depends on: 23674
Blocks: 39491
  Show dependency treegraph
 
Reported: 2024-06-27 14:44 UTC by Lucas Gass (lukeg)
Modified: 2025-04-22 06:42 UTC (History)
5 users (show)

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


Attachments
Bug 37211: Make notes in patron transactions table editable (6.60 KB, patch)
2024-06-27 19:23 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 37211: Add new subpermission in updatecharge 'edit_notes' (2.92 KB, patch)
2024-06-27 19:23 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 37211: Make notes in patron transactions table editable (6.61 KB, patch)
2024-08-02 15:21 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 37211: Add new subpermission in updatecharge 'edit_notes' (2.92 KB, patch)
2024-08-02 15:21 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 37211: Make notes in patron transactions table editable (6.66 KB, patch)
2024-08-09 20:04 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 37211: Add new subpermission in updatecharge 'edit_notes' (2.97 KB, patch)
2024-08-09 20:04 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 37211: (QA follow-up) Don't reset creation date on update (1.03 KB, patch)
2025-01-28 09:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37211: (QA follow-up) Update database update to be idempotent (1.11 KB, patch)
2025-01-28 10:02 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37211: Make notes in patron transactions table editable (6.73 KB, patch)
2025-01-28 10:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37211: Add new subpermission in updatecharge 'edit_notes' (3.04 KB, patch)
2025-01-28 10:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37211: (QA follow-up) Don't reset creation date on update (1.09 KB, patch)
2025-01-28 10:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37211: (QA follow-up) Update database update to be idempotent (1.18 KB, patch)
2025-01-28 10:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37211: (QA follow-up) Update for bootstrap5 and remove copy/paste (4.00 KB, patch)
2025-01-28 10:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37211: (QA follow-up) Make note edit button styles less intrusive, align (2.00 KB, patch)
2025-01-28 11:23 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37211: Make notes in patron transactions table editable (5.84 KB, patch)
2025-03-11 19:43 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37211: Add new subpermission in updatecharge 'edit_notes' (3.19 KB, patch)
2025-03-11 19:43 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37211: (QA follow-up) Update database update to be idempotent (1.18 KB, patch)
2025-03-11 19:43 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37211: (QA follow-up) Update for bootstrap5 and remove copy/paste (3.93 KB, patch)
2025-03-11 19:43 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
problem with buttons when using flex (8.65 KB, image/png)
2025-03-11 19:46 UTC, Lucas Gass (lukeg)
Details
Bug 37211: Tidy (1.77 KB, patch)
2025-03-31 09:51 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37211: Update permission name to edit_accountline_notes (4.19 KB, patch)
2025-03-31 09:58 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37211: Use HTML textarea for the notes (1.26 KB, patch)
2025-03-31 10:19 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37211: Use HTML textarea for the notes (1.23 KB, patch)
2025-03-31 11:14 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37211: (follow-up) Correct Auth/Permissions.t test to account for new permission (1.07 KB, patch)
2025-04-14 16:32 UTC, Lucas Gass (lukeg)
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) 2024-06-27 14:44:24 UTC
Bug 23674 will add the ability to have a note on the transaction table in a patron account. This enhancement will make each of those notes editable after they have been submitted.
Comment 1 Sam Lau 2024-06-27 19:23:16 UTC Comment hidden (obsolete)
Comment 2 Sam Lau 2024-06-27 19:23:18 UTC Comment hidden (obsolete)
Comment 3 Roman Dolny 2024-07-29 14:34:26 UTC
Applying: Bug 37211: Make notes in patron transactions table editable
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt).
error: could not build fake ancestor
Patch failed at 0001 Bug 37211: Make notes in patron transactions table editable
Comment 4 Sam Lau 2024-08-02 15:21:52 UTC Comment hidden (obsolete)
Comment 5 Sam Lau 2024-08-02 15:21:54 UTC Comment hidden (obsolete)
Comment 6 Eric Garcia 2024-08-05 17:44:11 UTC
Applying: Bug 37211: Add new subpermission in updatecharge 'edit_notes'
error: sha1 information is lacking or useless (installer/data/mysql/mandatory/userpermissions.sql).
error: could not build fake ancestor
Patch failed at 0001 Bug 37211: Add new subpermission in updatecharge 'edit_notes'
Comment 7 Sam Lau 2024-08-05 18:17:26 UTC
(In reply to Eric Garcia from comment #6)
> Applying: Bug 37211: Add new subpermission in updatecharge 'edit_notes'
> error: sha1 information is lacking or useless
> (installer/data/mysql/mandatory/userpermissions.sql).
> error: could not build fake ancestor
> Patch failed at 0001 Bug 37211: Add new subpermission in updatecharge
> 'edit_notes'

I'm not getting this... maybe try a 'git pull'?
Comment 8 Roman Dolny 2024-08-09 20:04:26 UTC Comment hidden (obsolete)
Comment 9 Roman Dolny 2024-08-09 20:04:28 UTC Comment hidden (obsolete)
Comment 10 Martin Renvoize (ashimema) 2025-01-28 09:40:15 UTC
Comment on attachment 170218 [details] [review]
Bug 37211: Make notes in patron transactions table editable

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

::: members/boraccount.pl
@@ +235,5 @@
> +        sub {
> +            # Update the note and date in the account line
> +            $payment->set(
> +                {
> +                    date => \'NOW()',

I'm not sure we should be updating the date here... we have a timestamp field in the accountlines table that already updates on any change to the accountline.. the date field is meant for recording when the accountline was created.
Comment 11 Martin Renvoize (ashimema) 2025-01-28 09:58:14 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize (ashimema) 2025-01-28 10:02:02 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize (ashimema) 2025-01-28 10:40:43 UTC
Created attachment 177248 [details] [review]
Bug 37211: Make notes in patron transactions table editable

This patch adds an edit button to each notes field in an accountline entry. This feature is controlled by a new staff permision under the 'updatecharges' permision called 'edit_notes'.

To test:
1) Apply patch, updatedatabase, restart_all
2) In the staff interface, give your logged in user superlibrarian permissions.
3) Visit a patron's accounting tab. Create a manual invoice for some amount and enter a note.
4) Create another manual invoice, this time do not enter a note.
5) Click on the transactions tab to view the table.
6) See that the notes column now has edit buttons.
7) Attempt to edit the note that is not blank.
8) A modal should pop up prompting you to edit your note. The previous note should be in the text box.
9) Edit this note and press confirm.
10) Ensure the note was updated and the updated column was filled with the correct time.
11) Now attempt to edit the blank note.
12) Again, you should see a modal but no prefilled message. Update this note and ensure correct changes.
13) Go back to the permissions for your logged in user and remove the 'edit_notes' subpermission under the 'updatecharges' permissions.
14) Go back to the patron's accounting tab. This time, there should not be any edit buttons in the notes column.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2025-01-28 10:40:46 UTC
Created attachment 177249 [details] [review]
Bug 37211: Add new subpermission in updatecharge 'edit_notes'

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize (ashimema) 2025-01-28 10:40:49 UTC
Created attachment 177250 [details] [review]
Bug 37211: (QA follow-up) Don't reset creation date on update

We shouldn't be resetting an accountlines creation date when we update
the associated note. We have 'timestamp' for updates already.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize (ashimema) 2025-01-28 10:40:51 UTC
Created attachment 177251 [details] [review]
Bug 37211: (QA follow-up) Update database update to be idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize (ashimema) 2025-01-28 10:40:54 UTC
Created attachment 177252 [details] [review]
Bug 37211: (QA follow-up) Update for bootstrap5 and remove copy/paste

There were some unneeded data elements being passed around and we also
needed to update from data- to data-bs- in places to get the modals
working again.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize (ashimema) 2025-01-28 10:42:01 UTC
I did some required follow-ups here to get it over the line.

Last comment is that I'm not sure about the styling/placement of the edit button... but I'm sure someone clever will come along and help clean that up so I'm not going to block the feature here.

Passing QA
Comment 19 Paul Derscheid 2025-01-28 11:23:23 UTC
Created attachment 177261 [details] [review]
Bug 37211: (QA follow-up) Make note edit button styles less intrusive, align

To override the padding on .btn-xs I had to inline style="padding: 0;", it would be nice to have inline-btn variants (that come without padding or margin) for datatables because this is probably a common problem.
Comment 20 Lucas Gass (lukeg) 2025-03-11 19:43:14 UTC
Created attachment 179178 [details] [review]
Bug 37211: Make notes in patron transactions table editable
Comment 21 Lucas Gass (lukeg) 2025-03-11 19:43:16 UTC
Created attachment 179179 [details] [review]
Bug 37211: Add new subpermission in updatecharge 'edit_notes'

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Lucas Gass (lukeg) 2025-03-11 19:43:18 UTC
Created attachment 179180 [details] [review]
Bug 37211: (QA follow-up) Update database update to be idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Lucas Gass (lukeg) 2025-03-11 19:43:20 UTC
Created attachment 179181 [details] [review]
Bug 37211: (QA follow-up) Update for bootstrap5 and remove copy/paste
Comment 24 Lucas Gass (lukeg) 2025-03-11 19:44:58 UTC
rebased patches for current main. 

Paul, I like your follow-up except for one problem with flex. I will attach an example.
Comment 25 Lucas Gass (lukeg) 2025-03-11 19:46:30 UTC
Created attachment 179182 [details]
problem with buttons when using flex
Comment 26 Katrin Fischer 2025-03-17 10:39:55 UTC
Hm, these patches are from last week, but I get an untidy warning:

0) QA script

 FAIL	installer/data/mysql/atomicupdate/bug_37211.pl
   FAIL	  tidiness
		File is not tidy, please run `perl misc/devel/tidy.pl installer/data/mysql/atomicupdate/bug_37211.pl`

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
   FAIL	  tidiness
		File is not tidy, please run `perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc`

Please check your setup! I have seen some of those and would like to get to the bottom of this. it's time consuming to squash into the right patches also.


There are also some other small issues:

1) We are missing the permission in the installer file for new installations. (blocker)

2) The permission name is very general with "edit_notes". I think it might pay off to be a bit more specific for people looking at these in reports etc. to give an example.  edit_accountlines_notes? (suggestion)

3) The input field for changing/editing the note is quite small. Can we make that a big more generous please? If you need to "scroll" within a longer text field for editing, this leads to errors and grup easily. (kind of blocker)
Comment 27 Lucas Gass (lukeg) 2025-03-31 09:51:15 UTC
Created attachment 179963 [details] [review]
Bug 37211: Tidy
Comment 28 Lucas Gass (lukeg) 2025-03-31 09:58:44 UTC
Created attachment 179967 [details] [review]
Bug 37211: Update permission name to edit_accountline_notes
Comment 29 Lucas Gass (lukeg) 2025-03-31 10:03:25 UTC
> There are also some other small issues:
> 
> 1) We are missing the permission in the installer file for new
> installations. (blocker)

Is there somewhere besides installer/data/mysql/mandatory/userpermissions.sql  that I am missing?
Comment 30 Lucas Gass (lukeg) 2025-03-31 10:07:58 UTC
> 3) The input field for changing/editing the note is quite small. Can we make
> that a big more generous please? If you need to "scroll" within a longer
> text field for editing, this leads to errors and grup easily. (kind of
> blocker)


Should we use textarea here?
Comment 31 Lucas Gass (lukeg) 2025-03-31 10:19:23 UTC
Created attachment 179972 [details] [review]
Bug 37211: Use HTML textarea for the notes
Comment 32 Lucas Gass (lukeg) 2025-03-31 11:14:25 UTC
Created attachment 179977 [details] [review]
Bug 37211: Use HTML textarea for the notes
Comment 33 Katrin Fischer 2025-04-14 06:52:27 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 34 Katrin Fischer 2025-04-14 15:49:19 UTC
Hi Lucas, 

we have a failing test, can you please check?

t_Koha_Auth_Permissions_t.superlibrarian tests
Comment 35 Lucas Gass (lukeg) 2025-04-14 16:32:25 UTC
Created attachment 180923 [details] [review]
Bug 37211: (follow-up) Correct Auth/Permissions.t test to account for new permission
Comment 36 Lucas Gass (lukeg) 2025-04-14 16:32:58 UTC
(In reply to Katrin Fischer from comment #34)
> Hi Lucas, 
> 
> we have a failing test, can you please check?
> 
> t_Koha_Auth_Permissions_t.superlibrarian tests

I added a follow-up patch here that should fix the issue.
Comment 37 Katrin Fischer 2025-04-22 06:42:11 UTC
Follow-up pushed to main! (just forgot to note here)