https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17501
--- Comment #22 from Marcel de Rooy <[email protected]> --- (In reply to Jonathan Druart from comment #21) > 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. Calling SUPER::delete always now. Leave two warns. Return false if deleting the file fails. > b. getCategories > - use C4::Koha; is missing > - The methods in itself should be moved to Koha::UploadedFiles. Agreed > 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. These three times made sense, but nevertheless moved them up into one call. I want to keep getCategories, since it hides the implementation from the script and can be changed easier (say moving it from auth values to separate table). > 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. Changed new->delete into delete. Removed delete_errors as a consequence for now. I suggest to keep delete here now, but we could open a new report to move it to Koha::Objects. > 3/ in tools/upload.pl, it's certainly better to use ->find($id) and then > check if the public method. Adjusted three calls. All adjustments in the last follow-up. Thanks for reviewing. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] 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/
