I do believe use of DBIx::Class in existing subs is both good and encouraged! That is, as long as there are no regressions! It seems like a good idea to add that field to the borrowers table. I'm sure there are many Koha administrators that need to sync data between Koha and other systems, and it seems like this field could be useful in a more general fashion. Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com ) On Tue, Jan 21, 2014 at 5:17 AM, Magnus Enger <magnus@enger.priv.no> wrote:
Hi!
I am working on Bug 11401 - Add support for Norwegian national library card: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11401
The goal is to sync patrons in Koha back and forth to the Norwegian national patron database. In order to do this I need to keep track of - which patrons should be synced (they can choose to opt out of the syncing) - a sync status, to see which patrons are new, which have been changed and which are already synced - the date of the last sync
I am a bit undecided about whether I should use extended patron attributes for this, or add columns to the "borrowers" table. Some pros and cons: - Attributes are less intrusive, they only need to be added to systems that want to sync with the Norwegian national patron database. - New columns could be re-used by others, to implement other syncing schemes, and they are easier to work with, as far as I can see. I am currently leaning towards adding columns.
Now the thing is that I want to change the "syncstatus" every time AddMember or ModMember is called, so I want to add some logic to these subroutines, perhaps like this ("..." indicates lines that have been edited out for clarity):
use Koha::Database; ... sub AddMember { my (%data) = @_; ... $data{'borrowernumber'}=InsertInTable("borrowers",\%data);
# If NorwegianPatronDBEnable is enabled, we set the nllastsync attribute to something that a # cronjob will use for syncing with NL if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) { my $borrower = Koha::Database->new->schema->resultset('Borrower')->find( $data{'borrowernumber'} ); $borrower->update( { 'syncstatus' => 'new' } ); } ... }
Would that be an OK way to do it? Or is mixing in DBIC in existing subroutines like this frowned upon?
Would it be cleaner to add a module like Koha::Borrower::Sync (that could be re-used by other syncing schemes), with methods like SetBorrowerSyncstatus( $patronnumber, $value ) that would then look something like this:
use Koha::Database; sub SetBorrowerSyncstatus { my ( $borrowernumber, $value ) = @_; my $borrower = Koha::Database->new->schema->resultset('Borrower')->find( $borrowernumber ); $borrower->update( { 'syncstatus' => 'new' } ); }
Any advice on this would be much appreciated!
Best regards, Magnus _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/