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/