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.
Created attachment 96976 [details] [review] Bug 24339: Add SIP2 payment types for installation
Created attachment 96988 [details] [review] Bug 24339: Remove SIP payment types from staff pay screens
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.
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
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 :(
Created attachment 97676 [details] [review] Bug 24339: Add SIP2 payment types for installation
Created attachment 97677 [details] [review] Bug 24339: Remove SIP payment types from staff pay screens
[% 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
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
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.
(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?
Misread your comment
OK, I like your followup, it's much nicer than my original attempt. Thanks!
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>
Created attachment 97965 [details] [review] Bug 24339: Add SIP2 payment types for installation Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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>
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>
Nice work everyone! Pushed to master for 20.05
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...
not backported to 19.11.x
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>
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>
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>
backported to 19.11.x for 19.11.06
missing dependencies for 19.05.x, no backport