[pro#549] apostrophe encoding

app/controllers/admin_request_controller.rb

@@ -35,13 +35,6 @@ def show
     if cannot? :admin, @info_request
       raise ActiveRecord::RecordNotFound
     end
-    vars_for_explanation = {:reason => params[:reason],
-                            :info_request => @info_request,
-                            :name_to => @info_request.user_name,
-                            :name_from => AlaveteliConfiguration::contact_name,
-                            :info_request_url => request_url(@info_request, :only_path => false)}
-    @request_hidden_user_explanation = render_to_string(:template => "admin_request/hidden_user_explanation",
-                                                        :locals => vars_for_explanation)
   end
 
   def edit

app/controllers/services_controller.rb

@@ -55,14 +55,15 @@ def other_country_message
 
   def hidden_user_explanation
     info_request = InfoRequest.find(params[:info_request_id])
-    render :template => "admin_request/hidden_user_explanation",
-      :content_type => "text/plain",
-      :layout => false,
-      :locals => {:name_to => info_request.user_name,
-                  :name_from => AlaveteliConfiguration.contact_name,
-                  :info_request => info_request, :reason => params[:reason],
-                  :info_request_url => 'http://' + AlaveteliConfiguration.domain + request_path(info_request),
-                  :site_name => site_name}
+    render template: "admin_request/hidden_user_explanation",
+           formats: [:text],
+           layout: false,
+           locals: {
+             name_to: info_request.user_name.html_safe,
+             info_request: info_request,
+             reason: params[:reason],
+             info_request_url: request_url(info_request),
+             site_name: site_name.html_safe }
   end
 
   private

app/models/outgoing_message.rb

@@ -92,9 +92,10 @@ def get_default_message
   end
 
   def set_signature_name(name)
-    # TODO: We use raw_body here to get unstripped one
+    # We compare against raw_body as body strips linebreaks and applies
+    # censor rules
     if raw_body == get_default_message
-      self.body = raw_body + name
+      self.body = get_default_message + name
     end
   end
 

app/views/admin_request/show.html.erb

@@ -200,7 +200,7 @@
         <div class="control-group" id="request_hidden_user_explanation">
           <label for="request_hidden_user_explanation_field" class="control-label">Reason for hiding the request (will be emailed to user):</label>
           <div class="controls">
-            <%= text_area_tag "explanation", h(@request_hidden_user_explanation), {:id => "request_hidden_user_explanation_field"} %>
+            <%= text_area_tag "explanation", "", {:id => "request_hidden_user_explanation_field"} %>
           </div>
         </div>
 

app/views/admin_user/_user_table.html.erb

@@ -7,7 +7,7 @@
         <span class="user-labels">
           <%= user_labels(user) %>
         </span>
-        <%= link_to("#{ h(user.name) }", admin_user_path(user)) %>
+        <%= link_to(user.name, admin_user_path(user)) %>
         <%= link_to("(#{ h(user.email) })", "mailto:#{ h(user.email) }") %>
       </span>
       <span class="item-metadata">

app/views/alaveteli_pro/info_request_batches/_message_preview.html.erb

@@ -11,7 +11,7 @@
   </p>
 
   <div class="correspondence_text">
-    <p><%= simple_format(outgoing_message.body) %></p>
+    <p><%= simple_format(outgoing_message.get_body_for_html_display) %></p>
   </div>
 </div>
 

app/views/alaveteli_pro/info_requests/_message_preview.html.erb

@@ -9,7 +9,7 @@
   </p>
 
   <div class="correspondence_text">
-    <p><%= simple_format(@outgoing_message.body) %></p>
+    <p><%= simple_format(@outgoing_message.get_body_for_html_display) %></p>
   </div>
 </div>
 

spec/controllers/admin_request_controller_spec.rb

@@ -109,19 +109,6 @@
       expect(response).to be_success
     end
 
-    it "shows a suitable default 'your email has been hidden' message" do
-      get :show, { :id => info_request.id }, { :user_id => admin_user.id }
-      expect(assigns[:request_hidden_user_explanation]).
-        to include(info_request.user.name)
-      expect(assigns[:request_hidden_user_explanation]).
-        to include("vexatious")
-      get :show, :id => info_request.id, :reason => "not_foi"
-      expect(assigns[:request_hidden_user_explanation]).
-        not_to include("vexatious")
-      expect(assigns[:request_hidden_user_explanation]).
-        to include("not a valid FOI")
-    end
-
     context 'if the request is embargoed' do
 
       before do

spec/controllers/services_controller_spec.rb

@@ -1,83 +1,114 @@
-# -*- encoding : utf-8 -*-
-require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
+require 'spec_helper'
 require 'fakeweb'
 
-describe ServicesController, "when returning a message for people in other countries" do
+describe ServicesController do
 
   render_views
 
-  # store and restore the locale in the context of the test suite to isolate
-  # changes made in these tests
-  before do
-    @old_locale = FastGettext.locale
-  end
+  describe '#other_country_message' do
 
-  it 'keeps the flash' do
-    # Make two get requests to simulate the flash getting swept after the
-    # first response.
-    get :other_country_message, nil, nil, :some_flash_key => 'abc'
-    get :other_country_message
-    expect(flash[:some_flash_key]).to eq('abc')
-  end
+    # store and restore the locale in the context of the test suite to isolate
+    # changes made in these tests
+    before do
+      @old_locale = FastGettext.locale
+    end
 
-  it "shows no alaveteli message when user in same country as deployed alaveteli" do
-    allow(AlaveteliConfiguration).to receive(:iso_country_code).and_return("DE")
-    allow(controller).to receive(:country_from_ip).and_return('DE')
-    get :other_country_message
-    expect(response.body).to eq("")
-  end
+    after do
+      FastGettext.set_locale(@old_locale)
+    end
 
-  it "shows a message if user location has a deployed FOI website" do
-    allow(AlaveteliConfiguration).to receive(:iso_country_code).and_return("ZZ")
-    allow(controller).to receive(:country_from_ip).and_return('DE')
-    get :other_country_message
-    expect(response.body).
-      to match(/requests within Deutschland at/)
-  end
+    it 'keeps the flash' do
+      # Make two get requests to simulate the flash getting swept after the
+      # first response.
+      get :other_country_message, nil, nil, :some_flash_key => 'abc'
+      get :other_country_message
+      expect(flash[:some_flash_key]).to eq('abc')
+    end
 
-  it "shows a message when user not in same country as deployed alaveteli and user country has no FOI website" do
-    allow(AlaveteliConfiguration).to receive(:iso_country_code).and_return("DE")
-    allow(controller).to receive(:country_from_ip).and_return('ZZ')
-    get :other_country_message
-    expect(response.body).to match(/outside Deutschland/)
-  end
+    it "shows no alaveteli message when user in same country as deployed alaveteli" do
+      allow(AlaveteliConfiguration).
+        to receive(:iso_country_code).and_return("DE")
+      allow(controller).to receive(:country_from_ip).and_return('DE')
+      get :other_country_message
+      expect(response.body).to eq("")
+    end
 
-  it "shows a message when user not in same country as deployed alaveteli and user country has no FOI website
-      and WorldFOIWebsites has no data about the deployed alaveteli" do
-    allow(AlaveteliConfiguration).to receive(:iso_country_code).and_return("XY")
-    allow(controller).to receive(:country_from_ip).and_return('ZZ')
-    get :other_country_message
-    expect(response.body).to match(/in other countries/)
-  end
+    it "shows a message if user location has a deployed FOI website" do
+      allow(AlaveteliConfiguration).
+        to receive(:iso_country_code).and_return("ZZ")
+      allow(controller).to receive(:country_from_ip).and_return('DE')
+      get :other_country_message
+      expect(response.body).
+        to match(/requests within Deutschland at/)
+    end
 
-  it "shows an EU message if the user location has a deployed FOI website and is covered by AskTheEU" do
-    allow(AlaveteliConfiguration).to receive(:iso_country_code).and_return("DE")
-    allow(controller).to receive(:country_from_ip).and_return('GB')
-    get :other_country_message
-    expect(response.body).to match(/within United Kingdom at/)
-    expect(response.body).to match(/EU institutions/)
-  end
+    context 'when user not in the same country as site' do
 
-  it "shows a message when user not in same country as deployed alaveteli and user country has no FOI website
-      but user country is covered by EU" do
-    allow(AlaveteliConfiguration).to receive(:iso_country_code).and_return("DE")
-    allow(controller).to receive(:country_from_ip).and_return('MT')
-    get :other_country_message
-    expect(response.body).to match(/outside Deutschland/)
-    expect(response.body).to match(/EU institutions/)
-  end
+      it "shows a message when user country has no FOI website" do
+        allow(AlaveteliConfiguration).
+          to receive(:iso_country_code).and_return("DE")
+        allow(controller).to receive(:country_from_ip).and_return('ZZ')
+        get :other_country_message
+        expect(response.body).to match(/outside Deutschland/)
+      end
+
+      it "shows a message when user country has no FOI website and WorldFOIWebsites has no data about the deployed alaveteli" do
+        allow(AlaveteliConfiguration).
+          to receive(:iso_country_code).and_return("XY")
+        allow(controller).to receive(:country_from_ip).and_return('ZZ')
+        get :other_country_message
+        expect(response.body).to match(/in other countries/)
+      end
+
+      it "shows an EU message if the user location has a deployed FOI website and is covered by AskTheEU" do
+        allow(AlaveteliConfiguration).
+          to receive(:iso_country_code).and_return("DE")
+        allow(controller).to receive(:country_from_ip).and_return('GB')
+        get :other_country_message
+        expect(response.body).to match(/within United Kingdom at/)
+        expect(response.body).to match(/EU institutions/)
+      end
+
+      it "shows a message when user country has no FOI website but user country is covered by EU" do
+        allow(AlaveteliConfiguration).
+          to receive(:iso_country_code).and_return("DE")
+        allow(controller).to receive(:country_from_ip).and_return('MT')
+        get :other_country_message
+        expect(response.body).to match(/outside Deutschland/)
+        expect(response.body).to match(/EU institutions/)
+      end
+
+      it "shows a message when and user country has no FOI website and WorldFOIWebsites has no data about the deployed alaveteli but user is covered by EU" do
+        allow(AlaveteliConfiguration).
+          to receive(:iso_country_code).and_return("XY")
+        allow(controller).to receive(:country_from_ip).and_return('MT')
+        get :other_country_message
+        expect(response.body).to match(/in other countries/)
+        expect(response.body).to match(/EU institutions/)
+      end
+
+    end
 
-  it "shows a message when user not in same country as deployed alaveteli and user country has no FOI website
-      and WorldFOIWebsites has no data about the deployed alaveteli but user is covered by EU" do
-    allow(AlaveteliConfiguration).to receive(:iso_country_code).and_return("XY")
-    allow(controller).to receive(:country_from_ip).and_return('MT')
-    get :other_country_message
-    expect(response.body).to match(/in other countries/)
-    expect(response.body).to match(/EU institutions/)
   end
 
-  after do
-    FastGettext.set_locale(@old_locale)
+  describe '#hidden_user_explanation' do
+
+    let(:user) { FactoryBot.create(:user, name: "P O'Toole") }
+    let(:info_request) { FactoryBot.create(:info_request, user: user) }
+
+    it 'generates plaintext output' do
+      get :hidden_user_explanation, info_request_id: info_request.id
+      expect(response.content_type).to eq 'text/plain'
+    end
+
+    it 'does not HTML escape the user or site name' do
+      allow(AlaveteliConfiguration).
+        to receive(:site_name).and_return('A&B Test')
+      get :hidden_user_explanation, info_request_id: info_request.id
+      expect(response.body).to match(/Dear P O'Toole/)
+      expect(response.body).to match(/Yours,\n\nThe A&B Test team/)
+    end
+
   end
 
 end

spec/integration/alaveteli_pro/batch_request_spec.rb

@@ -212,10 +212,13 @@ def search_results
     using_pro_session(pro_user_session) do
       start_batch_request
       fill_in_batch_message
+      fill_in "Your request",
+              with: "Dear [Authority name], this is a <b>batch</b> request."
       click_button "Preview and send request"
 
       # Preview page
-      drafts = AlaveteliPro::DraftInfoRequestBatch.where(title: "Does the pro batch request form work?")
+      drafts = AlaveteliPro::DraftInfoRequestBatch.
+        where(title: "Does the pro batch request form work?")
       expect(drafts).to exist
       draft = drafts.first
 
@@ -228,7 +231,9 @@ def search_results
                                    "form work?")
       # It should substitue an authority name in when previewing
       first_authority = draft.public_bodies.first
-      expect(page).to have_content("Dear #{first_authority.name}, this is a batch request.")
+      expect(page).
+        to have_content("Dear #{first_authority.name}, this is a " \
+                        "<b>batch</b> request.")
       expect(page).to have_content(
         "Requests in this batch will be private until " \
         "#{AlaveteliPro::Embargo.three_months_from_now.strftime('%-d %B %Y')}")

spec/integration/alaveteli_pro/request_creation_spec.rb

@@ -82,6 +82,18 @@
       end
     end
 
+    it 'does not render HTML on the preview page' do
+      public_body.update_attribute(:name, "Test's <sup>html</sup> authority")
+      using_pro_session(pro_user_session) do
+        visit show_public_body_path(:url_name => public_body.url_name)
+        click_link("Make a request to this authority")
+        fill_in 'Subject', :with => "HTML test"
+        click_button "Preview and send"
+
+        expect(page).to have_content("Dear Test's <sup>html</sup> authority")
+      end
+    end
+
     it "allows us to send the request" do
       using_pro_session(pro_user_session) do
         # New request form