Merge pull request #364 from ansonK/redirect-after-signing-petition

Redirect to clean petition url

app/controllers/signatures_controller.rb

@@ -26,7 +26,14 @@ def signed
     verify_token
 
     if @signature.validated?
-      @petition = @signature.petition
+
+      if @signature.seen_signed_confirmation_page?
+        redirect_to petition_url @signature.petition
+      else
+        @signature.mark_seen_signed_confirmation_page!
+        @petition = @signature.petition
+      end
+
     else
       redirect_to(verify_signature_url(@signature, @signature.perishable_token))
     end

app/models/signature.rb

@@ -111,6 +111,10 @@ def validate!
     end
   end
 
+  def mark_seen_signed_confirmation_page!
+    update seen_signed_confirmation_page: true
+  end
+
   def unsubscribe!(token)
     if unsubscribed?
       errors.add(:base, "Already Unsubscribed")

db/migrate/20150730110838_add_seen_signed_confirmation_page_to_signatures.rb

@@ -0,0 +1,5 @@
+class AddSeenSignedConfirmationPageToSignatures < ActiveRecord::Migration
+  def change
+    add_column :signatures, :seen_signed_confirmation_page, :boolean, null: false, default: false
+  end
+end

db/structure.sql

@@ -462,7 +462,8 @@ CREATE TABLE signatures (
     unsubscribe_token character varying,
     constituency_id character varying,
     validated_at timestamp without time zone,
-    number integer
+    number integer,
+    seen_signed_confirmation_page boolean DEFAULT false NOT NULL
 );
 
 
@@ -1154,3 +1155,5 @@ INSERT INTO schema_migrations (version) VALUES ('20150709152530');
 
 INSERT INTO schema_migrations (version) VALUES ('20150714140659');
 
+INSERT INTO schema_migrations (version) VALUES ('20150730110838');
+

features/step_definitions/common_steps.rb

@@ -46,3 +46,8 @@
   binding.pry
   :debugger
 end
+
+When(/^I click the shared link$/) do
+  expect(@shared_link).not_to be_blank
+  visit @shared_link
+end

features/step_definitions/petition_steps.rb

@@ -121,7 +121,7 @@
   @petition = FactoryGirl.create(:archived_petition, :rejected, title: title, reason_for_rejection: reason_for_rejection)
 end
 
-When /^I view the petition$/ do
+When(/^I view the petition$/) do
   if @petition.is_a?(ArchivedPetition)
     visit archived_petition_url(@petition)
   else

features/step_definitions/signature_steps.rb

@@ -127,6 +127,15 @@ def should_be_signature_count_of(count)
          :postcode => "SW14 9RQ", :name => "Sam Wibbledon")
 end
 
+Given(/^Suzie has already signed the petition and validated her email$/) do
+  @suzies_signature = FactoryGirl.create(:validated_signature, :petition => @petition, :email => "[email protected]",
+         :postcode => "SW14 9RQ", :name => "Womboid Wibbledon")
+end
+
+When(/^Suzie shares the signatory confirmation link with Eric$/) do
+  @shared_link = signed_signature_url(@suzies_signature, token: @suzies_signature.perishable_token)
+end
+
 When /^I try to sign the petition with the same email address and a different name$/ do
   steps %Q{
     When I decide to sign the petition

features/suzie_signs_a_petition.feature

@@ -103,3 +103,9 @@ Feature: Suzie signs a petition
     And I should see "2 signatures"
     And I can click on a link to return to the petition
     Then I should see that I have already signed the petition
+
+  Scenario: Eric clicks the link shared to him by Suzie
+    When Suzie has already signed the petition and validated her email
+    And Suzie shares the signatory confirmation link with Eric
+    And I click the shared link
+    Then I view the petition

spec/controllers/signatures_controller_spec.rb

@@ -109,40 +109,66 @@
   describe "signed" do
     let(:petition) { FactoryGirl.create(:petition) }
 
+    def make_signed_request(token = nil)
+      get :signed, id: signature.to_param, token: (token || signature.perishable_token)
+    end
+
     context 'for validated signatures' do
       let(:signature) { FactoryGirl.create(:validated_signature, petition: petition) }
 
-      it 'renders the signed template' do
-        get :signed, id: signature.to_param, token: signature.perishable_token
-        expect(response).to be_success
-        expect(response).to render_template('signatures/signed')
+      it "raises exception if token does not match the signature" do
+        expect { make_signed_request "not_a_valid_token" }.to raise_error(ActiveRecord::RecordNotFound)
       end
 
-      it 'exposes the signature and its petition' do
-        get :signed, id: signature.to_param, token: signature.perishable_token
-        expect(assigns['signature']).to eq signature
-        expect(assigns['petition']).to eq petition
+      context 'signer has not seen the signed page before' do
+        before do
+          signature.update! seen_signed_confirmation_page: false
+        end
+
+        it 'exposes the signature and its petition' do
+          make_signed_request
+          expect(assigns['signature']).to eq signature
+          expect(assigns['petition']).to eq petition
+        end
+
+        it 'marks the signature as the signer having seen the confirmation page' do
+          make_signed_request
+          expect(signature.reload.seen_signed_confirmation_page).to be_truthy
+        end
+
+        it 'renders the signed template' do
+          make_signed_request
+          expect(response).to be_success
+          expect(response).to render_template('signatures/signed')
+        end
       end
 
-      it "raises exception if token does not match the signature" do
-        expect do
-          get :signed, id: signature.to_param, token: "#{signature.perishable_token}a"
-        end.to raise_error(ActiveRecord::RecordNotFound)
+      context 'signer has already seen the signed page (clicking link in the email again)' do
+        before do
+          signature.mark_seen_signed_confirmation_page!
+        end
+
+        it 'keeps the signature as the signer having seen the confirmation page' do
+          expect { make_signed_request }.not_to change(signature.reload, :seen_signed_confirmation_page)
+        end
+
+        it 'redirects to the petition show page' do
+          make_signed_request
+          expect(response).to redirect_to "https://petition.parliament.uk/petitions/#{petition.id}"
+        end
       end
     end
 
     context 'for unvalidated signatures' do
       let(:signature) { FactoryGirl.create(:pending_signature, petition: petition) }
 
       it "redirects to the signature verify page" do
-        get :signed, id: signature.to_param, token: signature.perishable_token
+        make_signed_request
         expect(response).to redirect_to("https://petition.parliament.uk/signatures/#{signature.id}/verify/#{signature.perishable_token}")
       end
 
       it "raises exception if token does not match" do
-        expect do
-          get :signed, id: signature.to_param, token: "#{signature.perishable_token}a"
-        end.to raise_error(ActiveRecord::RecordNotFound)
+        expect { make_signed_request "not_a_valid_token" }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
   end

spec/factories.rb

@@ -193,8 +193,9 @@
     state                Signature::VALIDATED_STATE
 
     after(:create) do |signature, evaluator|
-      if signature.petition
-        signature.petition.increment!(:signature_count) if signature.validated?
+      if signature.petition && signature.validated?
+        signature.petition.increment!(:signature_count)
+        signature.increment!(:number)
       end
     end
   end
@@ -204,8 +205,9 @@
   end
 
   factory :validated_signature, :parent => :signature do
-    state          Signature::VALIDATED_STATE
-    validated_at { Time.current }
+    state                         Signature::VALIDATED_STATE
+    validated_at                  { Time.current }
+    seen_signed_confirmation_page true
   end
 
   sequence(:sponsor_email) { |n| "sponsor#{n}@example.com" }