From 153f8b073e5b1d0fd559325efba496c4df511424 Mon Sep 17 00:00:00 2001 From: Bill Erickson Date: Mon, 21 Dec 2015 11:19:19 -0500 Subject: [PATCH] LP#1527342 Patron checkout history TPAC display Checkout history is now derived from the new action.usr_circ_history table. When a patron disables circ history, all history is deleted from the new table. Also, when disabling circ or holds history, the patron is now warned if data will be deleted or, in the case of holds, become inaccessible. Signed-off-by: Bill Erickson Signed-off-by: Kathy Lussier --- .../perlmods/lib/OpenILS/Application/Actor.pm | 106 ++++++++++-- .../lib/OpenILS/WWW/EGCatLoader/Account.pm | 160 +++++++++++++----- Open-ILS/src/templates/opac/css/style.css.tt2 | 6 + .../templates/opac/myopac/prefs_settings.tt2 | 17 +- 4 files changed, 231 insertions(+), 58 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm index e5b43162cd..281298a1e0 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm @@ -4215,33 +4215,47 @@ sub event_def_opt_in_settings { __PACKAGE__->register_method( - method => "user_visible_circs", - api_name => "open-ils.actor.history.circ.visible", + method => "user_circ_history", + api_name => "open-ils.actor.history.circ", stream => 1, + authoritative => 1, signature => { - desc => 'Returns the set of opt-in visible circulations accompanied by circulation chain summaries', + desc => 'Returns user circ history objects for the calling user', params => [ { desc => 'Authentication token', type => 'string'}, - { desc => 'User ID. If no user id is present, the authenticated user is assumed', type => 'number' }, { desc => 'Options hash. Supported fields are "limit" and "offset"', type => 'object' }, ], return => { - desc => q/An object with 2 fields: circulation and summary. - circulation is the "circ" object. summary is the related "accs" object/, + desc => q/Stream of 'auch' circ history objects/, type => 'object', } } ); __PACKAGE__->register_method( - method => "user_visible_circs", - api_name => "open-ils.actor.history.circ.visible.print", + method => "user_circ_history", + api_name => "open-ils.actor.history.circ.clear", stream => 1, signature => { - desc => 'Returns printable output for the set of opt-in visible circulations', + desc => 'Delete all user circ history entries for the calling user', + params => [ + { desc => 'Authentication token', type => 'string'}, + ], + return => { + desc => q/1 on success, event on error/, + type => 'object', + } + } +); + +__PACKAGE__->register_method( + method => "user_circ_history", + api_name => "open-ils.actor.history.circ.print", + stream => 1, + signature => { + desc => q/Returns printable output for the caller's circ history objects/, params => [ { desc => 'Authentication token', type => 'string'}, - { desc => 'User ID. If no user id is present, the authenticated user is assumed', type => 'number' }, { desc => 'Options hash. Supported fields are "limit" and "offset"', type => 'object' }, ], return => { @@ -4252,11 +4266,11 @@ __PACKAGE__->register_method( ); __PACKAGE__->register_method( - method => "user_visible_circs", - api_name => "open-ils.actor.history.circ.visible.email", + method => "user_circ_history", + api_name => "open-ils.actor.history.circ.email", stream => 1, signature => { - desc => 'Emails the set of opt-in visible circulations to the requestor', + desc => q/Emails the caller's circ history/, params => [ { desc => 'Authentication token', type => 'string'}, { desc => 'User ID. If no user id is present, the authenticated user is assumed', type => 'number' }, @@ -4268,6 +4282,68 @@ __PACKAGE__->register_method( } ); +sub user_circ_history { + my ($self, $conn, $auth, $options) = @_; + $options ||= {}; + + my $for_print = ($self->api_name =~ /print/); + my $for_email = ($self->api_name =~ /email/); + my $for_clear = ($self->api_name =~ /clear/); + + # No perm check is performed. Caller may only access his/her own + # circ history entries. + my $e = new_editor(authtoken => $auth); + return $e->event unless $e->checkauth; + + my %limits = (); + if (!$for_clear) { # clear deletes all + $limits{offset} = $options->{offset} if defined $options->{offset}; + $limits{limit} = $options->{limit} if defined $options->{limit}; + } + + my $circs = $e->search_action_user_circ_history([ + {usr => $e->requestor->id}, + { # order newest to oldest by default + order_by => {auch => 'xact_start DESC'}, + %limits + }, + {substream => 1} # could be a large list + ]); + + if ($for_print) { + return $U->fire_object_event(undef, + 'circ.format.history.print', $circs, $e->requestor->home_ou); + } + + $e->xact_begin if $for_clear; + $conn->respond_complete(1) if $for_email; # no sense in waiting + + for my $circ (@$circs) { + + if ($for_email) { + # events will be fired from action_trigger_runner + $U->create_events_for_hook('circ.format.history.email', + $circ, $e->editor->home_ou, undef, undef, 1); + + } elsif ($for_clear) { + + $e->delete_action_user_circ_history($circ) + or return $e->die_event; + + } else { + $conn->respond($circ); + } + } + + if ($for_clear) { + $e->commit; + return 1; + } + + return undef; +} + + __PACKAGE__->register_method( method => "user_visible_circs", api_name => "open-ils.actor.history.hold.visible", @@ -4321,10 +4397,10 @@ __PACKAGE__->register_method( } ); -sub user_visible_circs { +sub user_visible_holds { my($self, $conn, $auth, $user_id, $options) = @_; - my $is_hold = ($self->api_name =~ /hold/); + my $is_hold = 1; my $for_print = ($self->api_name =~ /print/); my $for_email = ($self->api_name =~ /email/); my $e = new_editor(authtoken => $auth); diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm index 30e07561c0..b3c15bf611 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm @@ -616,18 +616,81 @@ sub load_myopac_prefs_settings { $settings{$key}= $val unless $$set_map{$key} eq $val; } + # Used by the settings update form when warning on history delete. + my $clear_circ_history = 0; + my $clear_hold_history = 0; + + # true if we need to show the warning on next page load. + my $hist_warning_needed = 0; + my $hist_clear_confirmed = $self->cgi->param('history_delete_confirmed'); + my $now = DateTime->now->strftime('%F'); - foreach my $key (qw/history.circ.retention_start history.hold.retention_start/) { + foreach my $key ( + qw/history.circ.retention_start history.hold.retention_start/) { + my $val = $self->cgi->param($key); if($val and $val eq 'on') { # Set the start time to 'now' unless a start time already exists for the user $settings{$key} = $now unless $$set_map{$key}; + } else { - # clear the start time if one previously existed for the user - $settings{$key} = undef if $$set_map{$key}; + + next unless $$set_map{$key}; # nothing to do + + $clear_circ_history = 1 if $key =~ /circ/; + $clear_hold_history = 1 if $key =~ /hold/; + + if (!$hist_clear_confirmed) { + # when clearing circ history, only warn if history data exists. + + if ($clear_circ_history) { + + if ($self->fetch_user_circ_history(0, 1)->[0]) { + $hist_warning_needed = 1; + next; # no history updates while confirmation pending + } + + } else { + + my $one_hold = $e->json_query({ + select => { + au => [{ + column => 'id', + transform => 'action.usr_visible_holds', + result_field => 'id' + }] + }, + from => 'au', + where => {id => $e->requestor->id}, + limit => 1 + })->[0]; + + if ($one_hold) { + $hist_warning_needed = 1; + next; # no history updates while confirmation pending + } + } + } + + $settings{$key} = undef; + + if ($key eq 'history.circ.retention_start') { + # delete existing circulation history data. + $U->simplereq( + 'open-ils.actor', + 'open-ils.actor.history.circ.clear', + $self->editor->authtoken); + } } } + # Warn patrons before clearing circ/hold history + if ($hist_warning_needed) { + $self->ctx->{clear_circ_history} = $clear_circ_history; + $self->ctx->{clear_hold_history} = $clear_hold_history; + $self->ctx->{confirm_history_delete} = 1; + } + # Send the modified settings off to be saved $U->simplereq( 'open-ils.actor', @@ -1526,38 +1589,62 @@ sub load_myopac_circ_history { $ctx->{circ_history_limit} = $limit; $ctx->{circ_history_offset} = $offset; - my $circ_ids; - if ($self->cgi->param('sort') ne "") { # Defer limitation to circ_history.tt2 - $circ_ids = $e->json_query({ - select => { - au => [{ - column => 'id', - transform => 'action.usr_visible_circs', - result_field => 'id' - }] + # Defer limitation to circ_history.tt2 when sorting + if ($self->cgi->param('sort')) { + $limit = undef; + $offset = undef; + } + + $ctx->{circs} = $self->fetch_user_circ_history(1, $limit, $offset); + return Apache2::Const::OK; +} + +# if 'flesh' is set, copy data etc. is loaded and the return value is +# a hash of 'circ' and 'marc_xml'. Othwerwise, it's just a list of +# auch objects. +sub fetch_user_circ_history { + my ($self, $flesh, $limit, $offset) = @_; + my $e = $self->editor; + + my %limits = (); + $limits{offset} = $offset if defined $offset; + $limits{limit} = $limit if defined $limit; + + my %flesh_ops = ( + flesh => 3, + flesh_fields => { + auch => ['target_copy'], + acp => ['call_number'], + acn => ['record'] }, - from => 'au', - where => {id => $e->requestor->id} - }); + ); - } else { - $circ_ids = $e->json_query({ - select => { - au => [{ - column => 'id', - transform => 'action.usr_visible_circs', - result_field => 'id' - }] + $e->xact_begin; + my $circs = $e->search_action_user_circ_history([ + {usr => $e->requestor->id}, + { # order newest to oldest by default + order_by => {auch => 'xact_start DESC'}, + $flesh ? %flesh_ops : (), + %limits }, - from => 'au', - where => {id => $e->requestor->id}, - limit => $limit, - offset => $offset + {substream => 1} + ]); + $e->rollback; + + return $circs unless $flesh; + + my @circs; + for my $circ (@$circs) { + push(@circs, { + circ => $circ, + marc_xml => ($circ->target_copy->call_number->id != -1) ? + XML::LibXML->new->parse_string( + $circ->target_copy->call_number->record->marc) : + undef # pre-cat copy, use the dummy title/author instead }); } - $ctx->{circs} = $self->fetch_user_circs(1, [map { $_->{id} } @$circ_ids]); - return Apache2::Const::OK; + return \@circs; } # TODO: action.usr_visible_holds does not return cancelled holds. Should it? @@ -2590,22 +2677,13 @@ sub load_myopac_circ_history_export { my $e = $self->editor; my $filename = $self->cgi->param('filename') || 'circ_history.csv'; - my $ids = $e->json_query({ - select => { - au => [{ - column => 'id', - transform => 'action.usr_visible_circs', - result_field => 'id' - }] - }, - from => 'au', - where => {id => $e->requestor->id} - }); + my $circs = $self->fetch_user_circ_history; $self->ctx->{csv} = $U->fire_object_event( undef, 'circ.format.history.csv', - $e->search_action_circulation({id => [map {$_->{id}} @$ids]}, {substream =>1}), + $e->search_action_circulation( + {id => [map {$_->id} @$circs]}, {substream =>1}), $self->editor->requestor->home_ou ); diff --git a/Open-ILS/src/templates/opac/css/style.css.tt2 b/Open-ILS/src/templates/opac/css/style.css.tt2 index 2bfe26b61e..fe98473323 100644 --- a/Open-ILS/src/templates/opac/css/style.css.tt2 +++ b/Open-ILS/src/templates/opac/css/style.css.tt2 @@ -2133,3 +2133,9 @@ label[for*=expert_] text-align: center; font-style: italic; } + +#clear-history-confirm { + font-weight: bold; + color: [% css_colors.text_badnews %]; + padding: 10px; +} diff --git a/Open-ILS/src/templates/opac/myopac/prefs_settings.tt2 b/Open-ILS/src/templates/opac/myopac/prefs_settings.tt2 index 9f1288c8b5..edefb5c581 100644 --- a/Open-ILS/src/templates/opac/myopac/prefs_settings.tt2 +++ b/Open-ILS/src/templates/opac/myopac/prefs_settings.tt2 @@ -72,7 +72,8 @@ + [% IF ctx.user_setting_map.$setting + AND !ctx.clear_circ_history; %] checked='checked' [% END %]/> [%- setting = 'history.hold.retention_start' -%] @@ -82,7 +83,8 @@ + [% IF ctx.user_setting_map.$setting + AND !ctx.clear_hold_history; %] checked='checked' [% END %]/> [%- setting = 'opac.temporary_list_no_warn' -%] @@ -121,7 +123,18 @@ + [% IF ctx.confirm_history_delete %] +
+ [% l('Disabling checkout or holds history will permanently remove all items from your history.') %] +
+ [% l('Are you sure you wish to continue?') %] +
+ [% l('Cancel') %] + + + [% ELSE %] + [% END %] [% INCLUDE "opac/parts/myopac/prefs_hints.tt2" %] [% END %] -- 2.43.2