[Koha-bugs] [Bug 17501] Koha Objects for uploaded files

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

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

Jonathan Druart <jonathan.druart at bugs.koha-community.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jonathan.druart at bugs.koha-c
                   |                            |ommunity.org

--- Comment #21 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
Code review:
1/ Koha::UploadedFile
a. Koha::UploadedFile->delete is weird, I'd suggest to call and return
$self->SUPER::delete in any cases and warn only `unless -e $file`.
As Koha::Object->delete returns 1 on success, I don't think you should return
anything else.
b. getCategories
 - use C4::Koha; is missing
 - The methods in itself should be moved to Koha::UploadedFiles.
I'd even suggest to remove it, and call GetAuthorisedValues only once in
tools/upload.pl. It does not make sense to call it 3 times in the same script.
2/ Koha::UploadedFiles
a. ->delete should not be called after ->new, but directly
(Koha::UploadedFiles->delete).
I am wondering if what you are doing in this method should not be moved to
Koha::Objects->delete because actually it is the expected behaviour I think.
3/ in tools/upload.pl, it's certainly better to use ->find($id) and then check
if the public method.

-- 
You are receiving this mail because:
You are watching all bug changes.