[Koha-bugs] [Bug 15504] Track Patron's Last Activity

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15504

--- Comment #101 from Martin Renvoize <martin.renvoize at ptfs-europe.com> ---
(In reply to Marcel de Rooy from comment #96)
> dbrev
> Blocker
> The default should probably be LOGIN IF TrackLastLogin was enabled.
> Please adjust.

Done, will post patch momentarily.. however, it's actually
'connection,login,check_out' to match the existing behaviour.

> Preferences
> Clearing all triggers is the same as turning off existing pref? Could we
> remove it ? Here or in a follow-up ?

I like that, perhaps as a quick follow-up bug.. I'm happy to code that tonight.

> my $cache_key = "track_login_" . $self->userid;
> No blocker. Would probably be better to cache with borrowernumber. Userid
> may change?

Totally agree on this one.. though we just used the existing key.  I've amended
it now as another follow-up anyway.. about to upload.

> +    Time::Fake->offset( $now->epoch + 5 );
> No blocker, just teasing :)
> You might have trouble in case you run the test close to midnight.
> Could we cache the epoch value ?

Oh, this could be fun.. one for later though in my opinion.. should it crop up.

> -sub track_login {
> -    my ( $self, $params ) = @_;
> -    return if
> -        !$params->{force} &&
> Didnt we use force somewhere before?

It was supported before, but never actually used. Matt and I discussed it and
decided to remove the functionality for now as it was never called that way.

> Login from OPAC => Login
> => Staff login counts as well. I think we should just remove OPAC here.

Fair, I'll do a follow-up for that too before posting up the current rebase.

> This code still needs attention.
> Blocker
> koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt
>                 [% IF Koha.Preference('TrackLastPatronActivity') %]
>                     <li>
>                         <label for="borrower_lastseen">who have not been
> connected since:</label>
>                         <input size="10" id="borrower_lastseen"
> name="borrower_lastseen" type="text" class="flatpickr" />
>                         <span class="hint">[% INCLUDE 'date-format.inc'
> %]</span>
>                     </li>
>                 [% END %]
> AND Similarly:
> misc/cronjobs/delete_patrons.pl
> =item B<--last_seen>
> Delete patrons who have not been connected since this date.
> The system preference TrackLastPatronActivity must be enabled to use this
> option.

Looking at these two now.

-- 
You are receiving this mail because:
You are watching all bug changes.