Permalink
Browse files

Merge pull request #134 from alphagov/service-feedback-in-feedex

Service feedback in feedex
  • Loading branch information...
bradwright committed Feb 7, 2014
2 parents 9e21ad4 + abb5411 commit d0d1afe2ea7dbe9da2e74cb2826235d669ffa3c9
Showing with 317 additions and 204 deletions.
  1. +1 −0 Gemfile
  2. +2 −0 Gemfile.lock
  3. +2 −2 app/controllers/anonymous_feedback/{problem_reports → }/explore_controller.rb
  4. +0 −17 app/controllers/anonymous_feedback/problem_reports_controller.rb
  5. +33 −0 app/controllers/anonymous_feedback_controller.rb
  6. +3 −3 app/helpers/application_helper.rb
  7. +20 −0 app/views/anonymous_feedback/_anonymous-contact.html.erb
  8. +13 −0 app/views/anonymous_feedback/_feedback.html.erb
  9. 0 app/views/anonymous_feedback/{problem_reports → }/explore/new.html.erb
  10. +1 −1 app/views/anonymous_feedback/{problem_reports → }/index.html.erb
  11. +0 −18 app/views/anonymous_feedback/problem_reports/_problem-report.html.erb
  12. +9 −3 config/routes.rb
  13. +18 −8 features/anonymous_feedback.feature
  14. +3 −3 features/feedex.feature
  15. +1 −0 features/step_definitions/anonymous_feedback_steps.rb
  16. +1 −1 features/step_definitions/feedex_steps.rb
  17. +10 −0 features/step_definitions/temporal_steps.rb
  18. +1 −1 lib/support/permissions/ability.rb
  19. +12 −0 lib/support/requests/anonymous/anonymous_contact.rb
  20. +0 −8 lib/support/requests/anonymous/problem_report.rb
  21. +19 −0 test/functional/anonymous_feedback/explore_controller_test.rb
  22. +0 −21 test/functional/anonymous_feedback/problem_reports/explore_controller_test.rb
  23. +0 −68 test/functional/anonymous_feedback/problem_reports_controller_test.rb
  24. +108 −0 test/functional/anonymous_feedback_controller_test.rb
  25. +15 −0 test/integration/redirects_test.rb
  26. +45 −5 test/unit/support/requests/anonymous/anonymous_contact_test.rb
  27. +0 −45 test/unit/support/requests/anonymous/problem_report_test.rb
View
@@ -48,6 +48,7 @@ group :test do
gem 'capybara', '1.1.2'
gem 'poltergeist', '0.7.0'
gem 'cucumber-rails', '1.3.0', :require => false
+ gem 'timecop', '0.7.1'
end
gem 'unicorn', '4.3.1'
View
@@ -251,6 +251,7 @@ GEM
ref
thor (0.18.1)
tilt (1.4.1)
+ timecop (0.7.1)
timeliness (0.3.7)
timers (1.1.0)
treetop (1.4.15)
@@ -315,6 +316,7 @@ DEPENDENCIES
sidekiq (= 2.17.1)
statsd-ruby (= 1.2.1)
therubyracer (= 0.12.0)
+ timecop (= 0.7.1)
uglifier (>= 1.0.3)
unicorn (= 4.3.1)
validates_timeliness (= 3.0.14)
@@ -1,6 +1,6 @@
require 'support/requests/anonymous/explore'
-class AnonymousFeedback::ProblemReports::ExploreController < AuthorisationController
+class AnonymousFeedback::ExploreController < AuthorisationController
authorize_resource class: Support::Requests::Anonymous::Explore
def new
@@ -10,7 +10,7 @@ def new
def create
@explore = Support::Requests::Anonymous::Explore.new(params[:support_requests_anonymous_explore])
if @explore.valid?
- redirect_to anonymous_feedback_problem_reports_url(path: @explore.path)
+ redirect_to anonymous_feedback_index_url(path: @explore.path)
else
render :new, status: 400
end
@@ -4,23 +4,6 @@
class AnonymousFeedback::ProblemReportsController < AnonymousFeedbackController
include Support::Requests
- def index
- authorize! :read, Anonymous::ProblemReport
-
- if params[:path].nil? or params[:path].empty?
- respond_to do |format|
- format.html { redirect_to anonymous_feedback_problem_reports_explore_url, status: 301 }
- format.json { render json: {"errors" => ["Please set a valid 'path' parameter"] }, status: 400 }
- end
- else
- @feedback = Anonymous::ProblemReport.find_all_starting_with_path(params[:path])
- respond_to do |format|
- format.html { render :index }
- format.json { render json: @feedback.as_json(root:false, only: [:what_wrong, :what_doing, :created_at, :url, :referrer]) }
- end
- end
- end
-
protected
def new_request
Anonymous::ProblemReport.new
@@ -1,4 +1,37 @@
class AnonymousFeedbackController < RequestsController
+ include Support::Requests
+
+ def index
+ authorize! :read, Anonymous::AnonymousContact
+
+ if params[:path].nil? or params[:path].empty?
+ respond_to do |format|
+ format.html { redirect_to anonymous_feedback_explore_url, status: 301 }
+ format.json { render json: {"errors" => ["Please set a valid 'path' parameter"] }, status: 400 }
+ end
+ else
+ @feedback = Anonymous::AnonymousContact.find_all_starting_with_path(params[:path])
+ respond_to do |format|
+ format.html { render :index }
+ format.json {
+ render json: @feedback.as_json(
+ root: false,
+ only: [
+ :what_wrong,
+ :what_doing,
+ :created_at,
+ :url,
+ :referrer,
+ :slug,
+ :details,
+ :service_satisfaction_rating
+ ]
+ )
+ }
+ end
+ end
+ end
+
def set_requester_on(request)
# this is anonymous feedback, so requester doesn't need to be set
end
@@ -21,12 +21,12 @@ def request_groups
end
def feedex_nav_link
- is_on_feedex_page = current_page?(controller: "anonymous_feedback/problem_reports/explore", action: :new) ||
- current_page?(controller: 'anonymous_feedback/problem_reports', action: :index)
+ is_on_feedex_page = current_page?(controller: "anonymous_feedback/explore", action: :new) ||
+ current_page?(controller: 'anonymous_feedback', action: :index)
class_name = is_on_feedex_page ? 'active' : ''
content_tag(:li, class: class_name, id: "feedex") do
- link_to "Feedback explorer", anonymous_feedback_problem_reports_explore_url
+ link_to "Feedback explorer", anonymous_feedback_explore_url
end
end
end
@@ -0,0 +1,20 @@
+<tr class="anonymous-contact">
+ <th scope="row"><%= anonymous_contact.created_at.strftime("%d.%m.%Y") %></th>
+ <td>
+ <%= render partial: "feedback", locals: { anonymous_contact: anonymous_contact } %>
+ </td>
+ <td>
+ <% if anonymous_contact.path %>
+ <%= link_to anonymous_contact.path, anonymous_contact.url %>
+ <% else %>
+ &ndash;
+ <% end %>
+ </td>
+ <td>
+ <% if anonymous_contact.referrer %>
+ <%= link_to anonymous_contact.referrer, anonymous_contact.referrer %>
+ <% else %>
+ &ndash;
+ <% end %>
+ </td>
+</tr>
@@ -0,0 +1,13 @@
+<% if anonymous_contact.is_a?(Support::Requests::Anonymous::ProblemReport) %>
+ <strong>action</strong>: <%= anonymous_contact.what_doing %>
+ <br/>
+ <strong>problem</strong>: <%= anonymous_contact.what_wrong %>
+<% elsif anonymous_contact.is_a?(Support::Requests::Anonymous::ServiceFeedback) %>
+ <strong>rating</strong>: <%= anonymous_contact.service_satisfaction_rating %>
+ <% unless anonymous_contact.details.empty? %>
+ <br/>
+ <strong>comment</strong>: <%= anonymous_contact.details %>
+ <% end %>
+<% else %>
+ <%= anonymous_contact.details %>
+<% end %>
@@ -11,6 +11,6 @@
<th>user came from</th>
</thead>
<tbody>
- <%= render partial: "problem-report", collection: @feedback, as: :problem_report %>
+ <%= render partial: "anonymous-contact", collection: @feedback, as: :anonymous_contact %>
</tbody>
</table>
@@ -1,18 +0,0 @@
-<tr class="problem-report">
- <td><%= problem_report.created_at.strftime("%d.%m.%Y") %></td>
- <td>
- <strong>action</strong>: <%= problem_report.what_doing %>
- <br/>
- <strong>problem</strong>: <%= problem_report.what_wrong %>
- </td>
- <td>
- <%= link_to URI(problem_report.url).path, problem_report.url %>
- </td>
- <td>
- <% if problem_report.referrer %>
- <%= link_to problem_report.referrer, problem_report.referrer %>
- <% else %>
- &ndash;
- <% end %>
- </td>
-</tr>
View
@@ -9,16 +9,22 @@
resources :named_contacts, only: :create
namespace :anonymous_feedback do
- resources :problem_reports, only: [ :create, :index ], format: false
+ resources :problem_reports, only: :create, format: false
resources :long_form_contacts, only: :create
resources :service_feedback, only: :create
+ get 'problem_reports', to: redirect {|p, req| req.params[:path] ? "/anonymous_feedback?path=" + req.params[:path] : "/anonymous_feedback"}
+
namespace :problem_reports do
- get :explore, to: "explore#new", format: false
- post :explore, to: "explore#create", format: false
+ get :explore, to: redirect("/anonymous_feedback/explore"), format: false
end
+
+ get :explore, to: "explore#new", format: false
+ post :explore, to: "explore#create", format: false
end
+ resources :anonymous_feedback, only: :index, format: false
+
match "acknowledge" => "support#acknowledge"
match "_status" => "support#queue_status"
root to: 'support#landing'
@@ -53,15 +53,25 @@ Feature: Anonymous feedback
"""
Scenario: successful service feedback submission with comment
+ Given the date is 2013-02-28
When the user submits feedback about a GOV.UK service through the API:
- | Slug | Satisfaction rating | Improvement comments | User agent | JS? |
- | done/find-court-tribunal | 3 | Make service less 'meh' | Safari | yes |
- # Then TBD...
- # for now, we store the feedback and expose it later through an API
+ | Slug | URL | Satisfaction rating | Improvement comments | User agent | JS? |
+ | done/find-court-tribunal | https://www.gov.uk/done/find-court-tribunal | 3 | Make service less 'meh' | Safari | yes |
+ And the user explores the feedback with the following filters:
+ | URL |
+ | https://www.gov.uk/done/find-court-tribunal |
+ Then the following result is shown:
+ | creation date | feedback | full path | user came from |
+ | 28.02.2013 | rating: 3\n comment: Make service less 'meh' | /done/find-court-tribunal | – |
Scenario: successful service feedback submission without comment
+ Given the date is 2013-02-28
When the user submits feedback about a GOV.UK service through the API:
- | Slug | Satisfaction rating | Improvement comments | User agent | JS? |
- | done/find-court-tribunal | 3 | | Safari | yes |
- # Then TBD...
- # for now, we store the feedback and expose it later through an API
+ | Slug | URL | Satisfaction rating | Improvement comments | User agent | JS? |
+ | done/find-court-tribunal | https://www.gov.uk/done/find-court-tribunal | 3 | | Safari | yes |
+ And the user explores the feedback with the following filters:
+ | URL |
+ | https://www.gov.uk/done/find-court-tribunal |
+ Then the following result is shown:
+ | creation date | feedback | full path | user came from |
+ | 28.02.2013 | rating: 3 | /done/find-court-tribunal | – |
View
@@ -19,8 +19,8 @@ Feature: Feedback explorer
| https://www.gov.uk/vat-rates |
Then the following result is shown:
| creation date | feedback | full path | user came from |
- | 01.03.2013 | action: looking at 3rd paragraph\n problem: typo in 2rd word | /vat-rates | https://www.gov.uk |
- | 01.02.2013 | action: looking at rates\n problem: standard rate is wrong | /vat-rates | https://www.gov.uk/pay-vat |
+ | 01.03.2013 | action: looking at 3rd paragraph\n problem: typo in 2rd word | /vat-rates | https://www.gov.uk |
+ | 01.02.2013 | action: looking at rates\n problem: standard rate is wrong | /vat-rates | https://www.gov.uk/pay-vat |
Scenario: explore doesn't return reports with email or national insurance numbers
Given the following problem reports have been left by members of the public:
@@ -33,4 +33,4 @@ Feature: Feedback explorer
| https://www.gov.uk/vat-rates |
Then the following result is shown:
| creation date | feedback | full path | user came from |
- | 01.03.2013 | action: looking at 3rd paragraph\n problem: typo in 2rd word | /vat-rates | https://www.gov.uk |
+ | 01.03.2013 | action: looking at 3rd paragraph\n problem: typo in 2rd word | /vat-rates | https://www.gov.uk |
@@ -43,6 +43,7 @@
params = {
"service_feedback" => {
"slug" => @request_details['Slug'],
+ "url" => @request_details['URL'],
"improvement_comments" => @request_details['Improvement comments'],
"service_satisfaction_rating" => @request_details['Satisfaction rating'].to_i,
"user_agent" => @request_details['User agent'],
@@ -1,7 +1,7 @@
require 'date'
Before do
- Support::Requests::Anonymous::ProblemReport.delete_all
+ Support::Requests::Anonymous::AnonymousContact.delete_all
end
Given /^the following problem reports have been left by members of the public:$/ do |table|
@@ -0,0 +1,10 @@
+require 'timecop'
+require 'date'
+
+Given /^the date is (.+)$/ do |time|
+ Timecop.travel Date.parse(time)
+end
+
+After do
+ Timecop.return
+end
@@ -15,7 +15,7 @@ def initialize(user)
can :create, [ CreateOrChangeUserRequest, RemoveUserRequest ] if user.has_permission?('user_managers')
can :create, [ FoiRequest, Anonymous::ProblemReport, Anonymous::LongFormContact, Anonymous::ServiceFeedback, NamedContact ] if user.has_permission?('api_users')
- can :read, Anonymous::ProblemReport if user.has_permission?('feedex')
+ can :read, Anonymous::AnonymousContact if user.has_permission?('feedex')
can :create, Support::Requests::Anonymous::Explore if user.has_permission?('feedex')
can :create, [GeneralRequest, AnalyticsRequest, TechnicalFaultReport, UnpublishContentRequest]
end
@@ -22,6 +22,18 @@ def self.free_of_personal_info
where(personal_information_status: "absent")
end
+ def path
+ URI(url).path
+ rescue ArgumentError
+ nil
+ rescue URI::InvalidURIError
+ nil
+ end
+
+ def self.find_all_starting_with_path(path)
+ where("url is not null and url like ?", "%" + path + "%").free_of_personal_info.order("created_at desc").select { |pr| pr.path.start_with?(path) }
+ end
+
private
def detect_personal_information
self.personal_information_status ||= personal_info_present? ? "suspected" : "absent"
@@ -13,17 +13,9 @@ class ProblemReport < AnonymousContact
validates :what_doing, length: { maximum: 2 ** 16 }
validates :what_wrong, length: { maximum: 2 ** 16 }
- def path
- URI(url).path
- end
-
def referrer_url_on_gov_uk?
referrer and URI.parse(referrer).host == "www.gov.uk"
end
-
- def self.find_all_starting_with_path(path)
- where("url like ?", "%" + path + "%").free_of_personal_info.order("created_at desc").select { |pr| pr.path.start_with?(path) }
- end
end
end
end
@@ -0,0 +1,19 @@
+require "test_helper"
+
+module AnonymousFeedback
+ class ExploreControllerTest < ActionController::TestCase
+ context "new request" do
+ should "return show the new form again for invalid requests" do
+ post :create, { support_requests_anonymous_explore: { by_url: "abc" } }
+
+ assert_response 400
+ end
+
+ should "redirect to the problem reports index page for successful requests" do
+ post :create, { support_requests_anonymous_explore: { by_url: "https://www.gov.uk/tax-disc" } }
+
+ assert_redirected_to "/anonymous_feedback?path=%2Ftax-disc"
+ end
+ end
+ end
+end
@@ -1,21 +0,0 @@
-require "test_helper"
-
-module AnonymousFeedback
- module ProblemReports
- class ExploreControllerTest < ActionController::TestCase
- context "new request" do
- should "return show the new form again for invalid requests" do
- post :create, { support_requests_anonymous_explore: { by_url: "abc" } }
-
- assert_response 400
- end
-
- should "redirect to the problem reports index page for successful requests" do
- post :create, { support_requests_anonymous_explore: { by_url: "https://www.gov.uk/tax-disc" } }
-
- assert_redirected_to "/anonymous_feedback/problem_reports?path=%2Ftax-disc"
- end
- end
- end
- end
-end
Oops, something went wrong.

0 comments on commit d0d1afe

Please sign in to comment.