Bug 17473

Summary: Permission for voiding payments
Product: Koha Reporter: Josef Moravec <josef.moravec>
Component: PatronsAssignee: Eric Garcia <cubingguy714>
Status: Failed QA --- QA Contact: Lucas Gass (lukeg) <lucas>
Severity: enhancement    
Priority: P5 - low CC: black23, gmcharlt, josef.moravec, kyle.m.hall, lucas, mail, martin.renvoize, roman.dolny
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
RM REVERTED
Version(s) released in:
Circulation function:
Bug Depends on: 18790    
Bug Blocks: 24849    
Attachments: Bug 17473: Add void payment permission
Bug 17473: Add void payment permission
Bug 17473: (follow-up) add semicolon after use Modern::Perl
Bug 17473: Add void payment permission
Bug 17473: (follow-up) add semicolon after use Modern::Perl
Bug 17473: Add void payment permission
Bug 17473: (follow-up) add semicolon after use Modern::Perl

Description Josef Moravec 2016-10-20 08:08:50 UTC
It would be nice to have permission just for reverse payments
Comment 1 Katrin Fischer 2018-05-16 23:08:09 UTC
With bug 20629 the option to reverse payments will be gone. I am changing this to be a permission for the replacement: voiding payments.
Comment 2 Eric Garcia 2024-07-17 20:45:58 UTC
Created attachment 169109 [details] [review]
Bug 17473: Add void payment permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Search for staff patron
3. More -> Set permissions
4. Check
    - Staff access, allows viewing of catalogue in staff interface
    - Add, modify and view patron information
    - Manage patrons fines and fee
5. Notice under Manage patrons fines and fee there is a new permission "Voiding Payments"
6. Sign into staff interface as your selected staff patron
7. Search for a non-staff patron -> Accounting
8. Click 'Create manual invoice', add an amount and click 'Save and pay' -> 'Confirm'
9. Go to transactions and notice the option to Void payment appears
10. Go back to your staff patron's permission and unselect 'Voiding Payments'
11. Go to your non-staff patron and notice the option to void payments is gone
Comment 3 Eric Garcia 2024-07-22 15:27:54 UTC
Created attachment 169310 [details] [review]
Bug 17473: Add void payment permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Search for staff patron
3. More -> Set permissions
4. Check
    - Staff access, allows viewing of catalogue in staff interface
    - Add, modify and view patron information
    - Manage patrons fines and fee
5. Notice under Manage patrons fines and fee there is a new permission "Voiding Payments"
6. Sign into staff interface as your selected staff patron
7. Search for a non-staff patron -> Accounting
8. Click 'Create manual invoice', add an amount and click 'Save and pay' -> 'Confirm'
9. Go to transactions and notice the option to Void payment appears
10. Go back to your staff patron's permission and unselect 'Voiding Payments'
11. Go to your non-staff patron and notice the option to void payments is gone
Comment 4 Roman Dolny 2024-07-22 19:40:25 UTC
There is error during updatedatabase:

ERROR - {UNKNOWN}: Can't use an undefined value as a subroutine reference at /kohadevbox/koha/C4/Installer.pm line 739. at /kohadevbox/koha/C4/Installer.pm line 741
Comment 5 Lucas Gass (lukeg) 2024-07-22 20:24:16 UTC
Created attachment 169384 [details] [review]
Bug 17473: (follow-up) add semicolon after use Modern::Perl
Comment 6 Roman Dolny 2024-07-22 20:56:01 UTC
Created attachment 169386 [details] [review]
Bug 17473: Add void payment permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Search for staff patron
3. More -> Set permissions
4. Check
    - Staff access, allows viewing of catalogue in staff interface
    - Add, modify and view patron information
    - Manage patrons fines and fee
5. Notice under Manage patrons fines and fee there is a new permission "Voiding Payments"
6. Sign into staff interface as your selected staff patron
7. Search for a non-staff patron -> Accounting
8. Click 'Create manual invoice', add an amount and click 'Save and pay' -> 'Confirm'
9. Go to transactions and notice the option to Void payment appears
10. Go back to your staff patron's permission and unselect 'Voiding Payments'
11. Go to your non-staff patron and notice the option to void payments is gone

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 7 Roman Dolny 2024-07-22 20:56:04 UTC
Created attachment 169387 [details] [review]
Bug 17473: (follow-up) add semicolon after use Modern::Perl

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 8 Lucas Gass (lukeg) 2024-11-22 18:17:58 UTC
This all looks right to me. DB update is correct, simple change to templates. I am passing QA here.
Comment 9 Lucas Gass (lukeg) 2024-11-22 18:18:42 UTC
Created attachment 174943 [details] [review]
Bug 17473: Add void payment permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Search for staff patron
3. More -> Set permissions
4. Check
    - Staff access, allows viewing of catalogue in staff interface
    - Add, modify and view patron information
    - Manage patrons fines and fee
5. Notice under Manage patrons fines and fee there is a new permission "Voiding Payments"
6. Sign into staff interface as your selected staff patron
7. Search for a non-staff patron -> Accounting
8. Click 'Create manual invoice', add an amount and click 'Save and pay' -> 'Confirm'
9. Go to transactions and notice the option to Void payment appears
10. Go back to your staff patron's permission and unselect 'Voiding Payments'
11. Go to your non-staff patron and notice the option to void payments is gone

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 10 Lucas Gass (lukeg) 2024-11-22 18:18:46 UTC
Created attachment 174944 [details] [review]
Bug 17473: (follow-up) add semicolon after use Modern::Perl

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 11 Katrin Fischer 2024-12-23 12:09:19 UTC
1) Capitalization

+   (10, 'void_payment', 'Voiding Payments'),

+        $dbh->do(q{
+            INSERT IGNORE permissions (module_bit, code, description) VALUES
+            (10, 'void_payment', 'Voiding Payments')

Etc.


For Koha we have decided to only upper-case names and the first word. Please check coding guidelines for more information.

2) This is missing a server side check.

I believe we also need to check for this permission server side and maybe in the API. 

I would be OK to handle the API on a separate bug if there is no veto, but would like to see a re-worked patch with fixed capitalization and the server side check looked into.
Comment 12 Katrin Fischer 2024-12-23 12:11:03 UTC
3) Database update

Oh! While you are on it, please update the atomicupate to use the recent template. The success message should be after both SQL statements or they each should have one. They should use say_success.
Comment 13 Katrin Fischer 2024-12-23 12:56:18 UTC
Added a note to the notes fields as I had accidentally pushed this despite setting to failed QA. Reverted for now, if we can't fix before 25.05 (which I don't hope) it will serve as a reminder to fix release notes.