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

--- Comment #13 from Martin Renvoize (ashimema) 
<[email protected]> ---
Comment on attachment 187941
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187941
Bug 41020: Add ability to use file transports for marc ordering accounts

Review of attachment 187941:
 --> 
(https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=41020&attachment=187941)
-----------------------------------------------------------------

Generally looks good, couple of minor queries of why we took a few shortcuts ;)

::: admin/marc_order_accounts.pl
@@ +61,5 @@
>      my @matchers = C4::Matcher::GetMatcherList();
>      $template->param( available_matchers => \@matchers );
>  
> +    # Get available file transports for selection
> +    my @file_transports = $schema->resultset('FileTransport')->search(

Was it intentional to use DBIC directly instead of doing a
Koha::File::Transports->search() call?

::: misc/cronjobs/marc_ordering_process.pl
@@ +111,5 @@
>      }
>  
>      my $working_dir = $acct->download_directory;
> +
> +    my $file_transport = $acct->file_transport_id ? 
> Koha::File::Transports->find( $acct->file_transport_id ) : undef;

Any reason to do this here rather than introducing a 'transport' relation
accessor to give you back a Koha::File::Transport object directly if one
exists?  i.e. why in the script not the object as a relation?

-- 
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/

Reply via email to