[Koha-bugs] [Bug 23049] Replace MANUAL_INV authorised value with a dedicated table

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23049

--- Comment #199 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
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.