[Koha-bugs] [Bug 17674] Allow UI to delete serials issues in batch

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

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Failed QA

--- Comment #103 from Jonathan Druart <jonathan.druart at 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.