https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17674
Jonathan Druart <jonathan.dru...@bugs.koha-community.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|Signed Off |Failed QA
--- Comment #103 from Jonathan Druart <jonathan.dru...@bugs.koha-community.org>
---
QA comments (reading the code):
1. I do not understand Koha::Serials::get_serial_items_count, several things
are wrong:
* you should not export, it must be a method and called like others:
Koha::Module->method
* my ( @self ) = @_;
No, $self must be the object, then you pass the parameters.
*
+ while ( my $s = $serialitems->next() ) {
+ $countitem++;
+ }
=> You actually want ->count
* And finally, what is it supposed to do? and why?
`git grep get_serial_items_count` returns only one occurrence:
serials/serials-collection.pl:my
@serialitemsinformation=get_serial_items_count(@ids);
It returns an array of hashes (2 keys: countitems and serialid), countitems is
used later, and serialid is not.
Moreover I have no idea what the following is supposed to do
130 foreach my $line (@serialitemsinformation){
131 DelItem($line);
132 }
It certainly does not work.
I would suggest you to rethink this as a method, for instance I am pretty sure
you are looking for something like that: Koha::Serial->items which would return
the items (Koha::Items!) for a given Koha::Serial object (already said, less
explicitly, on comment 32)
2. You do not need to concat with '!' then split, you can pass several times a
parameters (and retrieve them pl side with CGI->multi_param). It will simplify
a lot the javascript code (function deleteIssues).
3. $delete and $confdelete should not be variables, but values of $op (usually
"delete_confirm" and "delete_confirmed", see admin/cities.pl)
4. function generateReceive (serials-collection.tt) re-added, has been deleted
previously by bug 18327.
5. Same for addsubscriptionid, delete by bug 19777.
IMO it would be easier to redo these patches from scratch and abandon the too
long patches history.
--
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/
