Bug 8309 - Koha namespace organisation, first implementation
Summary: Koha namespace organisation, first implementation
Status: CLOSED WORKSFORME
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P1 - high enhancement (vote)
Assignee: Paul Poulain
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-25 13:04 UTC by Paul Poulain
Modified: 2017-06-14 22:07 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
DBIx::Class 1st step: the schema (417.09 KB, patch)
2012-06-25 13:04 UTC, Paul Poulain
Details | Diff | Splinter Review
Branches table management (7.16 KB, patch)
2012-06-25 13:05 UTC, Paul Poulain
Details | Diff | Splinter Review
Borrowers table management (6.64 KB, patch)
2012-06-25 13:05 UTC, Paul Poulain
Details | Diff | Splinter Review
Branchrelation package, 1st draft (2.52 KB, patch)
2012-06-25 13:05 UTC, Paul Poulain
Details | Diff | Splinter Review
Categories management (31.12 KB, patch)
2012-06-25 13:05 UTC, Paul Poulain
Details | Diff | Splinter Review
DBIx::Class Branch.pm, first try (2.61 KB, patch)
2012-06-25 13:06 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Poulain 2012-06-25 13:04:18 UTC
During KohaCon12 hackfest, some of us had a discussion about Koha namespace organisation and content. The result was a wiki page: http://wiki.koha-community.org/wiki/Koha_Namespace_RFC

I've investigated this organisation by writing some DBIx::Class & Moose packages. The results are great, a lot of code can be removed, and the code is safer. Note that the patches attached here are just a first draft, not requesting sign-off, just comments !

My feedback of this first try:
 * defining the schema has been easy, thanks to DBIx::Class documentation. Error messages are very clear and easy to understand for a newbie like me !
 * investigating a little bit more, there are many tools/tricks that will make us save a lot of time/lines. Like caching (with DBIx::Class::Cursor::Cached), paging (see http://search.cpan.org/~arodland/DBIx-Class-0.08196/lib/DBIx/Class/Manual/Cookbook.pod#Paged_results, not used in this 1st POC), doing JOIN or finding relations between datas, or returning array of hashrefs (DBIx::Class::ResultClass::HashRefInflator)
 * cleaning categories.pl resulted in a code that is *much* smaller and readable than the previous one (something like 100 lines removed)
Comment 1 Paul Poulain 2012-06-25 13:04:59 UTC
Created attachment 10473 [details] [review]
DBIx::Class 1st step: the schema

This DBIx::Class schema has been generated using
perl -MDBIx::Class::Schema::Loader=make_schema_at,dump_to_dir:.Koha -e 'make_schema_at("temp::Schema", { debug => 1 }, [ "dbi:mysql:dbname=Koha","LOGIN", "PASSWORD" ])'

After my 1st tests, I encountered the following problem
DBIx::Class::Carp::__ANON__(): "might_have/has_one" must not be on columns with is_nullable set to true (temp::Schema::Result::Borrower/cardnumber). This might indicate an incorrect use of those relationship helpers instead of belongs_to. at /usr/share/perl5/DBIx/Class/Relationship/HasOne.pm line 96
by setting cardnumber to non-nullable

I've switched the cardnumber to non-nullable in Borrower.pm
Comment 2 Paul Poulain 2012-06-25 13:05:40 UTC
Created attachment 10474 [details] [review]
Branches table management

it's called Branch2.pm because I tried another organisation, then switched to this one, which is better I think
Comment 3 Paul Poulain 2012-06-25 13:05:42 UTC
Created attachment 10475 [details] [review]
Borrowers table management

first draft, the interesting part is the testrelation.pl script that shows how to search a patron and access other informations from here

What is important is the "prefetch" option in read. With it you'll LEFT join the prefetched table, thus improving performances

You can also notice the cache duration of 2 seconds in borrower search. The idea is that, if this sub is called more than once on a given page, there will be only one SQL query.
Comment 4 Paul Poulain 2012-06-25 13:05:43 UTC
Created attachment 10476 [details] [review]
Branchrelation package, 1st draft

unused in my tests I think, commiting it just in case
Comment 5 Paul Poulain 2012-06-25 13:05:45 UTC
Created attachment 10477 [details] [review]
Categories management

This package is a POC to manage the Categories table. It contains :
* Category.pm (DB, DataObject and BusinessLogic levels)
* the Category.t test
* categorie.pl DBIx::Classed
  * updating copyright
  * removing a lot of useless files
  * tabs replaced by 4 spaces, result in some minor changes
  * use [% IF condition %] in templates, to avoid some pl level hacks
Comment 6 Paul Poulain 2012-06-25 13:06:17 UTC
Created attachment 10478 [details] [review]
DBIx::Class Branch.pm, first try

This package, and the test implement CRUD for Branches table.
Very basic, but proves we can do many things in a few lines !
Comment 7 Paul Poulain 2012-06-25 13:11:20 UTC
mmm... patches not in the historical order, sorry.
I made patch 6 (Branch.pm first try) first, then Branches table management (Branch2.pm).
To explain : I made some experiments on Branch.pm, then tried something else I called Branch2.pm
Branch2.pm seems better, so if we decide to go this way, the original Branch.pm will be dropped and Branch2.pm renamed. As it's just a POC, that's not a big deal I think.

In fact, the most important part is the testrelation.pl attached in "Borrowers table management" and Categories management, that include admin/categorie.pl rewritten
Comment 8 Elliott Davis 2012-12-21 15:35:07 UTC
*** Bug 8798 has been marked as a duplicate of this bug. ***
Comment 9 Magnus Enger 2016-06-03 07:40:04 UTC
4 years... Could this be closed?