Permalink
Browse files

Merge pull request #39 from alphagov/improving_error_handling

Improving error handling
  • Loading branch information...
2 parents 5227859 + 2eac26d commit b033efb427e74ee251aa7387577b9dcf4630d24c @heathd heathd committed Jan 17, 2013
View
@@ -22,7 +22,7 @@ gem 'validates_timeliness', '3.0.14'
if ENV['GDS_ZENDESK_DEV']
gem "gds_zendesk", :path => '../gds_zendesk'
else
- gem "gds_zendesk", :git => 'https://github.com/alphagov/gds_zendesk.git', :ref => 'cb88a21e1f'
+ gem "gds_zendesk", '0.0.3'
end
group :test do
View
@@ -1,12 +1,3 @@
-GIT
- remote: https://github.com/alphagov/gds_zendesk.git
- revision: cb88a21e1f74bdb8a28f43d124eadbda6ea61f46
- ref: cb88a21e1f
- specs:
- gds_zendesk (0.0.1)
- null_logger (= 0.0.1)
- zendesk_api (= 0.1.2)
-
GEM
remote: https://rubygems.org/
remote: https://gems.gemfury.com/vo6ZrmjBQu5szyywDszE/
@@ -96,6 +87,9 @@ GEM
rack-accept (~> 0.4.4)
rails (>= 3.0.0)
warden (~> 1.2)
+ gds_zendesk (0.0.3)
+ null_logger (= 0.0.1)
+ zendesk_api (= 0.1.2)
gherkin (2.11.2)
json (>= 1.4.6)
hashie (1.2.0)
@@ -256,7 +250,7 @@ DEPENDENCIES
exception_notification (~> 2.4.1)
formtastic-bootstrap (= 2.0.0)
gds-sso (= 3.0.0)
- gds_zendesk!
+ gds_zendesk (= 0.0.3)
jquery-rails
jquery-ui-rails (= 2.0.2)
less-rails-bootstrap (= 2.1.1)
@@ -13,8 +13,21 @@
//= require jquery
//= require jquery_ujs
//= require jquery.ui.datepicker
+//= require twitter/bootstrap/collapse
//= require_tree .
$(document).ready(function() {
$('input[calendar-enabled=true]').datepicker({minDate: 0, dateFormat: 'dd-mm-yy'});
+
+ $('#problem-details-toggle, .icon-chevron-right').removeClass("hidden");
+ $('#problem-details').removeClass("in");
+
+ $('#problem-details').on('hidden', function () {
+ $('.icon-chevron-down').addClass('hidden');
+ $('.icon-chevron-right').removeClass('hidden');
+ });
+ $('#problem-details').on('shown', function () {
+ $('.icon-chevron-right').addClass('hidden');
+ $('.icon-chevron-down').removeClass('hidden');
+ });
});
@@ -11,7 +11,7 @@ def create
if @request.valid?
process_valid_request(@request)
else
- render :new, :status => 400
+ render :new, status: 400
end
end
@@ -27,7 +27,11 @@ def raise_ticket(ticket)
if ticket
redirect_to acknowledge_path
else
- return render "support/zendesk_error", :locals => {:error_string => "zendesk_error_upon_submit"}
+ return render "support/zendesk_error", locals: { error_string: "Zendesk timed out", ticket: ticket }
end
+ rescue GDSZendesk::ZendeskError => e
+ ExceptionNotifier::Notifier.exception_notification(request.env, e).deliver
+ render "support/zendesk_error", status: 500, locals: { error_string: e.underlying_message,
+ ticket: ticket }
end
end
@@ -1,10 +1,41 @@
<%= content_for :page_title, "Home" %>
<%= content_for :header, "Welcome to GOV.UK Support" %>
<div class="alert alert-error">
- <h2>
- <%= I18n.t error_string %>
- </h2>
+ <h3>
+ Unfortunately, your request could not be submitted to Zendesk.
+ </h3>
<p>
- Please wait and try again in a little while. Thank you!
+ This could be due to:
+
+ <ul>
+ <li>a problem with the GOV.UK support form</li>
+ <li>a problem with Zendesk</li>
+ <li>a typo or error within your request</li>
+ </ul>
</p>
+
+ <h5>
+ <a data-toggle="collapse" data-target="#problem-details">
+ <i class="icon-chevron-right hidden"></i><i class="icon-chevron-down hidden"></i>
+ The error message <small id="problem-details-toggle" class="hidden">(click to show/hide)</small>
+ </a>
+ </h5>
+
+ <div id="problem-details" class="collapse in">
+ <p>
+ <pre>
+<%= error_string %>
+ </pre>
+ </p>
+ </div>
</div>
+
+<h3>Next steps</h3>
+
+<p>
+ We have been notified of the problem and received a copy of your request. Please review it below. If needed, you can use your browser's Back button to correct and re-submit your request.
+
+ <pre>
+<%= ticket.to_s %>
+ </pre>
+<p>
View
@@ -1,6 +0,0 @@
-# Sample localization file for English. Add more files in this directory for other locales.
-# See https://github.com/svenfuchs/rails-i18n/tree/master/rails%2Flocale for starting points.
-
-en:
- zendesk_error_upon_submit: "We're sorry, we were unable to submit your ticket to our ticketing system successfully."
- zendesk_error_upon_new_form: "We're sorry, it appears we are unable to connect to our ticketing system right now."
@@ -1,5 +1,5 @@
require 'zendesk_ticket'
-require 'comment_snippet'
+require 'labelled_snippet'
class CampaignRequestZendeskTicket < ZendeskTicket
def subject
@@ -13,16 +13,16 @@ def tags
protected
def comment_snippets
[
- CommentSnippet.new(on: @request.campaign, field: :title,
+ LabelledSnippet.new(on: @request.campaign, field: :title,
label: "Campaign title"),
- CommentSnippet.new(on: @request.campaign, field: :erg_reference_number,
+ LabelledSnippet.new(on: @request.campaign, field: :erg_reference_number,
label: "ERG reference number"),
- CommentSnippet.new(on: @request.campaign, field: :start_date),
- CommentSnippet.new(on: @request.campaign, field: :description),
- CommentSnippet.new(on: @request.campaign, field: :affiliated_group_or_company),
- CommentSnippet.new(on: @request.campaign, field: :info_url,
+ LabelledSnippet.new(on: @request.campaign, field: :start_date),
+ LabelledSnippet.new(on: @request.campaign, field: :description),
+ LabelledSnippet.new(on: @request.campaign, field: :affiliated_group_or_company),
+ LabelledSnippet.new(on: @request.campaign, field: :info_url,
label: "URL with more information"),
- CommentSnippet.new(on: @request, field: :additional_comments)
+ LabelledSnippet.new(on: @request, field: :additional_comments)
]
end
end
@@ -1,5 +1,5 @@
require 'zendesk_ticket'
-require 'comment_snippet'
+require 'labelled_snippet'
class ContentChangeRequestZendeskTicket < ZendeskTicket
def subject
@@ -13,15 +13,15 @@ def tags
protected
def comment_snippets
[
- CommentSnippet.new(on: @request, field: :formatted_request_context,
+ LabelledSnippet.new(on: @request, field: :formatted_request_context,
label: "Which part of GOV.UK is this about?"),
- CommentSnippet.new(on: @request, field: :url,
+ LabelledSnippet.new(on: @request, field: :url,
label: "URL of content to be changed"),
- CommentSnippet.new(on: @request, field: :related_urls,
+ LabelledSnippet.new(on: @request, field: :related_urls,
label: "Related URLs"),
- CommentSnippet.new(on: @request, field: :details_of_change,
+ LabelledSnippet.new(on: @request, field: :details_of_change,
label: "Details of what should be added, amended or removed"),
- CommentSnippet.new(on: @request.time_constraint, field: :time_constraint_reason)
+ LabelledSnippet.new(on: @request.time_constraint, field: :time_constraint_reason)
]
end
end
@@ -1,5 +1,5 @@
require 'zendesk_ticket'
-require 'comment_snippet'
+require 'labelled_snippet'
class CreateNewUserRequestZendeskTicket < ZendeskTicket
def subject
@@ -13,17 +13,17 @@ def tags
protected
def comment_snippets
[
- CommentSnippet.new(on: @request, field: :formatted_tool_role,
+ LabelledSnippet.new(on: @request, field: :formatted_tool_role,
label: "Tool/Role"),
- CommentSnippet.new(on: @request.requested_user, field: :name,
+ LabelledSnippet.new(on: @request.requested_user, field: :name,
label: "Requested user's name"),
- CommentSnippet.new(on: @request.requested_user, field: :email,
+ LabelledSnippet.new(on: @request.requested_user, field: :email,
label: "Requested user's email"),
- CommentSnippet.new(on: @request.requested_user, field: :job,
+ LabelledSnippet.new(on: @request.requested_user, field: :job,
label: "Requested user's job title"),
- CommentSnippet.new(on: @request.requested_user, field: :phone,
+ LabelledSnippet.new(on: @request.requested_user, field: :phone,
label: "Requested user's phone number"),
- CommentSnippet.new(on: @request, field: :additional_comments)
+ LabelledSnippet.new(on: @request, field: :additional_comments)
]
end
end
@@ -1,5 +1,5 @@
require 'zendesk_ticket'
-require 'comment_snippet'
+require 'labelled_snippet'
class GeneralRequestZendeskTicket < ZendeskTicket
def subject
@@ -13,9 +13,9 @@ def tags
protected
def comment_snippets
[
- CommentSnippet.new(on: @request, field: :url),
- CommentSnippet.new(on: @request, field: :user_agent),
- CommentSnippet.new(on: @request, field: :additional)
+ LabelledSnippet.new(on: @request, field: :url),
+ LabelledSnippet.new(on: @request, field: :user_agent),
+ LabelledSnippet.new(on: @request, field: :additional)
]
end
end
@@ -1,6 +1,6 @@
require 'active_support/inflector'
-class CommentSnippet
+class LabelledSnippet
def initialize(options)
@options = options
end
@@ -1,5 +1,5 @@
require 'zendesk_ticket'
-require 'comment_snippet'
+require 'labelled_snippet'
class NewFeatureRequestZendeskTicket < ZendeskTicket
attr_reader :time_constraint
@@ -16,11 +16,11 @@ def tags
protected
def comment_snippets
[
- CommentSnippet.new(on: @request, field: :formatted_request_context,
+ LabelledSnippet.new(on: @request, field: :formatted_request_context,
label: "Which part of GOV.UK is this about?"),
- CommentSnippet.new(on: @request, field: :user_need),
- CommentSnippet.new(on: @request, field: :url_of_example),
- CommentSnippet.new(on: @request.time_constraint, field: :time_constraint_reason)
+ LabelledSnippet.new(on: @request, field: :user_need),
+ LabelledSnippet.new(on: @request, field: :url_of_example),
+ LabelledSnippet.new(on: @request.time_constraint, field: :time_constraint_reason)
]
end
end
@@ -1,5 +1,5 @@
require 'zendesk_ticket'
-require 'comment_snippet'
+require 'labelled_snippet'
class RemoveUserRequestZendeskTicket < ZendeskTicket
def subject
@@ -13,11 +13,11 @@ def tags
protected
def comment_snippets
[
- CommentSnippet.new(on: @request, field: :formatted_tool_role,
+ LabelledSnippet.new(on: @request, field: :formatted_tool_role,
label: "Tool/Role"),
- CommentSnippet.new(on: @request, field: :user_name),
- CommentSnippet.new(on: @request, field: :user_email),
- CommentSnippet.new(on: @request, field: :reason_for_removal)
+ LabelledSnippet.new(on: @request, field: :user_name),
+ LabelledSnippet.new(on: @request, field: :user_email),
+ LabelledSnippet.new(on: @request, field: :reason_for_removal)
]
end
end
View
@@ -0,0 +1,10 @@
+class SnippetCollection
+ def initialize(snippets)
+ @snippets = snippets
+ end
+
+ def to_s
+ applicable_snippets = @snippets.select(&:applies?)
+ applicable_snippets.collect(&:to_s).join("\n\n")
+ end
+end
View
@@ -1,6 +1,8 @@
require 'forwardable'
require 'date'
require 'active_support'
+require 'snippet_collection'
+require 'labelled_snippet'
class ZendeskTicket
extend Forwardable
@@ -13,8 +15,7 @@ def initialize(request)
def_delegators :@requester, :email, :collaborator_emails
def comment
- applicable_snippets = comment_snippets.select(&:applies?)
- applicable_snippets.collect(&:to_s).join("\n\n")
+ SnippetCollection.new(comment_snippets).to_s
end
def not_before_date
@@ -41,8 +42,19 @@ def inside_government_tag_if_needed
@request.inside_government_related? ? ["inside_government"] : []
end
+ def to_s
+ SnippetCollection.new(base_attribute_snippets + comment_snippets).to_s
+ end
+
private
-
+ def base_attribute_snippets
+ [
+ LabelledSnippet.new(on: @requester, field: :email, label: "Requester email"),
+ LabelledSnippet.new(on: @requester, field: :collaborator_emails),
+ LabelledSnippet.new(on: self, field: :tags),
+ ]
+ end
+
def has_value?(param, target = nil)
target ||= @request
target.respond_to?(param) and not target.send(param).blank?
@@ -58,6 +58,7 @@ class RequestsControllerTest < ActionController::TestCase
end
teardown do
+ GDS_ZENDESK_CLIENT.reset
Rails.application.reload_routes!
end
@@ -83,7 +84,7 @@ def prevent_implicit_rendering
post :create, params
end
- should "submit it to ZenDesk" do
+ should "submit it to Zendesk" do
params = valid_params_for_test_request
post :create, params
@@ -92,6 +93,18 @@ def prevent_implicit_rendering
assert_redirected_to "/acknowledge"
end
+ should "notify the user and operations if there was an error submitting to Zendesk" do
+ @zendesk_api.ticket.should_raise_error
+ params = valid_params_for_test_request
+
+ @controller.expects(:render).with("support/zendesk_error", has_entry(status: 500))
+ ExceptionNotifier::Notifier.expects(:exception_notification)
+ .with(anything, kind_of(GDSZendesk::ZendeskError))
+ .returns(stub("mailer", deliver: true))
+
+ post :create, params
+ end
+
should "set collaborators if they're set on the request" do
params = valid_params_for_test_request.tap do |p|
p["test_request"]["requester_attributes"].merge!("collaborator_emails" => "ab@c.com, def@g.com")
Oops, something went wrong.

0 comments on commit b033efb

Please sign in to comment.