aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--app/controllers/request_controller.rb13
-rw-r--r--app/models/info_request_batch.rb12
-rw-r--r--app/views/request/new.html.erb7
-rw-r--r--db/migrate/20131024152540_add_body_to_info_request_batches.rb11
-rw-r--r--spec/controllers/request_controller_spec.rb11
-rw-r--r--spec/factories.rb1
-rw-r--r--spec/models/info_request_batch_spec.rb45
7 files changed, 95 insertions, 5 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 047fc7acf..ccf824e75 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -188,13 +188,9 @@ class RequestController < ApplicationController
redirect_to select_authorities_path and return
end
- # TODO: I do think we should probably check for double submission of batch
- # requests as we do in 'new' for ordinary requests with find_existing
-
# TODO: Decide if we make batch requesters describe their undescribed requests
# before being able to make a new batch request
-
if !authenticated_user.can_file_requests?
@details = authenticated_user.can_fail_html
render :template => 'user/banned' and return
@@ -205,12 +201,18 @@ class RequestController < ApplicationController
return render_new_compose(batch=true)
end
+ # Check for double submission of batch
+ @existing_batch = InfoRequestBatch.find_existing(authenticated_user,
+ params[:info_request][:title],
+ params[:outgoing_message][:body],
+ params[:public_body_ids])
+
@info_request = InfoRequest.create_from_attributes(params[:info_request],
params[:outgoing_message],
authenticated_user)
@outgoing_message = @info_request.outgoing_messages.first
@info_request.is_batch_request_template = true
- if !@info_request.valid?
+ if !@existing_batch.nil? || !@info_request.valid?
# We don't want the error "Outgoing messages is invalid", as in this
# case the list of errors will also contain a more specific error
# describing the reason it is invalid.
@@ -227,6 +229,7 @@ class RequestController < ApplicationController
# TODO: give messages about bodies
# that are no longer requestable
@info_request_batch = InfoRequestBatch.create!(:title => params[:info_request][:title],
+ :body => params[:outgoing_message][:body],
:user => authenticated_user)
@public_bodies = PublicBody.where({:id => params[:public_body_ids]}).all
@public_bodies.each do |public_body|
diff --git a/app/models/info_request_batch.rb b/app/models/info_request_batch.rb
index af6956095..05175a889 100644
--- a/app/models/info_request_batch.rb
+++ b/app/models/info_request_batch.rb
@@ -16,5 +16,17 @@ class InfoRequestBatch < ActiveRecord::Base
validates_presence_of :user
validates_presence_of :title
+ validates_presence_of :body
+
+ # When constructing a new batch, use this to check user hasn't double submitted.
+ def InfoRequestBatch.find_existing(user, title, body, public_body_ids)
+ find(:first, :conditions => ['info_request_batches.user_id = ?
+ AND info_request_batches.title = ?
+ AND body = ?
+ AND info_requests.public_body_id in (?)',
+ user, title, body, public_body_ids],
+ :include => :info_requests)
+ end
+
end
diff --git a/app/views/request/new.html.erb b/app/views/request/new.html.erb
index 91171da3e..8534bf0da 100644
--- a/app/views/request/new.html.erb
+++ b/app/views/request/new.html.erb
@@ -33,6 +33,13 @@
</li>
</ul></div>
<% end %>
+ <% if @existing_batch %>
+ <div class="errorExplanation" id="errorExplanation"><ul>
+ <li>
+ <%= _('You already created the same batch of requests on {{date}}. You can either view the <a href="{{existing_batch}}">existing batch</a>, or edit the details below to make a new but similar batch of requests.', :date=>simple_date(@existing_batch.created_at), :existing_batch=>info_request_batch_path(@existing_batch)) %>
+ </li>
+ </ul></div>
+ <% end %>
<%= foi_error_messages_for :info_request, :outgoing_message %>
diff --git a/db/migrate/20131024152540_add_body_to_info_request_batches.rb b/db/migrate/20131024152540_add_body_to_info_request_batches.rb
new file mode 100644
index 000000000..5f9b3af10
--- /dev/null
+++ b/db/migrate/20131024152540_add_body_to_info_request_batches.rb
@@ -0,0 +1,11 @@
+class AddBodyToInfoRequestBatches < ActiveRecord::Migration
+ def up
+ add_column :info_request_batches, :body, :text
+ add_index :info_request_batches, [:user_id, :body, :title]
+ end
+
+ def down
+ remove_column :info_request_batches, :body
+ end
+
+end
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 6652d8f1f..c61e0c780 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -2573,6 +2573,17 @@ describe RequestController, "#new_batch", :focus => true do
response.should redirect_to(info_request_batch_path(new_info_request_batch))
end
+ it 'should prevent double submission of a batch request' do
+ params = @default_post_params.merge(:preview => 0)
+ post :new_batch, params, { :user_id => @user.id }
+ new_info_request_batch = assigns[:info_request_batch]
+ response.should redirect_to(info_request_batch_path(new_info_request_batch))
+ post :new_batch, params, { :user_id => @user.id }
+ response.should render_template('new')
+ assigns[:existing_batch].should_not be_nil
+ end
+
+
context "when the user is banned" do
before do
diff --git a/spec/factories.rb b/spec/factories.rb
index 66388af6e..3ea943030 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -148,5 +148,6 @@ FactoryGirl.define do
factory :info_request_batch do
title "Example title"
user
+ body "Some text"
end
end
diff --git a/spec/models/info_request_batch_spec.rb b/spec/models/info_request_batch_spec.rb
index f6d9bea0c..787b812cf 100644
--- a/spec/models/info_request_batch_spec.rb
+++ b/spec/models/info_request_batch_spec.rb
@@ -18,4 +18,49 @@ describe InfoRequestBatch, "when validating" do
@info_request_batch.errors.full_messages.should == ["Title can't be blank"]
end
+ it 'should require a body' do
+ @info_request_batch.body = nil
+ @info_request_batch.valid?.should be_false
+ @info_request_batch.errors.full_messages.should == ["Body can't be blank"]
+ end
+
+end
+
+describe InfoRequestBatch, "when finding an existing batch" do
+
+ before do
+ @info_request_batch = FactoryGirl.create(:info_request_batch, :title => 'Matched title',
+ :body => 'Matched body')
+ @first_request = FactoryGirl.create(:info_request, :info_request_batch => @info_request_batch)
+ @second_request = FactoryGirl.create(:info_request, :info_request_batch => @info_request_batch)
+ end
+
+ it 'should return a batch with the same user, title and body sent to one of the same public bodies' do
+ InfoRequestBatch.find_existing(@info_request_batch.user,
+ @info_request_batch.title,
+ @info_request_batch.body,
+ [@first_request.public_body_id]).should_not be_nil
+ end
+
+ it 'should not return a batch with the same title and body sent to another public body' do
+ InfoRequestBatch.find_existing(@info_request_batch.user,
+ @info_request_batch.title,
+ @info_request_batch.body,
+ [FactoryGirl.create(:public_body).id]).should be_nil
+ end
+
+ it 'should not return a batch sent the same public bodies with a different title and body' do
+ InfoRequestBatch.find_existing(@info_request_batch.user,
+ 'Other title',
+ 'Other body',
+ [@first_request.public_body_id]).should be_nil
+ end
+
+ it 'should not return a batch sent to one of the same public bodies with the same title and body by
+ a different user' do
+ InfoRequestBatch.find_existing(FactoryGirl.create(:user),
+ @info_request_batch.title,
+ @info_request_batch.body,
+ [@first_request.public_body_id]).should be_nil
+ end
end