[Koha-bugs] [Bug 11983] code to select patrons to purge needs to be centralized

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

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

--- Comment #10 from Martin Renvoize <martin.renvoize at ptfs-europe.com> ---
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.