[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #24 from Jonathan Druart --- anonymise_issue_history changes need to be adjusted, the method has been removed. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Jonathan Druart changed: What|Removed |Added Status|Failed QA |Patch doesn't apply --- Comment #23 from Jonathan Druart --- Can we move forward here? The patches do not longer apply. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Lucas Gass changed: What|Removed |Added Keywords|rel_22_05_candidate | -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Lucas Gass changed: What|Removed |Added Keywords||rel_22_05_candidate -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Lucas Gass changed: What|Removed |Added CC||lu...@bywatersolutions.com Keywords|rel_22_05_candidate | -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Jonathan Druart changed: What|Removed |Added Keywords|rel_21_11_candidate,|rel_22_05_candidate |RM_priority | -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Jonathan Druart changed: What|Removed |Added Keywords|rel_21_05_candidate |rel_21_11_candidate -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #22 from Nick Clemens --- Also QA failure: sort (...) interpreted as function sort (...) interpreted as function -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #21 from Martin Renvoize --- Thanks Nick, I've dug out my branch and it looks like I started added more tests for chaining and combination of these filters.. I'll look at working through completing those tests and take into account your comments.. I like the cleaner JOIN you suggest.. though we may need to start naming our joins to maintain chain-ability.. I can imagine issues getting joined multiple times and then us ending up with queries clashing. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Nick Clemens changed: What|Removed |Added Status|Signed Off |Failed QA --- Comment #20 from Nick Clemens --- Hi Martin, Sorry, I should have read the proposed language closer - pending_checkouts is confusing, I think it can just be 'checkouts' filter_by_has_checkouts I wonder about the methods for filtering Why do you search issues for borrowernumber, then use an in? Would it be simpler to use a single query? I think it is clearer to do something like: $self->search("issues.borrowernumber" => { -not => undef } },{join=>"issues"}); Later for housebound_choosers we do a search related to housebound_roles, then another to borrowernumber - this works, but it comes out as "FROM borrowers JOIN housebound_role JOIN borrowers" -Nick -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Depends on|20271 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20271 [Bug 20271] Merge deleted biblio, biblioitems, biblio_metadata, and items tables -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #19 from Martin Renvoize --- Created attachment 115041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115041&action=edit Bug 11983: (follow-up) Terminology fixes As discussed, this patch updates the class (and tests) to reflect the agreed upon terms -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Attachment #114735|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #18 from Martin Renvoize --- Created attachment 115040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115040&action=edit Bug 11983: Centralised Koha::Patrons filtering methods This patch adds various filter_by_* methods to Koha::Patrons to aid in complex filtering situations. == Test plan == prove t/db_dependent/Koha/Patrons.t Signed-off-by: Victor Grousset/tuxayo -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #17 from Nick Clemens --- (In reply to Victor Grousset/tuxayo from comment #16) > Created attachment 114735 [details] [review] > Bug 11983: Centralised Koha::Patrons method OPTION 2 > > == Test plan == > prove t/db_dependent/Koha/Patrons.t > > Signed-off-by: Victor Grousset/tuxayo Couple questions: 1 - It looks like the wording from commet 9 was not done? has->have issues->checkouts 2 - filter_by_in_lists/filter_by_not_in_lists - for guarantors/guarantees the in/not-in option is a parameter, couldn't we do the same here? 3 - Where did the list for which functions we are defining come from? search_patrons_to_update_category could use some of these, but also needs a 'filter_by_age' or similar 4 - Is there a second bug for using these new methods? An example one would be nice -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Attachment #76227|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Victor Grousset/tuxayo changed: What|Removed |Added Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Victor Grousset/tuxayo changed: What|Removed |Added Attachment #114731|0 |1 is obsolete|| --- Comment #16 from Victor Grousset/tuxayo --- Created attachment 114735 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114735&action=edit Bug 11983: Centralised Koha::Patrons method OPTION 2 == Test plan == prove t/db_dependent/Koha/Patrons.t Signed-off-by: Victor Grousset/tuxayo -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Status|In Discussion |Needs Signoff --- Comment #15 from Martin Renvoize --- Rebased OPTION 2 as it seemed to be the one we landed on.. I can't for the life of me get rid of that last HAVING in the amountsoutstanding.. I think this is ready for review again at this point.. running the included tests should be enough for SO -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Attachment #114730|0 |1 is obsolete|| --- Comment #14 from Martin Renvoize --- Created attachment 114731 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114731&action=edit Bug 11983: Centralised Koha::Patrons method OPTION 2 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Attachment #76701|0 |1 is obsolete|| --- Comment #13 from Martin Renvoize --- Created attachment 114730 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114730&action=edit Bug 11983: Centralised Koha::Patrons method OPTION 2 https://bugs.koha-community.org/show_bug.cgi?id=11982 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Jonathan Druart changed: What|Removed |Added Keywords|rel_20_11_target|rel_21_05_candidate -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Jonathan Druart changed: What|Removed |Added Blocks|20469 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20469 [Bug 20469] Add item status to staff article requests form -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Keywords|rel_20_05_candidate |rel_20_11_target -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Andrew Isherwood changed: What|Removed |Added CC|andrew.isherwood@ptfs-europ | |e.com | -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Keywords||rel_20_05_target, ||RM_priority -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Jonathan Druart changed: What|Removed |Added Summary|Code to select patrons to |Code to select patrons to |purge needs to be |purge needs to be |centralized |centralized (use of ||filter_by) -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Jonathan Druart changed: What|Removed |Added Blocks||16846 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16846 [Bug 16846] Move patron related code to Koha::Patron -- 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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Tomás Cohen Arazi changed: What|Removed |Added CC||tomasco...@gmail.com -- 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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Marcel de Rooy changed: What|Removed |Added Blocks||20469 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20469 [Bug 20469] Add item status to staff article requests form -- 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/
[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Katrin Fischer changed: What|Removed |Added Summary|code to select patrons to |Code to select patrons to |purge needs to be |purge needs to be |centralized |centralized -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 David Cook changed: What|Removed |Added CC||dc...@prosentient.com.au --- Comment #12 from David Cook --- Sounds good to me. I'm all for having method naming conventions. With Perl's "method names as strings" (https://perldoc.perl.org/perlobj.html#Method-Call-Variations), I think you can get the flexibility of Option 1 while using the code in Option 2. my $method = "filter_by_$arg"; my $filtered = $obj->$method(); Boom! And yeah by standardizing method naming conventions, I think we could potentially see some code re-use for methods that are more complex than your average DIBC methods but less complex than super unique methods. You can do some cool stuff with aliasing functions so that you can have different module-specific names for the same generic method as well, so you might get that relevant API while still having code re-use. I see nothing to disagree with (which must be novel considering it's me). Awesome ideas, Martin! -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Marcel de Rooy changed: What|Removed |Added CC||m.de.r...@rijksmuseum.nl --- Comment #11 from Marcel de Rooy --- Hmm. At first glance option 1 shows less code lines and more flexibility with parameters. It might be easier to switch one parameter than selecting another method each time ? But no strong opinion here, and not studied it in detail.. -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #10 from Martin Renvoize --- Thanks for the review, Jonathan :) I far prefer option 2 too now I've written it. I think splitting out the final combinations for script + tool interfaces into their own subclasses also makes sense. Regarding your remarks: - Interesting regarding the join.. perhaps my brain is failing me but I couldn't see how I could do it without the having (whilst maintaining chainability). Happy to be guided here.. I might ask ribasushi for his thoughts there :) - Coming up with method names was almost the hardest bit of this and I'm more than happy to be advised here.. What I would like to see is a fairly standardised recommendation for method names of this type as a guideline. We came up with 'filter_by_' to denote that you are filtering the resultset down. Following that by the 'how' and 'what' makes sense. It's describing the 'how' and 'what' which is causing fun here.. - `last_issued` could be more clearly written as `when_last_issued` perhaps? - oh, did I get the standardisation wrong.. I thought we'd gone from 'checkout' to 'issue', my bad.. that should also be corrected in the above one too. - I'm fine with has_pending_checkouts, I can't think of anything better or more descriptive. I think it is used in cleanborrowers (only it's hidden inside other filters) - 'has' vs 'have'.. hmm, we're in Koha::Patrons so 'have' is right.. thanks - I actually made this one flexible at someone's request.. though I can't remember who.. perhaps it would be better as distinct methods.. more than happy to get some discussion going here. I shall try to raise all this at the next dev meeting.. see if we can bring some more people into the conversation. -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Jonathan Druart changed: What|Removed |Added Status|NEW |In Discussion --- Comment #9 from Jonathan Druart --- I prefer the second approach even if more verbose. It's easier to read and to test. It will certainly be easier to maintain and less error prone. Few remarks: - The having could be replaced with a join, and so more efficient. - filter_by_last_issued sounds wrong. I agree we should have a pattern (here "filter_by_") to explicitly tell what we are doing, but it should also be meaningful. Maybe it is for native English speakers ;) - filter_by_has_issues($options) is not enough explicit IMO. Moreover $options is a boolean, and "issue" must be "checkout". I would suggest: $patrons->filter_by_has_pending_checkouts and another one (?) But maybe I am making things more complicated here. (note: it seems that this subroutine will not be used by cleanborrowers.pl, we do not have this use case) - Should not filter_by_has_issues be filter_by_have_issues? - filter_by_when_expired - the interface allows to filter by "expired before", now we provide a "and/or expire after", but it is not used. It means more code, more tests and so... more potential bugs. It is not a big deal for this one, but we could have filter_by_expired_before that will be easier to read/write/test. Not blocker but worth mentioning/discussing it I think. -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #8 from Martin Renvoize --- So the core proposal here is that we stop adding `->search_for_functionality_x` methods to the DB level Koha::Objects and rather add generalized filters for complex joins etc if we need to. If we really want to add a `for_functionality_x` method/routine to centralize code for say patron manipulations (where we may have a UI and Script driven element) we should break that out into a subclass of some kind that contains the functions/methods for just that area of functionality. Koha::Patron::BulkActions perhaps as an example.. containing `->bulk_selection(filter_a, filter_b)` which internally just calls either a large constructed query (like the code in option 1) or a chained set of queries (from option 2) and then various available `->bulk_actions` that may be applied to the selected set. `->bulk_delete`, `bulk_anonymise`, `bulk_modify` for example? Thoughts? -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #7 from Martin Renvoize --- The option 2 patch now includes the beginnings of tests (I still need to test method chaining). Would appreciate any further feedback on this option.. method names, how they're constructed, whether it's a reasonable idea and should form the basis of a coding guideline to improve the consistency of Koha::Objects internal apis etc. -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Attachment #76228|0 |1 is obsolete|| --- Comment #6 from Martin Renvoize --- Created attachment 76701 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76701&action=edit Bug 11983: Centralised Koha::Patrons method OPTION 2 -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #5 from Jonathan Druart --- IMO Option 1 will be hard to maintain, and is not very readable. Option 2 is more verbose but clearer. See also Koha/Virtualshelves.pm for existing examples. -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added CC||colin.campbell@ptfs-europe. ||com -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added CC||andrew.isherwood@ptfs-europ ||e.com -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added CC||katrin.fisc...@bsz-bw.de -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added CC||n...@bywatersolutions.com -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added CC||jonathan.dru...@bugs.koha-c ||ommunity.org -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #4 from Martin Renvoize --- Feedback wanted on just the Koha::Patrons method options here before I write the associated tests and hook the code up to the scripts and tools. -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #3 from Martin Renvoize --- Created attachment 76228 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76228&action=edit Bug 11983: Centralised Koha::Patrons method OPTION 2 -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #2 from Martin Renvoize --- Created attachment 76227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76227&action=edit Bug 11983: Centralised Koha::Patrons method OPTION 1 -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 --- Comment #1 from Martin Renvoize --- I intend to work on this but won't do so until after 20271 is in as it will drastically affect the code. -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Depends on||20271 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20271 [Bug 20271] Merge deleted* tables with their "alive" cousins -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Martin Renvoize changed: What|Removed |Added Assignee|gmcha...@gmail.com |martin.renvoize@ptfs-europe ||.com CC||martin.renvoize@ptfs-europe ||.com -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Victor Grousset/tuxayo changed: What|Removed |Added CC||victor.grous...@biblibre.co ||m -- 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/
[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983 Galen Charlton changed: What|Removed |Added Depends on||11352 -- 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/