Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix handling of publish failures in Ajax publish requests.

This fixes the half of bug #277, when one attempts to publish from a desk or workspace.

When `PUBLISH_RELATED_FAIL_BEHAVIOR = fail` and a related asset fails to publish, we now call `raise_conflict` to return a 409 and do a better job displaying *all* the appropriate error messages.

When `PUBLISH_RELATED_FAIL_BEHAVIOR = warn` and  related asset fails to publish, we call a new moethod, `show_accepted()`. This method returns a 202 status code, which I'm abusing a bit here, but it comes closest to what we want. The story properly publishes and disappears from the desk, but a new handler in the Ajax code also shows the errors when related failed to publish. I also added code to abort at the end of the Desk `publish` callback when a request is Ajax so that no other stuff gets sent back to the browser. This is because in "warn" mode, we wnt the full request to succeed, with no rollbacks or anything, and all subsequent code should execute, so that the story will properly be published and removed from workflow.

Tomorrow I'll have to figure out what to do about non-ajax publish requests in order to properly and finally fix bug # 277.
  • Loading branch information...
commit ca5f60ff338263875bfad7b467e0ab6fee730d02 1 parent 12959c7
@theory theory authored
View
6 comp/media/js/lib.js
@@ -1239,7 +1239,11 @@ var Desk = {
onSuccess: onSuccess,
onFailure: Bricolage.handleError,
on403: Bricolage.handleForbidden,
- on409: Bricolage.handleConflict
+ on409: Bricolage.handleConflict,
+ on202: function(req) {
+ Bricolage.handleConflict(req);
+ onSuccess(req);
+ }
} );
},
View
47 lib/Bric/App/Callback.pm
@@ -6,8 +6,8 @@ __PACKAGE__->register_subclass;
use constant CLASS_KEY => 'Callback';
use HTML::Entities;
use Bric::App::Cache;
-use Bric::App::Util qw(get_pref add_msg);
-use Bric::Util::ApacheConst qw(HTTP_CONFLICT HTTP_FORBIDDEN);
+use Bric::App::Util qw(get_pref add_msg get_msg clear_msg);
+use Bric::Util::ApacheConst qw(HTTP_CONFLICT HTTP_FORBIDDEN HTTP_ACCEPTED);
use Bric::Util::DBI qw(begin rollback);
use Bric::Util::Language;
@@ -24,34 +24,49 @@ sub add_message {
sub raise_status {
my ($self, $status) = (shift, shift);
- my $r = $self->apache_req or return $self->add_message(@_);
+ $self->add_message(@_);
+ my $r = $self->apache_req or return;
# If it's not an Ajax request, it's just a message.
- return $self->add_message(@_)
- if ($r->headers_in->{'X-Requested-With'} || '') ne 'XMLHttpRequest';
+ return if ($r->headers_in->{'X-Requested-With'} || '') ne 'XMLHttpRequest';
# Abort the database transaction.
rollback(1);
begin(1);
- # Prep the error message.
- my $txt = Bric::Util::Language->instance->maketext(@_);
- if ($txt =~ /(.*)<span class="l10n">(.*)<\/span>(.*)/) {
- $txt = encode_entities($1) . '<span class="l10n">'
- . encode_entities($2) . '</span>' . encode_entities($3);
- } else {
- $txt = encode_entities($txt);
- }
-
# Send the status to the browser and abort.
- $r->status($status);
- $r->print(qq{<div class="errorMsg">$txt</div>});
+ $self->_print_messages($r, $status);
$self->abort;
}
sub raise_conflict { shift->raise_status( HTTP_CONFLICT, @_ ) }
sub raise_forbidden { shift->raise_status( HTTP_FORBIDDEN, @_ ) }
+sub show_accepted {
+ my $self = shift;
+ $self->add_message(@_);
+ my $r = $self->apache_req or return;
+
+ # If it's not an Ajax request, it's just a message.
+ return if ($r->headers_in->{'X-Requested-With'} || '') ne 'XMLHttpRequest';
+
+ $self->_print_messages($r, HTTP_ACCEPTED);
+}
+
+sub _print_messages {
+ my ($self, $r, $status) = @_;
+ # Prep the error message(s).
+ my $txt = join '<br /><br />', map {
+ /(.*)<span class="l10n">(.*)<\/span>(.*)/
+ ? encode_entities($1) . '<span class="l10n">'
+ . encode_entities($2) . '</span>' . encode_entities($3)
+ : encode_entities($_)
+ } get_msg;
+ clear_msg;
+ $r->status($status);
+ $r->print(qq{<div class="errorMsg">$txt</div>});
+}
+
1;
=head1 Name
View
17 lib/Bric/App/Callback/Desk.pm
@@ -366,16 +366,15 @@ sub publish : Callback {
# By this point we now know if we're going to fail this publish
# if we set the fail behaviour to fail rather than warn
if (PUBLISH_RELATED_ASSETS && @messages) {
+ $self->add_message(@$_) for @messages;
if (PUBLISH_RELATED_FAIL_BEHAVIOR eq 'fail') {
- $self->add_message(@$_) for @messages;
- my $msg = 'Publish aborted due to errors above. Please fix the '
- . ' above problems and try again.';
- throw_error error => $msg,
- maketext => $msg;
+ $self->raise_conflict(
+ ['Publish aborted due to errors above. Please fix the above problems and try again.']);
} else {
# we are set to warn, should we add a further warning to the msg ?
- $self->raise_conflict(@$_) for @messages,
- 'Some of the related assets were not published.';
+ $self->show_accepted(
+ ['Some of the related assets were not published.']
+ );
}
} else {
$self->raise_conflict(@$_) for @messages;
@@ -408,6 +407,10 @@ sub publish : Callback {
} else {
$self->set_redirect('/workflow/profile/publish');
}
+
+ # Don't render anything for Ajax requests.
+ my $r = $self->apache_req or return;
+ $self->abort if ($r->headers_in->{'X-Requested-With'} || '') eq 'XMLHttpRequest';
}
sub deploy : Callback {
Please sign in to comment.
Something went wrong with that request. Please try again.