Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

reinsert removing_need_to_specify_requester_email

Revert "Revert "Merge pull request #30 from alphagov/removing_need_to_specify_requester_email""

This reverts commit 156bb70.

Conflicts:

	Gemfile.lock
	test/test_helper.rb
  • Loading branch information...
commit 51e6830fe4459e0ea268e2aa8c697a444d0a4383 1 parent a9e52ec
Jake Benilov benilovj authored
Showing with 183 additions and 71 deletions.
  1. +1 −0  Gemfile
  2. +18 −0 Gemfile.lock
  3. +1 −0  app/assets/javascripts/application.js
  4. +1 −1  app/controllers/create_new_user_requests_controller.rb
  5. +1 −1  app/controllers/remove_user_requests_controller.rb
  6. +8 −0 app/controllers/requests_controller.rb
  7. +0 −23 app/models/read_only_user.rb
  8. +3 −1 app/models/shared/requester.rb
  9. +38 −0 app/models/user.rb
  10. +1 −1  app/views/campaign_requests/new.html.erb
  11. +1 −1  app/views/content_change_requests/new.html.erb
  12. +0 −2  app/views/create_new_user_requests/new.html.erb
  13. +1 −1  app/views/general_requests/new.html.erb
  14. +18 −0 app/views/layouts/application.html.erb
  15. +1 −1  app/views/new_feature_requests/new.html.erb
  16. +0 −2  app/views/remove_user_requests/new.html.erb
  17. +3 −0  app/views/support/_collaborators.html.erb
  18. +0 −6 app/views/support/_requester.html.erb
  19. +2 −0  config/application.rb
  20. +1 −1  config/initializers/gds-sso.rb
  21. +1 −1  features/general_requests.feature
  22. +2 −2 features/remove_user_requests.feature
  23. +0 −16 features/step_definitions/request_steps.rb
  24. +3 −2 features/step_definitions/user_steps.rb
  25. +2 −1  lib/zendesk_ticket.rb
  26. +1 −1  lib/zendesk_tickets.rb
  27. +13 −1 test/functional/requests_controller_test.rb
  28. +16 −1 test/functional/support_controller_test.rb
  29. +4 −4 test/test_helper.rb
  30. +2 −0  test/unit/models/requester_test.rb
  31. +38 −0 test/unit/models/user_test.rb
  32. +2 −1  test/unit/zendesk/zendesk_ticket_test.rb
1  Gemfile
View
@@ -24,6 +24,7 @@ if ENV['GDS_ZENDESK_DEV']
else
gem "gds_zendesk", '0.0.5'
end
+gem 'redis-rails', '3.2.3'
group :test do
gem "mocha", "0.12.6", require: false
18 Gemfile.lock
View
@@ -182,6 +182,23 @@ GEM
rake (10.0.3)
rdoc (3.12.1)
json (~> 1.4)
+ redis (3.0.2)
+ redis-actionpack (3.2.3)
+ actionpack (~> 3.2.3)
+ redis-rack (~> 1.4.0)
+ redis-store (~> 1.1.0)
+ redis-activesupport (3.2.3)
+ activesupport (~> 3.2.3)
+ redis-store (~> 1.1.0)
+ redis-rack (1.4.2)
+ rack (~> 1.4.1)
+ redis-store (~> 1.1.0)
+ redis-rails (3.2.3)
+ redis-actionpack (~> 3.2.3)
+ redis-activesupport (~> 3.2.3)
+ redis-store (~> 1.1.0)
+ redis-store (1.1.3)
+ redis (>= 2.2.0)
rubyzip (0.9.9)
sass (3.2.5)
sass-rails (3.2.6)
@@ -258,6 +275,7 @@ DEPENDENCIES
plek (= 1.1.0)
poltergeist (= 0.7.0)
rails (= 3.2.12)
+ redis-rails (= 3.2.3)
sass-rails (~> 3.2.3)
shoulda (~> 3.3.2)
therubyracer (~> 0.9.4)
1  app/assets/javascripts/application.js
View
@@ -13,6 +13,7 @@
//= require jquery
//= require jquery_ujs
//= require jquery.ui.datepicker
+//= require twitter/bootstrap/dropdown
//= require twitter/bootstrap/collapse
//= require_tree .
2  app/controllers/create_new_user_requests_controller.rb
View
@@ -5,7 +5,7 @@
class CreateNewUserRequestsController < RequestsController
protected
def new_request
- CreateNewUserRequest.new(requester: Requester.new, requested_user: RequestedUser.new)
+ CreateNewUserRequest.new(requested_user: RequestedUser.new)
end
def zendesk_ticket_class
2  app/controllers/remove_user_requests_controller.rb
View
@@ -3,7 +3,7 @@
class RemoveUserRequestsController < RequestsController
protected
def new_request
- RemoveUserRequest.new(:requester => Requester.new, :time_constraint => TimeConstraint.new)
+ RemoveUserRequest.new(time_constraint: TimeConstraint.new)
end
def zendesk_ticket_class
8 app/controllers/requests_controller.rb
View
@@ -8,6 +8,8 @@ def new
def create
@request = parse_request_from_params
+ set_logged_in_user_as_requester_on(@request)
+
if @request.valid?
process_valid_request(@request)
else
@@ -21,6 +23,12 @@ def process_valid_request(submitted_request)
end
private
+ def set_logged_in_user_as_requester_on(request)
+ request.requester ||= Requester.new
+ request.requester.name = current_user.name
+ request.requester.email = current_user.email
+ end
+
def raise_ticket(ticket)
ticket = ZendeskTickets.new(GDS_ZENDESK_CLIENT).raise_ticket(ticket)
23 app/models/read_only_user.rb
View
@@ -1,23 +0,0 @@
-require 'gds-sso/user'
-
-class ReadOnlyUser < OpenStruct
- def self.attr_accessible(*args)
- end
-
- include GDS::SSO::User
-
- def self.find_by_uid(uid)
- ReadOnlyUser.new(uid: uid)
- end
-
- def self.create!(auth_hash, options={})
- ReadOnlyUser.new(auth_hash)
- end
-
- def update_attribute(*args)
- end
-
- def update_attributes(params, hash)
- ReadOnlyUser.new(params)
- end
-end
4 app/models/shared/requester.rb
View
@@ -1,12 +1,14 @@
require 'shared/tableless_model'
class Requester < TablelessModel
- attr_accessor :email
+ attr_accessor :email, :name
validates_presence_of :email
validates :email, format: { with: /@/ }
+ validates_presence_of :name
+
validate :collaborator_emails_are_all_valid
def email=(new_email)
38 app/models/user.rb
View
@@ -0,0 +1,38 @@
+require 'gds-sso/user'
+
+class User < OpenStruct
+ def self.attr_accessible(*args)
+ end
+
+ include GDS::SSO::User
+
+ def self.find_by_uid(uid)
+ auth_hash = Rails.cache.fetch(uid)
+ auth_hash ? User.new(auth_hash) : nil
+ end
+
+ def self.create!(auth_hash, options={})
+ Rails.cache.write(auth_hash["uid"], auth_hash)
+ User.new(auth_hash)
+ end
+
+ def remotely_signed_out?
+ remotely_signed_out
+ end
+
+ def update_attribute(key, value)
+ if uid
+ old_attributes = Rails.cache.fetch(uid)
+ new_attributes = old_attributes.merge(key => value)
+ Rails.cache.write(new_attributes["uid"], new_attributes)
+ end
+ send("#{key}=", value)
+ end
+
+ def update_attributes(params, hash)
+ params.each do |key, value|
+ send("#{key}=", value)
+ end
+ Rails.cache.write(params["uid"], params)
+ end
+end
2  app/views/campaign_requests/new.html.erb
View
@@ -4,7 +4,7 @@
<div class="well">
<%= semantic_form_for @request, url: { action: "create" }, html: { novalidate: false } do |f| %>
- <%= render partial: "support/requester", locals: { f: f, show_collaborators: true } %>
+ <%= render partial: "support/collaborators", locals: { f: f } %>
<%= render partial: "campaign_details", locals: { f: f } %>
2  app/views/content_change_requests/new.html.erb
View
@@ -21,7 +21,7 @@
<div class="well">
<%= semantic_form_for @request, url: { action: "create" }, html: { novalidate: false } do |f| %>
- <%= render partial: "support/requester", locals: { f: f, show_collaborators: true } %>
+ <%= render partial: "support/collaborators", locals: { f: f } %>
<%= render partial: "request_details", locals: { f: f} %>
2  app/views/create_new_user_requests/new.html.erb
View
@@ -4,8 +4,6 @@
<div class="well">
<%= semantic_form_for @request, url: { action: "create" }, html: { novalidate: false } do |f| %>
- <%= render partial: "support/requester", locals: { f: f, show_collaborators: false } %>
-
<%= render partial: "request_details", locals: { f: f } %>
<%= f.action :submit, label: "Submit", button_html: { class: "btn btn-success" } %>
2  app/views/general_requests/new.html.erb
View
@@ -12,7 +12,7 @@
<div class="well">
<%= semantic_form_for @request, url: { action: "create" }, html: { novalidate: false } do |f| %>
- <%= render partial: "support/requester", locals: { f: f, show_collaborators: true } %>
+ <%= render partial: "support/collaborators", locals: { f: f } %>
<%= render partial: "request_details", locals: { f: f } %>
18 app/views/layouts/application.html.erb
View
@@ -16,6 +16,24 @@
<div class="navbar-inner">
<div class="container-fluid">
<%= link_to 'GOV.UK Support', root_path, :class => "brand" %>
+ <ul class="nav pull-right">
+ <li class="dropdown">
+ <a href="#"
+ class="dropdown-toggle"
+ data-toggle="dropdown">
+ Services
+ <b class="caret"></b>
+ </a>
+ <ul class="dropdown-menu">
+ <li><a href="http://digital.cabinetoffice.gov.uk/">GDS blog</a></li>
+ <li><a href="http://wiki.digital.cabinet-office.gov.uk/">GDS wiki</a></li>
+ <li class="divider"></li>
+ <li><%= link_to 'Sign out', gds_sign_out_path %></li>
+ </ul>
+ </li>
+ </ul>
+
+ <p id="logged-in-user" class="navbar-text pull-right">Signed in as <a href="<%= Plek.current.find('signon') %>"><%= current_user.name %></a></p>
</div>
</div>
</header>
2  app/views/new_feature_requests/new.html.erb
View
@@ -4,7 +4,7 @@
<div class="well">
<%= semantic_form_for @request, url: { action: "create" }, html: { novalidate: false } do |f| %>
- <%= render partial: "support/requester", locals: { f: f, show_collaborators: true } %>
+ <%= render partial: "support/collaborators", locals: { f: f } %>
<%= render partial: "request_details", locals: { f: f } %>
2  app/views/remove_user_requests/new.html.erb
View
@@ -4,8 +4,6 @@
<div class="well">
<%= semantic_form_for @request, url: { action: "create" }, html: { novalidate: false } do |f| %>
- <%= render partial: "support/requester", locals: { f: f, show_collaborators: false } %>
-
<%= render partial: "request_details", locals: { f: f } %>
<%= f.action :submit, label: "Submit", button_html: { class: "btn btn-success" } %>
3  app/views/support/_collaborators.html.erb
View
@@ -0,0 +1,3 @@
+<%= f.semantic_fields_for :requester do |r| %>
+ <%= r.input :collaborator_emails, label: "Should anybody else be copied in on this request? (comma-separated list of emails)", required: false, input_html: { class: "span6", value: r.object.collaborator_emails.join(", ") } %>
+<% end %>
6 app/views/support/_requester.html.erb
View
@@ -1,6 +0,0 @@
-<%= f.semantic_fields_for :requester do |r| %>
- <%= r.input :email, label: "Your email", as: :email, required: true, input_html: { :"aria-required" => true, :class => "span6" } %>
- <% if show_collaborators %>
- <%= r.input :collaborator_emails, label: "Should anybody else be copied in on this request? (comma-separated list of emails)", required: false, input_html: { class: "span6", value: r.object.collaborator_emails.join(", ") } %>
- <% end %>
-<% end %>
2  config/application.rb
View
@@ -49,6 +49,8 @@ class Application < Rails::Application
# Enable escaping HTML in JSON.
config.active_support.escape_html_entities_in_json = true
+ config.cache_store = :redis_store
+
# Use SQL instead of Active Record's schema dumper when creating the database.
# This is necessary if your schema can't be completely dumped by the schema dumper,
# like if you have constraints or database-specific column types
2  config/initializers/gds-sso.rb
View
@@ -1,5 +1,5 @@
GDS::SSO.config do |config|
- config.user_model = 'ReadOnlyUser'
+ config.user_model = 'User'
config.oauth_id = 'abcdefghjasndjkasndsupport'
config.oauth_secret = 'secret'
config.default_scope = "Support"
2  features/general_requests.feature
View
@@ -14,7 +14,7 @@ Feature: General requests
| The site is down | https://www.gov.uk |
Then the following ticket is raised in ZenDesk:
| Subject | Requester email | Requester name |
- | Govt Agency General Issue | john.smith@email.com | john.smith@email.com |
+ | Govt Agency General Issue | john.smith@email.com | John Smith |
And the ticket is tagged with "govt_form govt_agency_general"
And the comment on the ticket is:
"""
4 features/remove_user_requests.feature
View
@@ -5,8 +5,8 @@ Feature: Remove user requests
Background:
* the following user has SSO access:
- | Name | Email | Job title | Phone |
- | John Smith | john.smith@email.com | Developer | 12345 |
+ | Name | Email |
+ | John Smith | john.smith@email.com |
Scenario: successful remove user request for publisher
When the user submits the following remove user request:
16 features/step_definitions/request_steps.rb
View
@@ -1,7 +1,3 @@
-When /^the user fills out their details$/ do
- fill_in "Your email", :with => @user_details["Email"]
-end
-
When /^the user fills out the time constraints$/ do
fill_in "MUST be published by", :with => @request_details["Needed by date"]
fill_in "MUST NOT be published BEFORE", :with => @request_details["Not before date"]
@@ -22,8 +18,6 @@
assert page.has_content?("Report a problem")
- step "the user fills out their details"
-
fill_in "Details", :with => @request_details['Details']
fill_in "URL (if applicable)", :with => @request_details['URL']
@@ -39,8 +33,6 @@
assert page.has_content?("Request a new feature/need")
- step "the user fills out their details"
-
within "#request-context" do
choose @request_details["Context"]
end
@@ -61,8 +53,6 @@
assert page.has_content?("Request a change")
- step "the user fills out their details"
-
within "#request-context" do
choose @request_details["Context"]
end
@@ -85,8 +75,6 @@
assert page.has_content?("Create a new user account")
- step "the user fills out their details"
-
within "#tool-role-choice" do
choose @request_details["Tool/Role"]
end
@@ -112,8 +100,6 @@
assert page.has_content?("Request to remove user access")
- step "the user fills out their details"
-
within "#tool-role-choice" do
choose @request_details["Tool/Role"]
end
@@ -138,8 +124,6 @@
assert page.has_content?("Request GDS support for a campaign")
- step "the user fills out their details"
-
fill_in "Campaign title", :with => @request_details["Campaign title"]
fill_in "ERG reference number", :with => @request_details["ERG ref number"]
fill_in "Start date", :with => @request_details["Start date"]
5 features/step_definitions/user_steps.rb
View
@@ -1,6 +1,7 @@
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
+ user_details = user_details.hashes.first
+
+ user = stub_everything('user', name: user_details["Name"], email: user_details["Email"], has_permission?: true)
login_as user
end
3  lib/zendesk_ticket.rb
View
@@ -12,7 +12,7 @@ def initialize(request)
@requester = request.requester
end
- def_delegators :@requester, :email, :collaborator_emails
+ def_delegators :@requester, :email, :name, :collaborator_emails
def comment
SnippetCollection.new(comment_snippets).to_s
@@ -49,6 +49,7 @@ def to_s
private
def base_attribute_snippets
[
+ LabelledSnippet.new(on: @requester, field: :name, label: "Requester name"),
LabelledSnippet.new(on: @requester, field: :email, label: "Requester email"),
LabelledSnippet.new(on: @requester, field: :collaborator_emails),
LabelledSnippet.new(on: self, field: :tags),
2  lib/zendesk_tickets.rb
View
@@ -10,7 +10,7 @@ def raise_ticket(ticket_to_raise)
:subject => ticket_to_raise.subject,
:description => "Created via Govt API",
:priority => "normal",
- :requester => {"locale_id" => 1, "email" => ticket_to_raise.email, "name" => ticket_to_raise.email},
+ :requester => {"locale_id" => 1, "email" => ticket_to_raise.email, "name" => ticket_to_raise.name},
:collaborators => ticket_to_raise.collaborator_emails,
:fields => [{"id" => GDSZendesk::FIELD_MAPPINGS[:needed_by_date], "value" => ticket_to_raise.needed_by_date},
{"id" => GDSZendesk::FIELD_MAPPINGS[:not_before_date], "value" => ticket_to_raise.not_before_date}],
14 test/functional/requests_controller_test.rb
View
@@ -48,6 +48,9 @@ def valid_params_for_test_request
class RequestsControllerTest < ActionController::TestCase
setup do
+ @logged_in_user_details = { name: "John Smith", email: "john.smith@gov.uk" }
+ login_as_stub_user(@logged_in_user_details[:name], @logged_in_user_details[:email])
+
Rails.application.routes.draw do
match 'new' => "test_requests#new"
match 'create' => "test_requests#create"
@@ -105,6 +108,15 @@ def prevent_implicit_rendering
post :create, params
end
+ should "read the signed-in user's details as the requester" do
+ params = valid_params_for_test_request
+
+ post :create, params
+
+ assert_equal @logged_in_user_details[:email], @zendesk_api.ticket.email
+ assert_equal @logged_in_user_details[:name], @zendesk_api.ticket.name
+ 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")
@@ -114,4 +126,4 @@ def prevent_implicit_rendering
assert_equal ["ab@c.com", "def@g.com"], @zendesk_api.ticket.collaborators
end
end
-end
+end
17 test/functional/support_controller_test.rb
View
@@ -1,10 +1,25 @@
require "test_helper"
class SupportControllerTest < ActionController::TestCase
+ setup do
+ login_as_stub_user(name: "John")
+ end
+
context "GET landing" do
- should "render the homepage" do
+ setup do
get :landing
+ end
+
+ should "render the homepage" do
assert_select "h1", /Welcome to GOV.UK Support/i
end
+
+ should "show the name of the user who is logged in" do
+ assert_select '#logged-in-user a', content: "John"
+ end
+
+ should "show have a link to log out" do
+ assert_select "a[href=/auth/gds/sign_out]", html: "Sign out"
+ end
end
end
8 test/test_helper.rb
View
@@ -12,14 +12,14 @@ class ActiveSupport::TestCase
def setup
super
WebMock.disable_net_connect!
- login_as_stub_user
+ login_as_stub_user if @user.nil?
switch_zendesk_into_dummy_mode
end
- def login_as_stub_user
+ def login_as_stub_user(name = "Stubby McStubby", email = "stubby@gov.uk")
@user = stub("stub user",
- name: "Stubby McStubby", remotely_signed_out?: false)
- request.env['warden'] = stub(:authenticate! => true, :authenticated? => true, :user => @user) unless request.nil?
+ name: name, remotely_signed_out?: false, email: email)
+ @request.env['warden'] = stub(:authenticate! => true, :authenticated? => true, :user => @user)
end
def switch_zendesk_into_dummy_mode
2  test/unit/models/requester_test.rb
View
@@ -3,6 +3,8 @@
class RequesterTest < Test::Unit::TestCase
should validate_presence_of(:email)
+ should validate_presence_of(:name)
+
should allow_value("ab@c.com").for(:email)
should allow_value("ab@ c.com").for(:email)
should allow_value("ab @c.com").for(:email)
38 test/unit/models/user_test.rb
View
@@ -0,0 +1,38 @@
+require 'test_helper'
+
+class UserTest < Test::Unit::TestCase
+ def setup
+ Rails.cache.clear
+ end
+
+ should "support persistent creation and retrieval" do
+ assert_nil User.find_by_uid("12345")
+ user = User.create!("uid" => "12345", "name" => "A", "email" => "a@b.com")
+
+ u = User.find_by_uid("12345")
+ assert_not_nil u
+ assert_equal "A", u.name
+ assert_equal "a@b.com", u.email
+ end
+
+ should "support remote sign-out" do
+ user = User.create!("uid" => "12345", "name" => "A", "email" => "a@b.com")
+ assert !user.remotely_signed_out?
+
+ user.update_attribute(:remotely_signed_out, true)
+ assert user.remotely_signed_out?
+
+ assert User.find_by_uid("12345").remotely_signed_out?
+ end
+
+ should "support mass updating of attributes" do
+ user = User.create!("uid" => "12345", "name" => "A", "email" => "a@b.com")
+
+ user.update_attributes({ "uid" => "12345", "name" => "Z", "email" => "x@y.com" }, { as: :somebody })
+
+ assert_equal "Z", user.name
+ assert_equal "x@y.com", user.email
+
+ assert_equal "Z", User.find_by_uid("12345").name
+ end
+end
3  test/unit/zendesk/zendesk_ticket_test.rb
View
@@ -39,11 +39,12 @@ def with_a_valid_requester
end
should "have a text representation" do
- ticket = new_ticket(with_requester(email: "ab@c.com", collaborator_emails: "a, b"))
+ ticket = new_ticket(with_requester(email: "ab@c.com", name: "A B", collaborator_emails: "a, b"))
ticket.stubs(:comment_snippets).returns([])
assert_includes ticket.to_s, "ab@c.com"
assert_includes ticket.to_s, "a, b"
+ assert_includes ticket.to_s, "A B"
end
context "with time constraints" do
Please sign in to comment.
Something went wrong with that request. Please try again.