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

[Jonathan Druart]

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)