"Last Few Circulations" total circulation count does not include aged circulations

[Chris Sharp]

The "Show Last Few Circulations" interface, available from several access points with the staff client, has a "Total Circs" column in its display that does not count archived circulations.

Following the thread from the interface's XUL file:

Open-ILS/xul/staff_client/server/cat/copy_summary.xul: 'render' : 'v = obj.network.simple_request("FM_CIRC_COUNT_RETRIEVE_VIA_COPY",[ my.acp.id() ]).total.count; v;'

Open-ILS/xul/staff_client/chrome/content/main/constants.js: 'FM_CIRC_COUNT_RETRIEVE_VIA_COPY' : { 'app' : 'open-ils.circ', 'method' : 'open-ils.circ.circulation.count' },

Open-ILS/src/perlmods/lib/OpenILS/Application/Circ.pm:

__PACKAGE__->register_method(
        method => "circ_count",
        api_name => "open-ils.circ.circulation.count",
        notes => q/
                Returns the number of times the item has circulated
                @param copyid The copy to check
        /);

sub circ_count {
        my( $self, $client, $copyid, $range ) = @_;
        my $e = OpenILS::Utils::Editor->new;
        return $e->request('open-ils.storage.asset.copy.circ_count', $copyid, $range);
}

Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/asset.pm:

sub circ_count {
        my $self = shift;
        my $client = shift;
        my $copy = shift;
        my $granularity = shift;

        my $c_table = action::circulation->table;

        if (lc($granularity) eq 'year') {
                $granularity = ", to_char(xact_start, 'YYYY') as when";
        } elsif (lc($granularity) eq 'month') {
                $granularity = ", to_char(xact_start, 'YYYY-MM') as when";
        } elsif (lc($granularity) eq 'day') {
                $granularity = ", to_char(xact_start, 'YYYY-MM-DD') as when";
        } else {
                $granularity = ", 'total' as when";
        }

        my $SQL = <<" SQL";
                SELECT COUNT(*) as count $granularity
                  FROM $c_table
                  WHERE target_copy = ?
        SQL

        if ($granularity !~ /total/o) {
                $SQL .= ' GROUP BY 2 ORDER BY 2';
        }

        $log->debug("Circ count SQL [$SQL]", DEBUG);

        return action::circulation->db_Main->selectall_hashref($SQL, 'when', {}, $copy);
}
__PACKAGE__->register_method(
        method => 'circ_count',
        api_name => 'open-ils.storage.asset.copy.circ_count',
        argc => 1,
);

"my $c_table = action::circulation->table;" - only includes "action.circulation"

Evergreen 2.3.6
OpenSRF 2.1.2
PostgreSQL 9.1.9
Ubuntu 12.04

[Chris Sharp (chrissharp123)]

My attempt at a fix:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/fix_show_last_few_circulations_circ_count

Dan Wells mentioned in IRC that we should try and leverage extend_reporter.full_circ_count here, which includes any entries to extend_reporter.legacy_circ_count in the totals. My fix here doesn't do that yet.

[Ben Shum (bshum)]

Assigning initial target of 2.5.1 (next bug fix release for 2.5 series). Do we want backport targets for other series?

[Ben Shum (bshum)]

Also, ready for "pullrequest" tagging?

[Bill Erickson (berick)]

I've pushed an alternate branch which includes legacy circs and collects data via cstore instead of storage.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1203734-last-few-circs-via-cstore

With the move to cstore, the storage call is no longer used, so I've removed it as well for the sake of cleanup.

--
Note, the storage call supported additional "granularity" options which were never used, so I didn't bother porting them to the cstore version. If there is 3rd-party code out there which does use it, we may have to reconsider.
--

My code sums rows from the action::circ_counts_by_year class, which is accessible to cstore/pcrud. A slightly more direct route would be to extend access to the extend_reporter::full_circ_count class outside of open-ils.reporter-store, but that seems like a bigger discussion.

[Chris Sharp (chrissharp123)]

Tested. Signoff branch at user/csharp/lp1203734-last-few-circs-via-cstore-signoff (http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1203734-last-few-circs-via-cstore-signoff)

[Ben Shum (bshum)]

Pushed with Chris' signoff to master and backported to rel_2_7 and rel_2_6. Thanks all!

[Ben Shum (bshum)]

Removing 2.next top level milestone, since this was a bug fix that was repaired in an actual series maintenance release.