https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17501
Jonathan Druart <jonathan.dru...@bugs.koha-community.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jonathan.dru...@bugs.koha-c
| |ommunity.org
--- Comment #21 from Jonathan Druart <jonathan.dru...@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.
_______________________________________________
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://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/
