Merge pull request #9 from alphagov/general_request_refactor

General request form refactoring

Gemfile

@@ -16,13 +16,15 @@ gem 'gds-sso', '2.1.0'
 gem 'jquery-rails'
 gem 'plek', '0.5.0'
 gem 'zendesk_api', '0.1.2'
-gem 'active_attr', '0.6.0'
 gem 'formtastic-bootstrap', '2.0.0'
 
 group :test do
   gem "mocha", "0.12.6", require: false
   gem "shoulda", "~> 3.3.2"
   gem "webmock", "1.8.11"
+  gem 'capybara', '1.1.2'
+  gem 'poltergeist', '0.7.0'
+  gem 'cucumber-rails', '1.3.0', :require => false
 end
 
 gem 'unicorn', '4.3.1'

Gemfile.lock

@@ -15,9 +15,6 @@ GEM
       rack-cache (~> 1.2)
       rack-test (~> 0.6.1)
       sprockets (~> 2.1.3)
-    active_attr (0.6.0)
-      activemodel (>= 3.0.2, < 4.1)
-      activesupport (>= 3.0.2, < 4.1)
     activemodel (3.2.8)
       activesupport (= 3.2.8)
       builder (~> 3.0.0)
@@ -40,6 +37,15 @@ GEM
       mime-types
       xml-simple
     builder (3.0.4)
+    capybara (1.1.2)
+      mime-types (>= 1.16)
+      nokogiri (>= 1.3.3)
+      rack (>= 1.0.0)
+      rack-test (>= 0.5.4)
+      selenium-webdriver (~> 2.0)
+      xpath (~> 0.1.4)
+    childprocess (0.3.5)
+      ffi (~> 1.0, >= 1.0.6)
     coffee-rails (3.2.2)
       coffee-script (>= 2.2.0)
       railties (~> 3.2.0)
@@ -48,14 +54,28 @@ GEM
       execjs
     coffee-script-source (1.3.3)
     crack (0.3.1)
+    cucumber (1.2.1)
+      builder (>= 2.1.2)
+      diff-lcs (>= 1.1.3)
+      gherkin (~> 2.11.0)
+      json (>= 1.4.6)
+    cucumber-rails (1.3.0)
+      capybara (>= 1.1.2)
+      cucumber (>= 1.1.8)
+      nokogiri (>= 1.5.0)
+    diff-lcs (1.1.3)
     erubis (2.7.0)
+    eventmachine (1.0.0)
     exception_notification (2.4.1)
     execjs (1.4.0)
       multi_json (~> 1.0)
     faraday (0.8.4)
       multipart-post (~> 1.1)
     faraday_middleware (0.8.8)
       faraday (>= 0.7.4, < 0.9)
+    faye-websocket (0.4.6)
+      eventmachine (>= 0.12.0)
+    ffi (1.1.5)
     formtastic (2.2.1)
       actionpack (>= 3.0)
     formtastic-bootstrap (2.0.0)
@@ -66,8 +86,11 @@ GEM
       rack-accept (~> 0.4.4)
       rails (>= 3.0.0)
       warden (~> 1.2)
+    gherkin (2.11.2)
+      json (>= 1.4.6)
     hashie (1.2.0)
     hike (1.2.1)
+    http_parser.rb (0.5.3)
     httpauth (0.2.0)
     i18n (0.6.1)
     inflection (1.0.0)
@@ -80,6 +103,8 @@ GEM
       multi_json (>= 1.0)
     kgio (2.7.4)
     libv8 (3.3.10.4)
+    libwebsocket (0.1.5)
+      addressable
     mail (2.4.4)
       i18n (>= 0.4.0)
       mime-types (~> 1.16)
@@ -90,6 +115,7 @@ GEM
       metaclass (~> 0.0.1)
     multi_json (1.3.6)
     multipart-post (1.1.5)
+    nokogiri (1.5.5)
     oauth2 (0.8.0)
       faraday (~> 0.8)
       httpauth (~> 0.1)
@@ -106,6 +132,12 @@ GEM
       omniauth (~> 1.0)
     plek (0.5.0)
       builder
+    poltergeist (0.7.0)
+      capybara (~> 1.1)
+      childprocess (~> 0.3)
+      faye-websocket (~> 0.4, >= 0.4.4)
+      http_parser.rb (~> 0.5.3)
+      multi_json (~> 1.0)
     polyglot (0.3.3)
     rack (1.4.1)
     rack-accept (0.4.5)
@@ -135,11 +167,17 @@ GEM
     rake (0.9.2.2)
     rdoc (3.12)
       json (~> 1.4)
+    rubyzip (0.9.9)
     sass (3.2.1)
     sass-rails (3.2.5)
       railties (~> 3.2.0)
       sass (>= 3.1.10)
       tilt (~> 1.3)
+    selenium-webdriver (2.25.0)
+      childprocess (>= 0.2.5)
+      libwebsocket (~> 0.1.3)
+      multi_json (~> 1.0)
+      rubyzip
     shoulda (3.3.2)
       shoulda-context (~> 1.0.1)
       shoulda-matchers (~> 1.4.1)
@@ -171,6 +209,8 @@ GEM
       addressable (>= 2.2.7)
       crack (>= 0.1.7)
     xml-simple (1.1.1)
+    xpath (0.1.4)
+      nokogiri (~> 1.3)
     zendesk_api (0.1.2)
       faraday (>= 0.8.0)
       faraday_middleware (>= 0.8.7)
@@ -184,15 +224,17 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
-  active_attr (= 0.6.0)
   aws-ses
+  capybara (= 1.1.2)
   coffee-rails (~> 3.2.1)
+  cucumber-rails (= 1.3.0)
   exception_notification (~> 2.4.1)
   formtastic-bootstrap (= 2.0.0)
   gds-sso (= 2.1.0)
   jquery-rails
   mocha (= 0.12.6)
   plek (= 0.5.0)
+  poltergeist (= 0.7.0)
   rails (= 3.2.8)
   sass-rails (~> 3.2.3)
   shoulda (~> 3.3.2)

app/controllers/general_requests_controller.rb

@@ -2,20 +2,18 @@
 
 class GeneralRequestsController <  ApplicationController
   def new
-    @request = GeneralRequest.new
-    @formdata = {}
+    @request = GeneralRequest.new(:requester => Requester.new)
     prepopulate_organisation_list
   end
 
   def create
-    params[:user_agent] = request.user_agent
-
     @request = GeneralRequest.new(params[:general_request])
+    @request.user_agent = request.user_agent
 
     load_client_and_organisations("zendesk_error_upon_submit")
 
     if @request.valid?
-      ticket = ZendeskRequest.raise_ticket(@client, GeneralRequestZendeskTicket.new(@request.attributes))
+      ticket = ZendeskRequest.raise_ticket(@client, GeneralRequestZendeskTicket.new(@request))
       if ticket
         redirect_to acknowledge_path
       else

app/models/general_request.rb

@@ -1,17 +1,17 @@
-class GeneralRequest
-  include ActiveAttr::Model
+require 'tableless_model'
+require 'requester'
 
-  attribute :name
-  attribute :email
-  attribute :job
-  attribute :phone
-  attribute :organisation
-  attribute :other_organisation
-  attribute :url
-  attribute :additional
+class GeneralRequest < TablelessModel
+  attr_accessor :requester, :url, :additional, :user_agent
 
-  validates_presence_of :name, :email, :job
-  validates_presence_of :organisation, :message => "information is required for a valid request."
-  validates :email, :format => {:with => /^[\w\d]+[^@]*@[\w\d]+[^@]*\.[\w\d]+[^@]*$/}
-  validates_presence_of :other_organisation, :if => Proc.new {|request| request.organisation == "other_organisation"}
+  validates_presence_of :requester
+  validate do |request|
+    if request.requester and not request.requester.valid?
+      errors[:base] << "Requester details are either not complete or invalid."
+    end
+  end
+
+  def requester_attributes=(attr)
+    self.requester = Requester.new(attr)
+  end
 end

app/models/requester.rb

@@ -0,0 +1,10 @@
+require 'tableless_model'
+
+class Requester < TablelessModel
+  attr_accessor :name, :email, :job, :phone, :organisation, :other_organisation
+
+  validates_presence_of :name, :email, :job
+  validates_presence_of :organisation, :message => "information is required for a valid request."
+  validates :email, :format => {:with => /^[\w\d]+[^@]*@[\w\d]+[^@]*\.[\w\d]+[^@]*$/}
+  validates_presence_of :other_organisation, :if => Proc.new {|request| request.organisation == "other_organisation"}
+end

app/models/tableless_model.rb

@@ -0,0 +1,17 @@
+class TablelessModel
+  include ActiveModel::Validations
+  include ActiveModel::Conversion  
+  extend ActiveModel::Naming
+
+  def initialize(attributes = {})
+    attributes.each do |key, value|
+      if respond_to? "#{key}="
+        send("#{key}=", value)
+      end
+    end
+  end
+
+  def persisted?  
+    false  
+  end
+end

app/views/general_requests/new.html.erb

@@ -8,7 +8,7 @@
 <div class="well">
   <%= semantic_form_for @request, :url => { :action => "create" }, :html => { :novalidate => false} do |f| %>
 
-    <%= render :partial => "support/originator", :locals => {:f => f} %>
+    <%= render :partial => "support/requester", :locals => {:f => f} %>
 
     <%= f.inputs "Details of your problem, request or suggestion" do %>
       <%= f.input :additional, :as => :text, :label => "Details", :input_html => {:class => "span6", :rows => 6, :cols => 50 } %>

app/views/support/_requester.html.erb

@@ -0,0 +1,8 @@
+<%= f.semantic_fields_for :requester, :label => "Your details" do |r| %>
+  <%= r.input :name, :label => "Name", :required => true, :input_html => {:"aria-required" => true, :class => "span6"} %>
+  <%= r.input :email, :label => "Email", :as => :email, :required => true, :input_html => {:"aria-required" => true, :class => "span6"} %>
+  <%= r.input :job, :label => "Job title", :required => true, :input_html => {:"aria-required" => true, :class => "span6"} %>
+  <%= r.input :phone, :label => "Phone number", :as => :phone, :input_html => {:"aria-required" => true, :class => "span6"} %>
+  <%= r.input :organisation, :label => "Organisation", :as => :select, :collection => @organisations, :include_blank => "Select Organisation", :input_html => {:"aria-required" => true, :class => "span6"} %>
+  <%= r.input :other_organisation, :label => "Please specify the organisation if you didn't find it in the list above", :input_html => {:class => "span6"} %>
+<% end %>

config/cucumber.yml

@@ -0,0 +1,8 @@
+<%
+rerun = File.file?('rerun.txt') ? IO.read('rerun.txt') : ""
+rerun_opts = rerun.to_s.strip.empty? ? "--format #{ENV['CUCUMBER_FORMAT'] || 'progress'} features" : "--format #{ENV['CUCUMBER_FORMAT'] || 'pretty'} #{rerun}"
+std_opts = "--format #{ENV['CUCUMBER_FORMAT'] || 'pretty'} --strict --tags ~@wip"
+%>
+default: <%= std_opts %> features
+wip: --tags @wip:3 --wip features
+rerun: <%= rerun_opts %> --format rerun --out rerun.txt --strict --tags ~@wip

features/general_requests.feature

@@ -0,0 +1,26 @@
+Feature: General requests
+  In order to provide GDS with feedback
+  As a government employee
+  I want a means to contact GDS in the cases when my request does not covered by the other forms
+
+  Background:
+    * the following user has SSO access:
+      | Name         | Email                | Job title | Organisation   |
+      | John Smith   | john.smith@email.com | Developer | Cabinet Office |
+
+  Scenario: successful request
+    When the user submits the following general request:
+      | Details          | URL               |
+      | The site is down | http://www.gov.uk |
+    Then the following ticket is raised in ZenDesk:
+      | Subject                   | Requester email      | Requester name | Job title | Organisation   |
+      | Govt Agency General Issue | john.smith@email.com | John Smith     | Developer | cabinet_office |
+    And the ticket is tagged with "govt_agency_general"
+    And the comment on the ticket is:
+      """
+      [Url]
+      http://www.gov.uk
+
+      [Additional]
+      The site is down
+      """

features/step_definitions/general_request_steps.rb

@@ -0,0 +1,19 @@
+When /^the user submits the following general request:$/ do |request_details_table|
+  request_details = request_details_table.hashes.first
+
+  visit '/'
+
+  click_on "General"
+
+  assert page.has_content?("Report a problem")
+
+  fill_in "Name", :with => @user_details["Name"]
+  fill_in "Email", :with => @user_details["Email"]
+  fill_in "Job title", :with => @user_details["Job title"]
+  select @user_details["Organisation"], :from => 'Organisation'
+
+  fill_in "Details", :with => request_details['Details']
+  fill_in "URL (if applicable)", :with => request_details['URL']
+
+  click_on "Submit"
+end

features/step_definitions/user_steps.rb

@@ -0,0 +1,6 @@
+Given /^the following user has SSO access:$/ do |user_details|
+  user = stub_everything('user', :name => "user", :has_permission? => true)
+  @user_details = user_details.hashes.first
+
+  login_as user
+end

features/step_definitions/zendesk_steps.rb

@@ -0,0 +1,18 @@
+Then /^the following ticket is raised in ZenDesk:$/ do |ticket_properties_table|
+  expected_ticket_props = ticket_properties_table.hashes.first
+  @raised_ticket = @zendesk_api.ticket
+
+  assert_equal expected_ticket_props["Subject"],         @raised_ticket.subject
+  assert_equal expected_ticket_props["Requester email"], @raised_ticket.email
+  assert_equal expected_ticket_props["Requester name"],  @raised_ticket.name
+  assert_equal expected_ticket_props["Job title"],       @raised_ticket.job
+  assert_equal expected_ticket_props["Organisation"],    @raised_ticket.organisation
+end
+
+Then /^the ticket is tagged with "(.*?)"$/ do |expected_tags|
+  assert_equal expected_tags, @raised_ticket.tags.join(" ")
+end
+
+Then /^the comment on the ticket is:$/ do |expected_comment_string|
+  assert_equal expected_comment_string, @raised_ticket.comment
+end

features/support/capybara.rb

@@ -0,0 +1,9 @@
+require 'capybara/poltergeist'
+
+Capybara.register_driver :rack_test do |app|
+  Capybara::RackTest::Driver.new(app, :browser => :chrome)
+end
+Capybara.register_driver :poltergeist do |app|
+  Capybara::Poltergeist::Driver.new(app, timeout: 5, debug: true)
+end
+Capybara.javascript_driver = :poltergeist