Bug 17473 - Permission for voiding payments
Summary: Permission for voiding payments
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Eric Garcia
QA Contact: Lucas Gass (lukeg)
URL:
Keywords:
Depends on: 18790
Blocks: 24849
  Show dependency treegraph
 
Reported: 2016-10-20 08:08 UTC by Josef Moravec
Modified: 2024-12-23 12:56 UTC (History)
8 users (show)

See Also:
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:


Attachments
Bug 17473: Add void payment permission (3.15 KB, patch)
2024-07-17 20:45 UTC, Eric Garcia
Details | Diff | Splinter Review
Bug 17473: Add void payment permission (5.05 KB, patch)
2024-07-22 15:27 UTC, Eric Garcia
Details | Diff | Splinter Review
Bug 17473: (follow-up) add semicolon after use Modern::Perl (770 bytes, patch)
2024-07-22 20:24 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 17473: Add void payment permission (5.10 KB, patch)
2024-07-22 20:56 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 17473: (follow-up) add semicolon after use Modern::Perl (822 bytes, patch)
2024-07-22 20:56 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 17473: Add void payment permission (5.15 KB, patch)
2024-11-22 18:18 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 17473: (follow-up) add semicolon after use Modern::Perl (877 bytes, patch)
2024-11-22 18:18 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 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.