From d13ea3e24cb747cf3a58b624745b5f2b15d16f0c Mon Sep 17 00:00:00 2001 From: "David E. Wheeler" Date: Tue, 19 Jan 2010 16:53:06 -0800 Subject: [PATCH] Add reversion to desk view. Added the "and Revert" or "and Delete" option to the "Check In" menu on desk/My Workspace views. This saves the user from having to click "Edit" and then "Cancel Checkout". Suggested by Matt Rolf and Phillip Smith. In order to make this happen, the code that handles cancelling a checkout, which was duplicated nearly verbatim in the callbacks for the story, media, and template profiles has been factored out into a new class `Bric::App::Callback::Util::Asset`. [#62 state:resolved] --- comp/widgets/desk/desk_item.html | 7 +- lib/Bric/App/Callback/Desk.pm | 27 +++- lib/Bric/App/Callback/Profile/Media.pm | 50 +------- lib/Bric/App/Callback/Profile/Story.pm | 51 +------- lib/Bric/App/Callback/Profile/Template.pm | 58 +-------- lib/Bric/App/Callback/Util/Asset.pm | 147 ++++++++++++++++++++++ lib/Bric/Biz/Asset/Template.pm | 22 ++++ lib/Bric/Changes.pod | 10 ++ lib/Bric/Util/Language/de_de.pm | 2 + lib/Bric/Util/Language/it_it.pm | 2 + lib/Bric/Util/Language/pt_pt.pm | 2 + lib/Bric/Util/Language/ru_ru.pm | 2 + lib/Bric/Util/Language/zh_cn.pm | 2 + lib/Bric/Util/Language/zh_tw.pm | 2 + 14 files changed, 230 insertions(+), 154 deletions(-) create mode 100644 lib/Bric/App/Callback/Util/Asset.pm diff --git a/comp/widgets/desk/desk_item.html b/comp/widgets/desk/desk_item.html index 82fad6930..c7f421b36 100644 --- a/comp/widgets/desk/desk_item.html +++ b/comp/widgets/desk/desk_item.html @@ -201,8 +201,11 @@

Publish:

push @$checkin_opts, @and_publish; } } - my @and_shelve = ['shelve', $lang->maketext('and Shelve')]; - push @$checkin_opts, @and_shelve; + push @$checkin_opts, ( + ['shelve', $lang->maketext('and Shelve')], + ['cancel', $lang->maketext('and ' . ($obj->get_version == 0 ? 'Delete' : 'Revert'))], + ); + } my ($name, $date, $date_label, $type_label, $uri, $uuid); diff --git a/lib/Bric/App/Callback/Desk.pm b/lib/Bric/App/Callback/Desk.pm index 23c86522a..a7d6c5007 100644 --- a/lib/Bric/App/Callback/Desk.pm +++ b/lib/Bric/App/Callback/Desk.pm @@ -12,6 +12,7 @@ use Bric::App::Session qw(:state :user); use Bric::App::Event qw(log_event); use Bric::App::Util qw(:pkg :aref); use Bric::App::Callback::Publish; +use Bric::App::Callback::Util::Asset; use Bric::Biz::Asset::Business::Media; use Bric::Biz::Asset::Business::Story; use Bric::Biz::Asset::Template; @@ -38,9 +39,6 @@ sub checkin : Callback { my $obj = $pkgs->{$class}->lookup({'id' => $id, checkout => 1}); my $desk = $obj->get_current_desk; - $desk->checkin($obj); - log_event("${class}_checkin", $obj, { Version => $obj->get_version }); - # If the same asset is cached in the session, remove it. if (my $cached = get_state_data("$class\_prof" => $class)) { if ($cached->get_id == $obj->get_id) { @@ -52,6 +50,29 @@ sub checkin : Callback { my ($next_desk_id, $next_workflow_id) = split /-/, $self->params->{"desk_asset|next_desk"}; + if ($next_desk_id eq 'cancel') { + my $action; + print STDERR "####### ", $obj->get_checked_out, $/; + if ($obj->get_version == 0) { + # If the version number is 0, the asset was never checked in to a + # desk. So just delete it. + Bric::App::Callback::Util::Asset->remove($obj); + $action = 'deleted'; + } else { + # Cancel the checkout. + Bric::App::Callback::Util::Asset->cancel_checkout($obj); + $action = 'check out canceled'; + } + $self->add_message( + ucfirst $obj->key_name . qq{ "[_1]" $action.}, + '' . $obj->get_title . '', + ); + return; + } + + $desk->checkin($obj); + log_event("${class}_checkin", $obj, { Version => $obj->get_version }); + if ($next_desk_id eq 'shelve') { $desk->remove_asset($obj)->save; $obj->set_workflow_id(undef); diff --git a/lib/Bric/App/Callback/Profile/Media.pm b/lib/Bric/App/Callback/Profile/Media.pm index 669badef1..212b2636c 100644 --- a/lib/Bric/App/Callback/Profile/Media.pm +++ b/lib/Bric/App/Callback/Profile/Media.pm @@ -26,6 +26,7 @@ use Bric::Util::Priv::Parts::Const qw(:all); use Bric::Util::MediaType; use Bric::Util::Trans::FS; use Bric::App::Callback::Search; +use Bric::App::Callback::Util::Asset; my $SEARCH_URL = '/workflow/manager/media/'; my $ACTIVE_URL = '/workflow/active/media/'; @@ -403,44 +404,7 @@ sub cancel : Callback(priority => 6) { return unless $handle_delete->($media, $self); } else { # Cancel the checkout. - $media->cancel_checkout; - log_event('media_cancel_checkout', $media); - - # If the media was last recalled from the library, then remove it - # from the desk and workflow. We can tell this because there will - # only be one media_moved event and one media_checkout event - # since the last media_add_workflow event. - my @events = Bric::Util::Event->list({ - class => 'Bric::Biz::Asset::Business::Media', - obj_id => $media->get_id - }); - my ($desks, $cos) = (0, 0); - while (@events && $events[0]->get_key_name ne 'media_add_workflow') { - my $kn = shift(@events)->get_key_name; - if ($kn eq 'media_moved') { - $desks++; - } elsif ($kn eq 'media_checkout') { - $cos++ - } - } - - # If one move to desk, and one checkout, and this isn't the first - # time the media has been in workflow since it was created... - # XXX Two events upon creation: media_create and media_moved. - if ($desks == 1 && $cos == 1 && @events > 2) { - # It was just recalled from the library. So remove it from the - # desk and from workflow. - my $desk = $media->get_current_desk; - $desk->remove_asset($media); - $media->set_workflow_id(undef); - $desk->save; - $media->save; - log_event("media_rem_workflow", $media); - } else { - # Just save the cancelled checkout. It will be left in workflow for - # others to find. - $media->save; - } + Bric::App::Callback::Util::Asset->cancel_checkout($media); $self->add_message('Media "[_1]" check out canceled.', $media->get_title); } $self->clear_my_state; @@ -901,15 +865,7 @@ sub _handle_contributors { $handle_delete = sub { my ($media, $self) = @_; - my $desk = $media->get_current_desk; - $desk->checkin($media); - $desk->remove_asset($media); - $media->set_workflow_id(undef); - $media->deactivate; - $desk->save; - $media->save; - log_event("media_rem_workflow", $media); - log_event("media_deact", $media); + Bric::App::Callback::Util::Asset->remove($media); $self->add_message('Media "[_1]" deleted.', $media->get_title); }; diff --git a/lib/Bric/App/Callback/Profile/Story.pm b/lib/Bric/App/Callback/Profile/Story.pm index f16abdc1d..0e7729c54 100644 --- a/lib/Bric/App/Callback/Profile/Story.pm +++ b/lib/Bric/App/Callback/Profile/Story.pm @@ -23,6 +23,7 @@ use Bric::Util::Fault qw(:all); use Bric::Util::Grp::Parts::Member::Contrib; use Bric::Util::Priv::Parts::Const qw(:all); use Bric::App::Callback::Search; +use Bric::App::Callback::Util::Asset; my $SEARCH_URL = '/workflow/manager/story/'; my $ACTIVE_URL = '/workflow/active/story/'; @@ -293,45 +294,7 @@ sub cancel : Callback(priority => 6) { return unless $self->_handle_delete($story); } else { # Cancel the checkout. - $story->cancel_checkout; - log_event('story_cancel_checkout', $story); - - # If the story was last recalled from the library, then remove it - # from the desk and workflow. We can tell this because there will - # only be one story_moved event and one story_checkout event - # since the last story_add_workflow event. - my @events = Bric::Util::Event->list({ - class => ref $story, - obj_id => $story->get_id - }); - my ($desks, $cos) = (0, 0); - while (@events && $events[0]->get_key_name ne 'story_add_workflow') { - my $kn = shift(@events)->get_key_name; - if ($kn eq 'story_moved') { - $desks++; - } elsif ($kn eq 'story_checkout') { - $cos++ - } - } - - # If one move to desk, and one checkout, and this isn't the first - # time the story has been in workflow since it was created... - # XXX Three events upon creation: story_create, story_add_category, - # and story_moved. - if ($desks == 1 && $cos == 1 && @events > 3) { - # It was just recalled from the library. So remove it from the - # desk and from workflow. - my $desk = $story->get_current_desk; - $desk->remove_asset($story); - $story->set_workflow_id(undef); - $desk->save; - $story->save; - log_event("story_rem_workflow", $story); - } else { - # Just save the cancelled checkout. It will be left in workflow for - # others to find. - $story->save; - } + Bric::App::Callback::Util::Asset->cancel_checkout($story); $self->add_message( 'Story "[_1]" check out canceled.', '' . $story->get_title . '', @@ -987,15 +950,7 @@ sub _handle_contributors { sub _handle_delete { my ($self, $story) = @_; - my $desk = $story->get_current_desk(); - $desk->checkin($story) if $story->get_checked_out; - $desk->remove_asset($story); - $story->set_workflow_id(undef); - $story->deactivate; - $desk->save; - $story->save; - log_event("story_rem_workflow", $story); - log_event("story_deact", $story); + Bric::App::Callback::Util::Asset->remove($story); $self->add_message( 'Story "[_1]" deleted.', '' . $story->get_title . '', diff --git a/lib/Bric/App/Callback/Profile/Template.pm b/lib/Bric/App/Callback/Profile/Template.pm index bab0c1e5a..9fb66db03 100644 --- a/lib/Bric/App/Callback/Profile/Template.pm +++ b/lib/Bric/App/Callback/Profile/Template.pm @@ -9,6 +9,7 @@ use Bric::App::Authz qw(:all); use Bric::App::Event qw(log_event); use Bric::App::Session qw(:state :user); use Bric::App::Util qw(:history); +use Bric::App::Callback::Util::Asset; use Bric::Biz::Asset::Template; use Bric::Biz::ElementType; use Bric::Biz::Workflow; @@ -176,47 +177,8 @@ sub cancel : Callback(priority => 6) { # desk. So just delete it. $delete_fa->($self, $fa); } else { - # Cancel the checkout and undeploy the template from the user's - # sand box. - $fa->cancel_checkout; - log_event('template_cancel_checkout', $fa); - my $sb = Bric::Util::Burner->new({user_id => get_user_id()}); - $sb->undeploy($fa); - - # If the template was last recalled from the library, then remove it - # from the desk and workflow. We can tell this because there will - # only be one template_moved event and one template_checkout event - # since the last template_add_workflow event. - my @events = Bric::Util::Event->list({ - class => ref $fa, - obj_id => $fa->get_id - }); - my ($desks, $cos) = (0, 0); - while (@events && $events[0]->get_key_name ne 'template_add_workflow') { - my $kn = shift(@events)->get_key_name; - if ($kn eq 'template_moved') { - $desks++; - } elsif ($kn eq 'template_checkout') { - $cos++ - } - } - - # If one move to desk, and one checkout, and this isn't the first - # time the template has been in workflow since it was created... - if ($desks == 1 && $cos == 1 && @events > 2) { - # It was just recalled from the library. So remove it from the - # desk and from workflow. - my $desk = $fa->get_current_desk; - $desk->remove_asset($fa); - $fa->set_workflow_id(undef); - $desk->save; - $fa->save; - log_event("template_rem_workflow", $fa); - } else { - # Just save the cancelled checkout. It will be left in workflow for - # others to find. - $fa->save; - } + # Cancel the checkout. + Bric::App::Callback::Util::Asset->cancel_checkout($fa); $self->add_message('Template "[_1]" check out canceled.', $fa->get_file_name); } clear_state($self->class_key); @@ -630,19 +592,7 @@ $check_syntax = sub { $delete_fa = sub { my ($self, $fa) = @_; - my $desk = $fa->get_current_desk; - $desk->checkin($fa); - $desk->remove_asset($fa); - $desk->save; - log_event("template_rem_workflow", $fa); - my $burn = Bric::Util::Burner->new; - $burn->undeploy($fa); - my $sb = Bric::Util::Burner->new({user_id => get_user_id() }); - $sb->undeploy($fa); - $fa->set_workflow_id(undef); - $fa->deactivate; - $fa->save; - log_event("template_deact", $fa); + Bric::App::Callback::Util::Asset->remove($fa); $self->add_message('Template "[_1]" deleted.', $fa->get_file_name); }; diff --git a/lib/Bric/App/Callback/Util/Asset.pm b/lib/Bric/App/Callback/Util/Asset.pm new file mode 100644 index 000000000..b34868c35 --- /dev/null +++ b/lib/Bric/App/Callback/Util/Asset.pm @@ -0,0 +1,147 @@ +package Bric::App::Callback::Util::Asset; + +use strict; +use Bric::App::Event qw(log_event); +use Bric::App::Session qw(:user); + +sub new { bless {} => shift } + +sub cancel_checkout { + my ($self, $ass) = @_; + $ass->cancel_checkout; + my $kn = $ass->key_name; + my $class = ref $ass; + $class =~ s/::Media::.++$/::Media/ if $class =~ /Business::Media/; + + log_event("$kn\_cancel_checkout", $ass); + if ($ass->isa('Bric::Biz::Asset::Template')) { + my $sb = Bric::Util::Burner->new({user_id => get_user_id()}); + $sb->undeploy($ass); + } + + # If the asset was last recalled from the library, then remove it from the + # desk and workflow. We can tell this because there will only be one + # $kn\_moved event and one $kn\_checkout event since the last + # $kn\_add_workflow event. + + my @events = Bric::Util::Event->list({ + class => $class, + obj_id => $ass->get_id + }); + + my ($desks, $cos) = (0, 0); + while (@events && $events[0]->get_key_name ne "$kn\_add_workflow") { + my $kn = shift(@events)->get_key_name; + if ($kn eq "$kn\_moved") { + $desks++; + } elsif ($kn eq "$kn\_checkout") { + $cos++ + } + } + + # If one move to desk, and one checkout, and this isn't the first time the + # asset has been in workflow since it was created... + # XXX Two events upon creation: $kn\_create and $kn\_moved. + + if ($desks == 1 && $cos == 1 && @events > 2) { + # It was just recalled from the library. So remove it from the + # desk and from workflow. + my $desk = $ass->get_current_desk; + $desk->remove_asset($ass); + $ass->set_workflow_id(undef); + $desk->save; + $ass->save; + log_event("$kn\_rem_workflow", $ass); + } else { + # Just save the cancelled checkout. It will be left in workflow for + # others to find. + $ass->save; + } +} + +sub remove { + my ($self, $ass) = @_; + my $desk = $ass->get_current_desk; + $desk->checkin($ass) if $ass->get_checked_out; + $desk->remove_asset($ass); + if ($ass->isa('Bric::Biz::Asset::Template')) { + my $burn = Bric::Util::Burner->new; + $burn->undeploy($ass); + my $sb = Bric::Util::Burner->new({user_id => get_user_id() }); + $sb->undeploy($ass); + } + $ass->set_workflow_id(undef); + $ass->deactivate; + $desk->save; + $ass->save; + my $kn = $ass->key_name; + log_event("$kn\_rem_workflow", $ass); + log_event("$kn\_deact", $ass); +}; + +1; + +=head1 Name + +Bric::App::Callback::Util::Asset - Asset utilities for callbacks + +=head1 Synopsis + + use Bric::App::Callback::Util::Asset; + Bric::App::Callback::Util::Asset->cancel_checkout($asset); + +=head1 Description + +This module provides utility methods for managing Bricolage assets. + +=head1 Interface + +=head2 Constructors + +=head3 new + + my $au = Bric::App::Callback::Util::Asset->new; + +Constructs a new Bric::App::Callback::Util::Asset object. + +=head2 Class Methods + +=head3 cancel_checkout + + Bric::App::Callback::Util::Asset->cancel_checkout($asset); + +Cancels the checkout of C<$asset>. This method does its best to remove all +vestiges of the current checkout of the asset. If the asset is a template, it +will be undeployed from the user's sandbox. + +=head3 remove + + Bric::App::Callback::Util::Asset->remove($asset); + +Checks in an asset (if it's checked out), removes it from workflow, and +deactivates it. If the asset is a template, it will be undeployed. + +=head1 Author + +David E. Wheeler + +=head1 See Also + +=over 4 + +=item L + +=item L + +=item L + +=item L + +=back + +=head1 Copyright and License + +Copyright (c) 2010 Kineticode, Inc. See L for +complete license terms and conditions. + +=cut diff --git a/lib/Bric/Biz/Asset/Template.pm b/lib/Bric/Biz/Asset/Template.pm index bc4be2de8..c562f9cae 100644 --- a/lib/Bric/Biz/Asset/Template.pm +++ b/lib/Bric/Biz/Asset/Template.pm @@ -1428,6 +1428,28 @@ NONE ################################################################################ +=item $file_name = $template->get_title() + +An alias for C. + +B + +NONE + +B + +NONE + +B + +NONE + +=cut + +sub get_title { shift->get_file_name } + +################################################################################ + =item $name = $template->get_output_channel_name; Return the name of the output channel. diff --git a/lib/Bric/Changes.pod b/lib/Bric/Changes.pod index ff1532798..32cf29c9d 100644 --- a/lib/Bric/Changes.pod +++ b/lib/Bric/Changes.pod @@ -35,6 +35,16 @@ C, if the XML file sent to the server contains no C<< >> elements, the existing file will be left alone, rather than deleted. Suggested by Ashlee Caul. [David] +=item * + +Added the "and Revert" or "and Delete" option to the "Check In" menu on +desk/My Workspace views. This saves the user from having to click "Edit" and +then "Cancel Checkout". Suggested by Matt Rolf and Phillip Smith. In order to +make this happen, the code that handles cancelling a checkout, which was +duplicated nearly verbatim in the callbacks for the story, media, and template +profiles has been factored out into a new class +L. [David] + =back =head2 Bug Fixes diff --git a/lib/Bric/Util/Language/de_de.pm b/lib/Bric/Util/Language/de_de.pm index 76a40c2ba..d2846a298 100644 --- a/lib/Bric/Util/Language/de_de.pm +++ b/lib/Bric/Util/Language/de_de.pm @@ -788,6 +788,8 @@ To Translate: 'Paste ([_1])' => 'Paste ([_1])', # As in Copy/Paste 'You do not have [_1] access to any desks in the "[_2]" workflow' => 'You do not have [_1] access to any desks in the "[_2]" workflow' 'Start Bricolage' => 'Start Bricolage', +'and Revert' => 'and Revert', +'and Delete' => 'and Delete', =end comment diff --git a/lib/Bric/Util/Language/it_it.pm b/lib/Bric/Util/Language/it_it.pm index b78b910b7..8f3cd65ec 100644 --- a/lib/Bric/Util/Language/it_it.pm +++ b/lib/Bric/Util/Language/it_it.pm @@ -603,6 +603,8 @@ To translate: 'Could not create keyword, "[_1]", as you have not been granted permission to create new keywords.' => 'Could not create keyword, "[_1]", as you have not been granted permission to create new keywords.', 'Paste ([_1])' => 'Paste ([_1])', # As in Copy/Paste 'You do not have [_1] access to any desks in the "[_2]" workflow' => 'You do not have [_1] access to any desks in the "[_2]" workflow' + 'and Revert' => 'and Revert', + 'and Delete' => 'and Delete', =end comment diff --git a/lib/Bric/Util/Language/pt_pt.pm b/lib/Bric/Util/Language/pt_pt.pm index 156506f08..3367960a3 100644 --- a/lib/Bric/Util/Language/pt_pt.pm +++ b/lib/Bric/Util/Language/pt_pt.pm @@ -774,6 +774,8 @@ To translate: 'Paste ([_1])' => 'Paste ([_1])', # As in Copy/Paste 'You do not have [_1] access to any desks in the "[_2]" workflow' => 'You do not have [_1] access to any desks in the "[_2]" workflow' 'Start Bricolage' => 'Start Bricolage', + 'and Revert' => 'and Revert', + 'and Delete' => 'and Delete', =end comment diff --git a/lib/Bric/Util/Language/ru_ru.pm b/lib/Bric/Util/Language/ru_ru.pm index 9e56b4924..5d87bddcf 100644 --- a/lib/Bric/Util/Language/ru_ru.pm +++ b/lib/Bric/Util/Language/ru_ru.pm @@ -770,6 +770,8 @@ To Translate: 'Paste ([_1])' => 'Paste ([_1])', # As in Copy/Paste 'You do not have [_1] access to any desks in the "[_2]" workflow' => 'You do not have [_1] access to any desks in the "[_2]" workflow' 'Start Bricolage' => 'Start Bricolage', + 'and Revert' => 'and Revert', + 'and Delete' => 'and Delete', =end comment diff --git a/lib/Bric/Util/Language/zh_cn.pm b/lib/Bric/Util/Language/zh_cn.pm index ca05214e2..a044dc5f8 100644 --- a/lib/Bric/Util/Language/zh_cn.pm +++ b/lib/Bric/Util/Language/zh_cn.pm @@ -642,6 +642,8 @@ To translate: 'Could not create keyword, "[_1]", as you have not been granted permission to create new keywords.' => 'Could not create keyword, "[_1]", as you have not been granted permission to create new keywords.', 'Paste ([_1])' => 'Paste ([_1])', # As in Copy/Paste 'You do not have [_1] access to any desks in the "[_2]" workflow' => 'You do not have [_1] access to any desks in the "[_2]" workflow' + 'and Revert' => 'and Revert', + 'and Delete' => 'and Delete', =end comment diff --git a/lib/Bric/Util/Language/zh_tw.pm b/lib/Bric/Util/Language/zh_tw.pm index fe230bc13..38e8856b5 100644 --- a/lib/Bric/Util/Language/zh_tw.pm +++ b/lib/Bric/Util/Language/zh_tw.pm @@ -768,6 +768,8 @@ To translate: 'Paste ([_1])' => 'Paste ([_1])', # As in Copy/Paste 'You do not have [_1] access to any desks in the "[_2]" workflow' => 'You do not have [_1] access to any desks in the "[_2]" workflow' 'Start Bricolage' => 'Start Bricolage', + 'and Revert' => 'and Revert', + 'and Delete' => 'and Delete', Notice: