[Koha-bugs] [Bug 11983] Code to select patrons to purge needs to be centralized (use of filter_by)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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=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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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=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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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=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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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=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)

[bugzilla-daemon]

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=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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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)

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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=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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11983

Galen Charlton gmcha...@gmail.com 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/