Advice on syncing and DBIC
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
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/
Magnus Enger schreef op di 21-01-2014 om 11:17 [+0100]:
- Attributes are less intrusive, they only need to be added to systems that want to sync with the Norwegian national patron database.
I'd tend to go towards attributes, but...
- 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.
...if not, I don't think it should be a new column. It probably should be a table that can be joined with borrowers. a) there's enough stuff in there as it is, but more b) things that are logically not specifically borrower details should have their own place to be. -- Robin Sheat Catalyst IT Ltd. ✆ +64 4 803 2204 GPG: 5FA7 4B49 1E4D CAA4 4C38 8505 77F5 B724 F871 3BDF
participants (3)
-
Kyle Hall -
Magnus Enger -
Robin Sheat