https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23049
--- Comment #199 from Marcel de Rooy <[email protected]> --- Tested on top of commit 480434bbf4a32750c5e47a3600b6a386d9732296 with your branch. Thanks for your hard work. I have noted comments 64 and 85 (thx Kyle). I made my preference clear, and continued on the current stand. Comments below are in order of time noted, not priority. User experience: During startup of admin/debit_types.pl I am seeing the 15 records for a short moment and end up with 3 records (filtered). It would be nice to see them all and only allow editing of the non-system types ? Same when saving a record (no blocker, but does not look good). When you do not want to show the records, do not fetch them at all? I saw that I now have a debit type F (Unexpected type found during upgrade). Obviously this had to do with old fines stuff. I understand that bug 22521 should have dealt with those, but somehow I still had such a record in accountlines. I am wondering if this will happen to more people and if we should anticipate on that by giving this F another description than unexpected ;) $dbh->do( qq{ INSERT IGNORE INTO account_debit_types ( code, description, can_be_added_manually, default_amount, is_system ) SELECT SUBSTR(accounttype, 1, 80), "Unexpected type found during upgrade", 1, NULL, 0 FROM accountlines WHERE amount > 0 } ); No need for the SUBSTR anymore. Please add a DISTINCT accounttype to this query to eliminate a lof of ignored inserts. And a question: If this type is unexpected, why do we enable Add manually ? sub UpdateFine You touch the following line, obviously only changing the type codes. But this is a condition for finding the overdues. Why type M in the first place, and why not yet another debit code? debit_type_code => [ 'OVERDUE', 'M' ], This needs fixing somehow but I agree that it should be done on its own report.. sub GetFine + WHERE debit_type_code LIKE 'OVERDUE' Shouldnt that be = ? flexability [a.o. :) ] No, not so flexible. sub adjust - my $account_type = $self->accounttype; [..] + my $debit_type_code = $self->debit_type_code; As I understand, the only allowed code is now overdue so debit. But this change does not look very solid to me. Hopefully, we will not adjust too much ;) Copier Fees Database: | Copier Fees | Copier Fees | 1 | 0.250000 | 0 | -INSERT INTO `authorised_values` (category, authorised_value, lib) VALUES ('MANUAL_INV','Copier Fees','.25'); git grep 'Copier Fees' installer/data/mysql/es-ES/optional/auth_val.sql:INSERT INTO `authorised_values` (category, authorised_value, lib) VALUES ('MANUAL_INV','Copier Fees','.25'); Oops, you should remove that one above. Since we previously installed Copier Fees as a manual invoice, I think you should do now too. So add it too account_debit_types.sql. And Copier Fees should also have a new code. Now the description and code are the same. It is inconsistent with the other debit types. You also changed lots of other codes, or not? Does this hold for some other debit types too that were formerly hardcoded? Stuff like sundry etc. ? Which actually is a horrible category.. + [%- SWITCH account.debit_type_code -%] + [%- CASE 'ACCOUNT' -%]Account creation fee + [%- CASE 'ACCOUNT_RENEW' -%]Account renewal fee + [%- CASE 'LOST' -%]Lost item + [%- CASE 'M' -%]Sundry + [%- CASE 'NEW_CARD' -%]New card + [%- CASE 'OVERDUE' -%]Fine + [%- CASE 'PROCESSING' -%]Lost item processing fee + [%- CASE 'RENT' -%]Rental fee + [%- CASE 'RENT_DAILY' -%]Daily rental fee + [%- CASE 'RENT_RENEW' -%]Renewal of rental item + [%- CASE 'RENT_DAILY_RENEW' -%]Rewewal of daily rental item + [%- CASE 'RESERVE' -%]Hold fee + [%- CASE 'RESERVE_EXPIRED' -%]Hold waiting too long + [%- CASE -%][% account.debit_type.description | html %] + [%- END -%] Note that koha-tmpl/intranet-tmpl/prog/en/includes/accounts.inc contains debit code M for sundry while account_debit_types.sql install M as Manual fee ! This is a BLOCKER. Please correct. At the credit side, I am seeing code FORW (and FOR and W and WO !) and having doubts about it, leaving it outside the scope for now.. Submitting another comment for the failing tests. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
