[Koha-devel] Koha namespace architecture - calls for discussions

[Jonathan Druart]

For the record:

2/ is on bug 16966 - Koha::Patrons - Move
GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise

and 3/ is on bug 17089 - Move the star ratings related code to Koha::Ratings

2016-07-21 12:20 GMT+01:00 Jonathan Druart
<jonathan.druart at bugs.koha-community.org>:
> Hi devs (me again),
>
> During the last dev meeting, we discussed about the naming of a new
> module in our Koha namespace. The conclusion was to ask the ML in
> order to get more ideas (as there was not a lot of attendees...).
> I'd like to take this opportunity to bring other architecture problems
> I faced recently (Bug 16846 - Move patron related code to
> Koha::Patron).
>
> 1/ Import stuffs modules
> The initial discussion comes from 2 modules which have been added in 2
> different bug reports about the same thought [1].
> To provide a module to import patron, the author of bug 12598 chose
> Koha::Patrons::Import when I chose Koha::Exporter::Record on bug 14722
> (the first one is about importing patrons, the second one is about
> exporting records (biblios, authorities)).
> We need to uniformise these 2 namespaces to keep consistency.
> The different possibilities are:
>   a. Koha::Patrons::Import
>   b. Koha::Importer::Patrons
>   c. Koha::Importer::Patron
>   d. Koha::Import::Patrons
>   e. Koha::Import::Patron
>
> My opinion is that having Koha::Exporter|Importer::Objects will add
> the ability to have a Koha::Exporter|Importer module to group
> export/import methods, if we want to (Objects means Patrons, Records,
> Lists(Shelves), Reports, etc.)
> But we could imagine a generic Koha::Exporter|Importer modules with
> specific Koha::Object[s?]::Export|Import modules.
>
> 2/ Move C4::Members::GetBorrowersWithIssuesHistoryOlderThan to the
> Koha namespace
> This subroutine is supposed to return the patrons with an issue
> history older than a given date.
> I first chose to move this code to
> Koha::Patrons->search_with_issues_history_older_than
> But looking at the code you will find that the subroutine actually
> returns the borrowernumber with the number of old issues (matching the
> "older than" date).
> IMO it would make sense to return the patrons (Koha::Patron) instead
> of just the borrowernumber, but we will have to join the 2 tables,
> which can lead to a perf issue (ref needed). This idea is enforced by
> the fact that GetBorrowersWhoHaveNeverBorrowed and
> GetBorrowersToExpunge fit perfectly into Koha::Patrons.
> Otherwise we could decide to move the subroutine to a method of
> Koha::OldIssues, to avoid the join. But that sounds weird to me.
>
> 3/ Move C4::Ratings
> I have started to move C4::Ratings to Koha::Rating[s]
> Everything went fine until I decided to move C4::Rating::GetRating. It
> actually calculates the ratings avg for a record and returns a hashref
> with different values, and not a Koha::Rating object.
> I think we need to split up with
>   Koha::Ratings->search({ biblionumber => $biblionumber
> })->calculate_avg (or something similar)
> and
>   Koha::Ratings->search({ biblionumber => $biblionumber,
> borrowernumber => $borrowernumber }); # which will return a
> Koha::Rating object
> In that case, 1 call to C4::Ratings::GetRating will be replaced with 2 calls.
> Does that make sense?
>
> 4/ Move subroutines not directly related to a Koha::Object
> In some cases, the move is not very obvious. For instance we have
> C4::Members::checkcardnumber and C4::Members::get_cardnumber_length,
> they are strongly linked and I think we would like to keep them
> together.
> We could imagine a Koha::Patron::Cardnumber to move
> get_cardnumber_length in, but it does not represent a patron's
> cardnumber, so it could be confusing.
> checkcardnumber could go easily to Koha::Patron->check_new_cardnumber(
> $new_cardnumber )
>
> 5/ patronflags?
> Any ideas what we could do with C4::Members::patronflags? :)
>
> Others will certainly come later!
>
> Cheers,
> Jonathan
>
> [1] http://meetings.koha-community.org/2016/development_irc_meeting_13_july_2016.2016-07-13-14.08.log.html
> from 14:29:48 <cait> #info Topic: Koha::Patrons::Import (bug 12598) vs
> Koha::Exporter::Record (bug 14722)