Bug 24339 - SIP codes are missing from the default payment_types on installation
Summary: SIP codes are missing from the default payment_types on installation
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 22610
Blocks: 24338 24525
  Show dependency treegraph
 
Reported: 2020-01-03 16:58 UTC by Martin Renvoize
Modified: 2020-05-14 22:00 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06


Attachments
Bug 24339: Add SIP2 payment types for installation (1.30 KB, patch)
2020-01-08 11:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24339: Remove SIP payment types from staff pay screens (3.71 KB, patch)
2020-01-08 12:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24339: Add SIP2 payment types for installation (1.30 KB, patch)
2020-01-21 16:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24339: Remove SIP payment types from staff pay screens (3.71 KB, patch)
2020-01-21 16:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24339: Do not assume there are exactly 3 SIP* AVs (2.95 KB, patch)
2020-01-27 10:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24339: Do not assume there are exactly 3 SIP* AVs (3.01 KB, patch)
2020-01-27 10:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24339: Add SIP2 payment types for installation (1.37 KB, patch)
2020-01-27 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24339: Remove SIP payment types from staff pay screens (3.80 KB, patch)
2020-01-27 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24339: Do not assume there are exactly 3 SIP* AVs (3.02 KB, patch)
2020-01-27 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
[19.11.x] Bug 24339: Add SIP2 payment types for installation (1.55 KB, patch)
2020-05-08 20:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[19.11.x] Bug 24339: Remove SIP payment types from staff pay screens (2.34 KB, patch)
2020-05-08 20:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[19.11.x] Bug 24339: Do not assume there are exactly 3 SIP* AVs (3.08 KB, patch)
2020-05-08 20:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2020-01-03 16:58:58 UTC
Bug 22610 moved the sip payment types out of accounttypes and into the payment_types authorized values.. however, it appears during rebases that the installation side of this was lost and only existing systems will populate the payment_type defaults on upgrade.
Comment 1 Martin Renvoize 2020-01-08 11:51:46 UTC
Created attachment 96976 [details] [review]
Bug 24339: Add SIP2 payment types for installation
Comment 2 Martin Renvoize 2020-01-08 12:49:58 UTC
Created attachment 96988 [details] [review]
Bug 24339: Remove SIP payment types from staff pay screens
Comment 3 Martin Renvoize 2020-01-08 12:52:14 UTC
Test plan

1) Perform a fresh install and note that the SIP code payments types are populated in the authorised_values table.
2) Make a payment via the staff client and note that the payment_type dropdown still does not appear when only the SIP types are defined.
3) Define some additional payment types and make a payment again.. Note that the new payment types are displayed in the dropdown but the SIP one's are not.
Comment 4 Katrin Fischer 2020-01-15 23:01:06 UTC
Hi Martin, can yo help me with the test plan? Where do we define the SIP payment types?

Also... this is failing QA tests rather spectacularly:

FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
   OK	  filters
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  js_in_body
   OK	  spelling
   OK	  tt_valid
   FAIL	  valid_template
		The method Koha::AuthorisedValue->size is not covered by tests!

Trace begun at /home/vagrant/kohaclone/Koha/Object.pm line 670
Koha::Object::AUTOLOAD('Koha::AuthorisedValue=HASH(0x56078bb1a4c0)') called at /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt line 273
eval {...} at /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt line 273
eval {...} at /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt line 7
Template::Document::__ANON__('Template::Context=HASH(0x56078c113160)') called at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Document.pm line 163
eval {...} at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Document.pm line 161
Template::Document::process('Template::Document=HASH(0x56078bb2c0e0)', 'Template::Context=HASH(0x56078c113160)') called at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Context.pm line 351
eval {...} at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Context.pm line 321
Template::Context::process('Template::Context=HASH(0x56078c113160)', 'Template::Document=HASH(0x56078bb2c0e0)') called at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Service.pm line 94
eval {...} at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Service.pm line 91
Template::Service::process('Template::Service=HASH(0x56078bfa9478)', '/home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt', 'HASH(0x56078c03e178)') called at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template.pm line 66
Template::process('Template=HASH(0x56078bf1d2f0)', '/home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt', 'HASH(0x56078c03e178)', 'SCALAR(0x560783ebf940)') called at /home/vagrant/qa-test-tools/QohA/File/Template.pm line 94
QohA::File::Template::check_valid_template('QohA::File::Template=HASH(0x560781181158)') called at /home/vagrant/qa-test-tools/QohA/File/Template.pm line 23
QohA::File::Template::run_checks('QohA::File::Template=HASH(0x560781181158)', 2) called at /home/vagrant/qa-test-tools/koha-qa.pl line 99
eval {...} at /home/vagrant/qa-test-tools/koha-qa.pl line 67
Comment 5 Martin Renvoize 2020-01-21 16:10:08 UTC
Regarding test plan, the Payments types are just Authorized Values as they have been for years.. this patch just ensures they are present for installs (currently they're just populated during upgrades).

Hmm, I can't replicate your test failure either :(
Comment 6 Martin Renvoize 2020-01-21 16:10:44 UTC
Created attachment 97676 [details] [review]
Bug 24339: Add SIP2 payment types for installation
Comment 7 Martin Renvoize 2020-01-21 16:10:47 UTC
Created attachment 97677 [details] [review]
Bug 24339: Remove SIP payment types from staff pay screens
Comment 8 Jonathan Druart 2020-01-22 09:55:45 UTC
[% SET excluded = ['SIP00', 'SIP01', 'SIP02'] %]
[% IF payment_types > 3 %]

I do not think it's regression free (maybe SIP* have been removed from the AV?)

I would say we should test if payment_types - excluded > 0
Comment 9 Martin Renvoize 2020-01-22 11:55:51 UTC
If we set it to >0 it'll be a regression in so much as we will now always show the 'Payment type' dropdown even if only system types are defined.

I'm not sure how we can reduce the payment_types list to not include the SIP types in the first place in those templates.. that would be the better solution.

This is really only meant as a stop gap solution.. I hope to merge the payment_types authorized value into the debit_types table and expose more clearly defined options relating to the types in bug 23894
Comment 10 Jonathan Druart 2020-01-27 10:07:18 UTC
Created attachment 97962 [details] [review]
Bug 24339: Do not assume there are exactly 3 SIP* AVs

Update DB entry 19.06.00.017:
We created SIP* AV from accountlines if '^Pay[[:digit:]]{2}$', so I
think we should assume that more than SIP00, SIP01, SIP02 can exist.
Comment 11 Jonathan Druart 2020-01-27 10:08:19 UTC
(In reply to Martin Renvoize from comment #9)
> If we set it to >0 it'll be a regression in so much as we will now always
> show the 'Payment type' dropdown even if only system types are defined.

I do not understand what you meant here.
I suggested "payment_types - excluded > 0"
What about this follow-up?
Comment 12 Martin Renvoize 2020-01-27 10:23:11 UTC
Misread your comment
Comment 13 Martin Renvoize 2020-01-27 10:26:31 UTC
OK, I like your followup, it's much nicer than my original attempt. Thanks!
Comment 14 Martin Renvoize 2020-01-27 10:31:07 UTC
Created attachment 97963 [details] [review]
Bug 24339: Do not assume there are exactly 3 SIP* AVs

Update DB entry 19.06.00.017:
We created SIP* AV from accountlines if '^Pay[[:digit:]]{2}$', so I
think we should assume that more than SIP00, SIP01, SIP02 can exist.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Jonathan Druart 2020-01-27 10:35:39 UTC
Created attachment 97965 [details] [review]
Bug 24339: Add SIP2 payment types for installation

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Jonathan Druart 2020-01-27 10:35:42 UTC
Created attachment 97966 [details] [review]
Bug 24339: Remove SIP payment types from staff pay screens

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Jonathan Druart 2020-01-27 10:35:46 UTC
Created attachment 97967 [details] [review]
Bug 24339: Do not assume there are exactly 3 SIP* AVs

Update DB entry 19.06.00.017:
We created SIP* AV from accountlines if '^Pay[[:digit:]]{2}$', so I
think we should assume that more than SIP00, SIP01, SIP02 can exist.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Martin Renvoize 2020-01-27 10:42:13 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 19 Jonathan Druart 2020-02-05 14:04:26 UTC
Could we unify the code in boraccount please?

Also it seems than we will need to move this code in an include file, to avoid seeing it c/p all over the members template modules...
Comment 20 Joy Nelson 2020-04-03 19:38:54 UTC
not backported to 19.11.x
Comment 21 Tomás Cohen Arazi 2020-05-08 20:52:25 UTC
Created attachment 104619 [details] [review]
[19.11.x] Bug 24339: Add SIP2 payment types for installation

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 22 Tomás Cohen Arazi 2020-05-08 20:52:32 UTC
Created attachment 104620 [details] [review]
[19.11.x] Bug 24339: Remove SIP payment types from staff pay screens

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 23 Tomás Cohen Arazi 2020-05-08 20:52:42 UTC
Created attachment 104621 [details] [review]
[19.11.x] Bug 24339: Do not assume there are exactly 3 SIP* AVs

Update DB entry 19.06.00.017:
We created SIP* AV from accountlines if '^Pay[[:digit:]]{2}$', so I
think we should assume that more than SIP00, SIP01, SIP02 can exist.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 24 Joy Nelson 2020-05-08 21:48:59 UTC
backported to 19.11.x for 19.11.06
Comment 25 Lucas Gass 2020-05-14 22:00:00 UTC
missing dependencies for 19.05.x, no backport