https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19036
--- Comment #12 from Martin Renvoize <[email protected]> --- Comment on attachment 99676 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99676 Bug 19036: Add ability to auto generate a number for account credits Review of attachment 99676: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=19036&attachment=99676) ----------------------------------------------------------------- Great work Julian, I really wasn't sure what the criteria were for this yet so I'm really pleased to see you've taken it on. Just a few comments and questions as outlined: ::: Koha/Account/Line.pm @@ +771,5 @@ > +sub store { > + my ($self) = @_; > + > + my $AutoCreditNumber = C4::Context->preference('AutoCreditNumber'); > + if ($AutoCreditNumber && !$self->in_storage && $self->is_credit && > !$self->credit_number) { Hmm, I slightly wonder if it's a good idea to allow passing credit_number and allowing it to override the AudoCreditNumber setting.. to me if it's set then we should throw an Exception if someone is trying to pass in their own? Not a failing case, but wanted to raise as a question. The second question here.. should this apply to ALL credit types, or just a subset of those that actually deal with taking money as apposed to writeoffs and other types of giving credit (like LOST_ITEM_RETURN)? @@ +773,5 @@ > + > + my $AutoCreditNumber = C4::Context->preference('AutoCreditNumber'); > + if ($AutoCreditNumber && !$self->in_storage && $self->is_credit && > !$self->credit_number) { > + my $rs = Koha::Database->new->schema->resultset($self->_type); > + It feels like this whole block should be inside a transaction.. you're doing a lookup followed by a store using calculated data.. what if another db connection adds a payment line at the same time? -- 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/
