Skip to content

Commit

Permalink
Fix minor bug redelivering to multiple
Browse files Browse the repository at this point in the history
When passing multiple destinations as a CSV [1], we're expecting to
strip these values down to an Array of two stripped ID numbers [2].

This wasn't happening as the `each` call wasn't returning the stripped
values. As such, we're left with some invalid ID numbers [3] that don't
get found later on.

This fixes this issue by using `map`, which returns the stripped values.

I've also added some specs. They're not the totally complete, but they
do fail prior to using `map` and pass afterwards, so good enough for
this specific issue.

[1] e.g. `"105, 106"`
[2] i.e. `["105", "106"]`
[3] i.e. `["105", " 106"]` – note the preceding space
  • Loading branch information
garethrees committed Sep 29, 2023
1 parent 8eb4842 commit f23d051
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
2 changes: 1 addition & 1 deletion app/controllers/admin_incoming_message_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def bulk_destroy
end

def redeliver
message_ids = params[:url_title].split(',').each(&:strip)
message_ids = params[:url_title].split(',').map(&:strip)
previous_request = @incoming_message.info_request
destination_request = nil

Expand Down
24 changes: 24 additions & 0 deletions spec/controllers/admin_incoming_message_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
FactoryBot.create(:incoming_message, info_request: previous_info_request)
end
let(:destination_info_request) { FactoryBot.create(:info_request) }
let(:destination_info_request_2) { FactoryBot.create(:info_request) }

it 'expires the file cache for the previous request' do
allow(IncomingMessage).to receive(:find).and_return(incoming_message)
Expand Down Expand Up @@ -107,6 +108,29 @@
expect(response).to redirect_to admin_request_url(incoming_message.info_request)
end

context 'when redelivering to multiple requests' do
before do
destination_params =
[destination_info_request, destination_info_request_2].
map(&:id).
join(', ')

post :redeliver,
params: { id: incoming_message.id,
url_title: destination_params }
end

it 'renders a message' do
msg = 'Message has been moved to request(s). Showing the last one:'
expect(flash[:notice]).to eq(msg)
end

it 'redirects to the last given destination request' do
expect(response).
to redirect_to admin_request_path(destination_info_request_2)
end
end

context 'if the request is embargoed', feature: :alaveteli_pro do
before do
incoming_message.info_request.create_embargo
Expand Down

0 comments on commit f23d051

Please sign in to comment.