It would be nice if we could filter long overdues based on itemtype the same way we can already filter based on borrower category code.
Created attachment 73026 [details] [review] Bug 20436 - Add ability to specify itemtypes for longoverdue.pl It would be nice if we could filter long overdues based on itemtype the same way we can already filter based on borrower category code. Test Plan: 1) Set up a number of overdues of various itemtypes 2) Run longoverdue.pl with --list-itemtypes to show itemtypes 3) Run longoverdue.pl with --itemtype to verify it only operates on that itemtype 4) Run longoverdue.pl with multiple --itemtype switches, verify it only operates on those itemtypes 5) Run longoverdue.pl with --skip-itemtype to verify it does not operate on that itemtype 6) Run longoverdue.pl with multiple --skip-itemtype switches, verify that it does not operate on those itemtypes
Created attachment 80984 [details] [review] Works like a charm! Thx Kyle Bug 20436 - Add ability to specify itemtypes for longoverdue.pl It would be nice if we could filter long overdues based on itemtype the same way we can already filter based on borrower category code. Test Plan: 1) Set up a number of overdues of various itemtypes 2) Run longoverdue.pl with --list-itemtypes to show itemtypes 3) Run longoverdue.pl with --itemtype to verify it only operates on that itemtype 4) Run longoverdue.pl with multiple --itemtype switches, verify it only operates on those itemtypes 5) Run longoverdue.pl with --skip-itemtype to verify it does not operate on that itemtype 6) Run longoverdue.pl with multiple --skip-itemtype switches, verify that it does not operate on those itemtypes Signed-off-by: Jesse Maseto <jesse@bywatersolution.com>
Created attachment 81024 [details] [review] Bug 20436 - Add ability to specify itemtypes for longoverdue.pl It would be nice if we could filter long overdues based on itemtype the same way we can already filter based on borrower category code. Test Plan: 1) Set up a number of overdues of various itemtypes 2) Run longoverdue.pl with --list-itemtypes to show itemtypes 3) Run longoverdue.pl with --itemtype to verify it only operates on that itemtype 4) Run longoverdue.pl with multiple --itemtype switches, verify it only operates on those itemtypes 5) Run longoverdue.pl with --skip-itemtype to verify it does not operate on that itemtype 6) Run longoverdue.pl with multiple --skip-itemtype switches, verify that it does not operate on those itemtypes Signed-off-by: Jesse Maseto <jesse@bywatersolution.com>
* Commit title does not start with 'Bug XXXXX: ' - 62f2b5d QA: Looking here
+ 'l|lost=s%' => \$lost, 'category=s' => $borrower_category, 'skip-category=s' => $skip_borrower_category, 'list-categories' => \$list_categories, + 'itemtype=s' => $itemtype, + 'skip-itemtype=s' => $skip_itemtype, + 'list-itemtypes' => \$list_itemtypes, This is a confusing mix of handling options. I think we should always pass a reference like name => \$option, but as you can see we already deviate from that here. But you are adding new cases.
Typo "The options --itemtype and --skip-itemtype are mually exclusive.\n" But I am not a native speaker, so I might be wrong?
+=item B<--list-itemtypes> + +List itemtypes available for use by B<--itemtype> or +B<--skip-itemtype>, and exit. I am not a fan of such options. There are better ways to list them. One is already in and I would rather remove it.
Changing status. Too much noise for me now
Just noting that this still applies and only needs attention to QA concerns.
Created attachment 86983 [details] [review] Bug 20436: Add ability to specify itemtypes for longoverdue.pl It would be nice if we could filter long overdues based on itemtype the same way we can already filter based on borrower category code. Test Plan: 1) Set up a number of overdues of various itemtypes 2) Run longoverdue.pl with --list-itemtypes to show itemtypes 3) Run longoverdue.pl with --itemtype to verify it only operates on that itemtype 4) Run longoverdue.pl with multiple --itemtype switches, verify it only operates on those itemtypes 5) Run longoverdue.pl with --skip-itemtype to verify it does not operate on that itemtype 6) Run longoverdue.pl with multiple --skip-itemtype switches, verify that it does not operate on those itemtypes Signed-off-by: Jesse Maseto <jesse@bywatersolution.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 86984 [details] [review] Bug 20436: (QA follow up) - reference and typo fixes
Fixed commit headlines on original patch, and addressed the reference and typo concerns. I leave the last one "+=item B<--list-itemtypes> + +List itemtypes available for use by B<--itemtype> or +B<--skip-itemtype>, and exit. I am not a fan of such options. There are better ways to list them. One is already in and I would rather remove it." for Kyle. Liz
Created attachment 89304 [details] [review] Bug 20436: Add ability to specify itemtypes for longoverdue.pl It would be nice if we could filter long overdues based on itemtype the same way we can already filter based on borrower category code. Test Plan: 1) Set up a number of overdues of various itemtypes 2) Run longoverdue.pl with --list-itemtypes to show itemtypes 3) Run longoverdue.pl with --itemtype to verify it only operates on that itemtype 4) Run longoverdue.pl with multiple --itemtype switches, verify it only operates on those itemtypes 5) Run longoverdue.pl with --skip-itemtype to verify it does not operate on that itemtype 6) Run longoverdue.pl with multiple --skip-itemtype switches, verify that it does not operate on those itemtypes Signed-off-by: Jesse Maseto <jesse@bywatersolution.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 89305 [details] [review] Bug 20436: (QA follow up) - reference and typo fixes Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Passing QA but frowning: + my @itemtypes = sort map { uc $_->[0] } @{ C4::Context->dbh->selectall_arrayref(q|SELECT itemtype FROM itemtypes|) }; +my @available_itemtypes = map { uc $_->[0] } @{ $dbh->selectall_arrayref(q|SELECT itemtype FROM itemtypes|) }; Old school SQL.. The earlier mention of --list-itemtypes is no blocker for me. Leaving final decision to RM ;)
(In reply to Marcel de Rooy from comment #15) This one can be done in a follow-up please Kyle? Pushing for now > Passing QA but frowning: > > + my @itemtypes = sort map { uc $_->[0] } @{ > C4::Context->dbh->selectall_arrayref(q|SELECT itemtype FROM itemtypes|) }; > > +my @available_itemtypes = map { uc $_->[0] } @{ > $dbh->selectall_arrayref(q|SELECT itemtype FROM itemtypes|) }; > > Old school SQL.. > This seems okay to me, we should make a rule somewhere if we like this or not > The earlier mention of --list-itemtypes is no blocker for me. > Leaving final decision to RM ;) Awesome work all! Pushed to master for 19.05
Created attachment 89962 [details] [review] Bug 20436: (RM follow-up) Use Koha objects instead of mysql
Created attachment 90040 [details] [review] Bug 20436: (RM follow-up) Use Koha objects instead of mysql Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We could probably remove the ->search()-> part of the call, but at this point I think we should do that first thing next cycle.
Last follow-up pushed to master
Enhancement will not be backported to 18.11.x series