https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22569
--- Comment #46 from Jonathan Druart <[email protected]> --- Adding here some notes about the dependent tree (I've squashed the whole tree for pre-review): 1. - $messages->{'WasTransfered'} = 1; + $messages->{'WasTransfered'} = $tbr; but t/db_dependent/SIP/Transaction.t: is_deeply($result,{ messages => { 'NotIssued' => $item->barcode, 'WasTransfered' => 1 } },"Messages show not issued and transferred"); So I ran the tests and there is a failure: # Failed test 'Messages show not issued and transferred' # at t/db_dependent/SIP/Transaction.t line 327. # Structures begin differing at: # $got->{messages}{TransferTrigger} = 'ReturnToHome' # $expected->{messages}{TransferTrigger} = Does not exist # Looks like you planned 5 tests but ran 2. 2. - my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->itemnumber ); + my $transfer = $item->get_transfer; This GetTransfers is pretty bad, it could return several transfers, but callers are not ready for that: opac/opac-detail.pl: my ( $transfertwhen, $transfertfrom, $transfertto ) = GetTransfers($itm->{itemnumber}); So that's definitely a good move to have a get_transfer method that will return only 1, the current one. However, cannot we enforce this constraint at DB level (DB unique key) and have a ->find call in ->get_transfer to replace the ->first? 3. Shouldn't Koha::Exceptions::Item::Transfer::Found be actually Koha::Exceptions::Item::Transfer::AlreadyInTransfer, to be more explicit? 4. Same, should we make Koha::Exceptions::Item::Transfer::Limit more explicit? Its description is "Transfer not allowed" but it seems that we can make it more exact. 5. in circ/transferstosend.pl + show_date => output_pref( + { dt => dt_from_string, dateformat => 'iso', dateonly => 1 } + ) I'd pass {today => dt_from_string}, it should be enough. 6. in circ/transferstosend.tt [% FOREACH branchesloo IN branchesloop %] should be [% FOREACH library IN libraries %] 7. There are some indentation inconsistencies in circ/transferstosend.tt 8. (not blocker) transferCollection.tt + [%- SWITCH message.type -%] + [%- CASE 'failure' %] ... + [%- CASE 'enqueu' -%] The usual pattern is push @message, { type => 'error', # or message code => 'enqueu(ed?)', %more_variables } -- 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/
