Browse Source

extract controller methods and associated tests to do with the delete logs feature into their own controller,

amend routes accordingly
pull/1657/head
Arthur Campbell 3 years ago
parent
commit
9883e94198
  1. 56
      app/controllers/delete_logs_controller.rb
  2. 39
      app/controllers/lettings_logs_controller.rb
  3. 1
      app/models/forms/delete_logs_form.rb
  4. 4
      app/views/logs/delete_logs_confirmation.html.erb
  5. 8
      config/routes.rb
  6. 262
      spec/requests/delete_logs_controller_spec.rb
  7. 261
      spec/requests/lettings_logs_controller_spec.rb

56
app/controllers/delete_logs_controller.rb

@ -0,0 +1,56 @@
class DeleteLogsController < ApplicationController
include Modules::LogsFilter
rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
before_action :session_filters, if: :current_user, except: [:delete_logs]
def delete_lettings_logs
@delete_logs_form = delete_logs_form
render "logs/delete_lettings_logs"
end
def delete_lettings_logs_with_selected_ids
selected_ids = params.require(:selected_ids).split.map(&:to_i)
@delete_logs_form = delete_logs_form(selected_ids:)
render "logs/delete_lettings_logs"
end
def delete_lettings_logs_confirmation
default_attributes = {
current_user:,
log_filters: @session_filters,
log_type: :lettings,
}
form_attributes = params.require(:forms_delete_logs_form).permit(:search_term, selected_ids: [])
attributes = form_attributes.merge(default_attributes)
attributes[:selected_ids] = [] unless attributes.key? :selected_ids
@delete_path = delete_logs_lettings_logs_path
@delete_logs_form = Forms::DeleteLogsForm.new(attributes)
if @delete_logs_form.valid?
render "logs/delete_logs_confirmation"
else
render "logs/delete_lettings_logs"
end
end
def discard_lettings_logs
logs = LettingsLog.find(params.require(:ids))
logs.each do |log|
authorize log, :destroy?
log.discard!
end
redirect_to lettings_logs_path, notice: I18n.t("notification.logs_deleted", count: logs.count)
end
private
def delete_logs_form(selected_ids: nil)
Forms::DeleteLogsForm.new(current_user:, search_term:, log_filters: @session_filters, log_type: :lettings, selected_ids:)
end
def search_term
params["search"]
end
end

39
app/controllers/lettings_logs_controller.rb

@ -112,41 +112,6 @@ class LettingsLogsController < LogsController
end
end
def delete_lettings_logs
@delete_logs_form = delete_logs_form
end
def delete_lettings_logs_with_selected_ids
selected_ids = params.require(:selected_ids).split.map(&:to_i)
@delete_logs_form = delete_logs_form(selected_ids:)
render :delete_lettings_logs
end
def delete_logs_confirmation
default_attributes = {
current_user:,
log_filters: @session_filters,
log_type: :lettings,
}
form_attributes = params.require(:forms_delete_logs_form).permit(:search_term, selected_ids: [])
attributes = form_attributes.merge(default_attributes)
attributes[:selected_ids] = [] unless attributes.key? :selected_ids
@delete_logs_form = Forms::DeleteLogsForm.new(attributes)
unless @delete_logs_form.valid?
render :delete_lettings_logs
end
end
def delete_logs
logs = LettingsLog.find(params.require(:ids))
logs.each do |log|
authorize log, :destroy?
log.discard!
end
redirect_to lettings_logs_path, notice: I18n.t("notification.logs_deleted", count: logs.count)
end
private
def session_filters
@ -163,10 +128,6 @@ private
)
end
def delete_logs_form(selected_ids: nil)
Forms::DeleteLogsForm.new(current_user:, search_term:, log_filters: @session_filters, log_type: :lettings, selected_ids:)
end
def authenticate_scope!
head :unauthorized and return if codes_only_export? && !current_user.support?
end

1
app/models/forms/delete_logs_form.rb

@ -1,6 +1,5 @@
module Forms
class DeleteLogsForm
include Modules::LogsFilter
include ActiveModel::Model
include ActiveModel::Validations

4
app/views/logs/delete_logs_confirmation.html.erb

@ -16,8 +16,8 @@
<% end %>
<div class="govuk-button-group">
<%= govuk_button_to "Delete logs", delete_logs_lettings_logs_path, method: "delete", params: { ids: @delete_logs_form.selected_ids } %>
<%= form_with url: delete_logs_lettings_logs_path do |f| %>
<%= govuk_button_to "Delete logs", @delete_path, method: "delete", params: { ids: @delete_logs_form.selected_ids } %>
<%= form_with url: @delete_path do |f| %>
<%= f.hidden_field :selected_ids, value: @delete_logs_form.selected_ids %>
<%= f.hidden_field :search, value: @delete_logs_form.search_term %>
<%= f.govuk_submit "Cancel", secondary: true %>

8
config/routes.rb

@ -170,10 +170,10 @@ Rails.application.routes.draw do
post "email-csv", to: "lettings_logs#email_csv"
get "csv-confirmation", to: "lettings_logs#csv_confirmation"
get "delete-logs", to: "lettings_logs#delete_lettings_logs"
post "delete-logs", to: "lettings_logs#delete_lettings_logs_with_selected_ids"
post "delete-logs-confirmation", to: "lettings_logs#delete_logs_confirmation"
delete "delete-logs", to: "lettings_logs#delete_logs"
get "delete-logs", to: "delete_logs#delete_lettings_logs"
post "delete-logs", to: "delete_logs#delete_lettings_logs_with_selected_ids"
post "delete-logs-confirmation", to: "delete_logs#delete_lettings_logs_confirmation"
delete "delete-logs", to: "delete_logs#discard_lettings_logs"
resources :bulk_upload_lettings_logs, path: "bulk-upload-logs", only: %i[show update] do
collection do

262
spec/requests/delete_logs_controller_spec.rb

@ -0,0 +1,262 @@
require "rails_helper"
RSpec.describe "DeleteLogs", type: :request do
let(:page) { Capybara::Node::Simple.new(response.body) }
describe "GET delete-logs" do
let(:user) { create(:user, name: "Richard MacDuff") }
let!(:log_1) { create(:lettings_log, :in_progress, created_by: user) }
let!(:log_2) { create(:lettings_log, :completed, created_by: user) }
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
end
it "calls the filter service with the filters in the session and the search term from the query params" do
search = "Schrödinger's cat"
logs_filters = {
"years" => [""],
"status" => ["", "in_progress"],
"user" => "all",
}
get lettings_logs_path(logs_filters) # adds the filters to the session
expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters
}.and_return LettingsLog.all
get delete_logs_lettings_logs_path(search:)
end
it "displays the logs returned by the filter service" do
get delete_logs_lettings_logs_path
table_body_rows = page.find_all("tbody tr")
expect(table_body_rows.count).to be 2
ids_in_table = table_body_rows.map { |row| row.first("td").text }
expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
end
it "checks all checkboxes by default" do
get delete_logs_lettings_logs_path
checkboxes = page.find_all("tbody tr").map { |row| row.find("input") }
expect(checkboxes.count).to be 2
expect(checkboxes).to all be_checked
end
end
describe "POST delete-logs" do
let(:user) { create(:user, name: "Richard MacDuff") }
let!(:log_1) { create(:lettings_log, :in_progress, created_by: user) }
let!(:log_2) { create(:lettings_log, :completed, created_by: user) }
let(:selected_ids) { log_1.id }
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
end
it "throws an error if selected ids are not provided" do
expect { post delete_logs_lettings_logs_path }.to raise_error ActionController::ParameterMissing
end
it "calls the filter service with the filters in the session and the search term from the query params" do
search = "Schrödinger's cat"
logs_filters = {
"years" => [""],
"status" => ["", "in_progress"],
"user" => "all",
}
get lettings_logs_path(logs_filters) # adds the filters to the session
expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters
}.and_return LettingsLog.all
post delete_logs_lettings_logs_path(search:, selected_ids:)
end
it "displays the logs returned by the filter service" do
post delete_logs_lettings_logs_path(selected_ids:)
table_body_rows = page.find_all("tbody tr")
expect(table_body_rows.count).to be 2
ids_in_table = table_body_rows.map { |row| row.first("td").text }
expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
end
it "only checks the selected checkboxes when selected_ids provided" do
post delete_logs_lettings_logs_path(selected_ids:)
checkboxes = page.find_all("tbody tr").map { |row| row.find("input") }
checkbox_expected_checked = checkboxes.find { |cb| cb.value == log_1.id.to_s }
checkbox_expected_unchecked = checkboxes.find { |cb| cb.value == log_2.id.to_s }
expect(checkbox_expected_checked).to be_checked
expect(checkbox_expected_unchecked).not_to be_checked
end
end
describe "POST delete-logs-confirmation" do
let(:user) { create(:user, name: "Urban Chronotis") }
let(:log_1) { create(:lettings_log, :in_progress) }
let(:log_2) { create(:lettings_log, :completed) }
let(:log_3) { create(:lettings_log, :in_progress) }
let(:params) do
{
forms_delete_logs_form: {
search_term: "milk",
selected_ids: [log_1, log_2].map(&:id),
},
}
end
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
post delete_logs_confirmation_lettings_logs_path, params: params
end
it "requires delete logs form data to be provided" do
expect { post delete_logs_confirmation_lettings_logs_path }.to raise_error(ActionController::ParameterMissing)
end
it "shows the correct title" do
expect(page.find("h1").text).to include "Are you sure you want to delete these logs?"
end
it "shows the correct information text to the user" do
expect(page).to have_selector("p", text: "You've selected 2 logs to delete")
end
context "when only one log is selected" do
let(:params) do
{
forms_delete_logs_form: {
search_term: "milk",
selected_ids: [log_1].map(&:id),
},
}
end
it "shows the correct information text to the user in the singular" do
post delete_logs_confirmation_lettings_logs_path, params: params
expect(page).to have_selector("p", text: "You've selected 1 log to delete")
end
end
it "shows a warning to the user" do
expect(page).to have_selector(".govuk-warning-text", text: "You will not be able to undo this action")
end
it "shows a button to delete the selected logs" do
expect(page).to have_selector("form.button_to button", text: "Delete logs")
end
it "the delete logs button submits the correct data to the correct path" do
form_containing_button = page.find("form.button_to")
expect(form_containing_button[:action]).to eq delete_logs_lettings_logs_path
expect(form_containing_button).to have_field "_method", type: :hidden, with: "delete"
expect(form_containing_button).to have_field "ids[]", type: :hidden, with: log_1.id
expect(form_containing_button).to have_field "ids[]", type: :hidden, with: log_2.id
end
it "shows a cancel button with the correct style" do
expect(page).to have_selector("button.govuk-button--secondary", text: "Cancel")
end
it "the cancel button submits the correct data to the correct path" do
form_containing_cancel = page.find_all("form").find { |form| form.has_selector?("button.govuk-button--secondary") }
expect(form_containing_cancel).to have_field("selected_ids", type: :hidden, with: [log_1, log_2].map(&:id).join(" "))
expect(form_containing_cancel).to have_field("search", type: :hidden, with: "milk")
expect(form_containing_cancel[:method]).to eq "post"
expect(form_containing_cancel[:action]).to eq delete_logs_lettings_logs_path
end
context "when no logs are selected" do
let(:params) do
{
forms_delete_logs_form: {
log_type: :lettings,
log_ids: [log_1, log_2, log_3].map(&:id).join(" "),
},
}
end
before do
post delete_logs_confirmation_lettings_logs_path, params: params
end
it "renders the list of logs table again" do
expect(page.find("h1").text).to include "Review the logs you want to delete"
end
it "displays an error message" do
expect(page).to have_selector(".govuk-error-summary", text: "Select at least one log to delete or press cancel to return")
end
it "renders the table with all checkboxes unchecked" do
checkboxes = page.find_all("tbody tr").map { |row| row.find("input") }
checkboxes.each do |checkbox|
expect(checkbox).not_to be_checked
end
end
end
end
describe "DELETE delete-logs" do
let(:urban_chronotis) { create(:user, :data_provider, name: "Urban Chronotis") }
let(:log_1) { create(:lettings_log, :in_progress, created_by: urban_chronotis) }
let(:params) { { ids: [log_1.id, log_2.id] } }
before do
allow(urban_chronotis).to receive(:need_two_factor_authentication?).and_return(false)
sign_in urban_chronotis
end
context "when the user is authorized to delete the logs provided" do
let(:log_2) { create(:lettings_log, :completed, created_by: urban_chronotis) }
it "deletes the logs provided" do
delete delete_logs_lettings_logs_path, params: params
log_1.reload
expect(log_1.status).to eq "deleted"
expect(log_1.discarded_at).not_to be nil
log_2.reload
expect(log_2.status).to eq "deleted"
expect(log_2.discarded_at).not_to be nil
end
it "redirects to the lettings log index and displays a notice that the logs have been deleted" do
delete delete_logs_lettings_logs_path, params: params
expect(response).to redirect_to lettings_logs_path
follow_redirect!
expect(page).to have_selector(".govuk-notification-banner--success")
expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted")
end
end
context "when the user is not authorized to delete all the logs provided" do
let(:log_2) { create(:lettings_log, :completed) }
it "returns unauthorised and only deletes logs for which the user is authorised" do
delete delete_logs_lettings_logs_path, params: params
expect(response).to have_http_status(:unauthorized)
log_1.reload
expect(log_1.status).to eq "deleted"
expect(log_1.discarded_at).not_to be nil
log_2.reload
expect(log_2.discarded_at).to be nil
end
end
end
end

261
spec/requests/lettings_logs_controller_spec.rb

@ -1841,265 +1841,4 @@ RSpec.describe LettingsLogsController, type: :request do
end
end
end
describe "GET delete-logs" do
let(:page) { Capybara::Node::Simple.new(response.body) }
let(:user) { create(:user, name: "Richard MacDuff") }
let!(:log_1) { create(:lettings_log, :in_progress, created_by: user) }
let!(:log_2) { create(:lettings_log, :completed, created_by: user) }
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
end
it "calls the filter service with the filters in the session and the search term from the query params" do
search = "Schrödinger's cat"
logs_filters = {
"years" => [""],
"status" => ["", "in_progress"],
"user" => "all",
}
get lettings_logs_path(logs_filters) # adds the filters to the session
expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters
}.and_return LettingsLog.all
get delete_logs_lettings_logs_path(search:)
end
it "displays the logs returned by the filter service" do
get delete_logs_lettings_logs_path
table_body_rows = page.find_all("tbody tr")
expect(table_body_rows.count).to be 2
ids_in_table = table_body_rows.map { |row| row.first("td").text }
expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
end
it "checks all checkboxes by default" do
get delete_logs_lettings_logs_path
checkboxes = page.find_all("tbody tr").map { |row| row.find("input") }
expect(checkboxes.count).to be 2
expect(checkboxes).to all be_checked
end
end
describe "POST delete-logs" do
let(:page) { Capybara::Node::Simple.new(response.body) }
let(:user) { create(:user, name: "Richard MacDuff") }
let!(:log_1) { create(:lettings_log, :in_progress, created_by: user) }
let!(:log_2) { create(:lettings_log, :completed, created_by: user) }
let(:selected_ids) { log_1.id }
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
end
it "throws an error if selected ids are not provided" do
expect { post delete_logs_lettings_logs_path }.to raise_error ActionController::ParameterMissing
end
it "calls the filter service with the filters in the session and the search term from the query params" do
search = "Schrödinger's cat"
logs_filters = {
"years" => [""],
"status" => ["", "in_progress"],
"user" => "all",
}
get lettings_logs_path(logs_filters) # adds the filters to the session
expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters
}.and_return LettingsLog.all
post delete_logs_lettings_logs_path(search:, selected_ids:)
end
it "displays the logs returned by the filter service" do
post delete_logs_lettings_logs_path(selected_ids:)
table_body_rows = page.find_all("tbody tr")
expect(table_body_rows.count).to be 2
ids_in_table = table_body_rows.map { |row| row.first("td").text }
expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
end
it "only checks the selected checkboxes when selected_ids provided" do
post delete_logs_lettings_logs_path(selected_ids:)
checkboxes = page.find_all("tbody tr").map { |row| row.find("input") }
checkbox_expected_checked = checkboxes.find { |cb| cb.value == log_1.id.to_s }
checkbox_expected_unchecked = checkboxes.find { |cb| cb.value == log_2.id.to_s }
expect(checkbox_expected_checked).to be_checked
expect(checkbox_expected_unchecked).not_to be_checked
end
end
describe "POST delete-logs-confirmation" do
let(:page) { Capybara::Node::Simple.new(response.body) }
let(:user) { create(:user, name: "Urban Chronotis") }
let(:log_1) { create(:lettings_log, :in_progress) }
let(:log_2) { create(:lettings_log, :completed) }
let(:log_3) { create(:lettings_log, :in_progress) }
let(:params) do
{
forms_delete_logs_form: {
search_term: "milk",
selected_ids: [log_1, log_2].map(&:id),
},
}
end
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
post delete_logs_confirmation_lettings_logs_path, params: params
end
it "requires delete logs form data to be provided" do
expect { post delete_logs_confirmation_lettings_logs_path }.to raise_error(ActionController::ParameterMissing)
end
it "shows the correct title" do
expect(page.find("h1").text).to include "Are you sure you want to delete these logs?"
end
it "shows the correct information text to the user" do
expect(page).to have_selector("p", text: "You've selected 2 logs to delete")
end
context "when only one log is selected" do
let(:params) do
{
forms_delete_logs_form: {
search_term: "milk",
selected_ids: [log_1].map(&:id),
},
}
end
it "shows the correct information text to the user in the singular" do
post delete_logs_confirmation_lettings_logs_path, params: params
expect(page).to have_selector("p", text: "You've selected 1 log to delete")
end
end
it "shows a warning to the user" do
expect(page).to have_selector(".govuk-warning-text", text: "You will not be able to undo this action")
end
it "shows a button to delete the selected logs" do
expect(page).to have_selector("form.button_to button", text: "Delete logs")
end
it "the delete logs button submits the correct data to the correct path" do
form_containing_button = page.find("form.button_to")
expect(form_containing_button[:action]).to eq delete_logs_lettings_logs_path
expect(form_containing_button).to have_field "_method", type: :hidden, with: "delete"
expect(form_containing_button).to have_field "ids[]", type: :hidden, with: log_1.id
expect(form_containing_button).to have_field "ids[]", type: :hidden, with: log_2.id
end
it "shows a cancel button with the correct style" do
expect(page).to have_selector("button.govuk-button--secondary", text: "Cancel")
end
it "the cancel button submits the correct data to the correct path" do
form_containing_cancel = page.find_all("form").find { |form| form.has_selector?("button.govuk-button--secondary") }
expect(form_containing_cancel).to have_field("selected_ids", type: :hidden, with: [log_1, log_2].map(&:id).join(" "))
expect(form_containing_cancel).to have_field("search", type: :hidden, with: "milk")
expect(form_containing_cancel[:method]).to eq "post"
expect(form_containing_cancel[:action]).to eq delete_logs_lettings_logs_path
end
context "when no logs are selected" do
let(:params) do
{
forms_delete_logs_form: {
log_type: :lettings,
log_ids: [log_1, log_2, log_3].map(&:id).join(" "),
},
}
end
before do
post delete_logs_confirmation_lettings_logs_path, params: params
end
it "renders the list of logs table again" do
expect(page.find("h1").text).to include "Review the logs you want to delete"
end
it "displays an error message" do
expect(page).to have_selector(".govuk-error-summary", text: "Select at least one log to delete or press cancel to return")
end
it "renders the table with all checkboxes unchecked" do
checkboxes = page.find_all("tbody tr").map { |row| row.find("input") }
checkboxes.each do |checkbox|
expect(checkbox).not_to be_checked
end
end
end
end
describe "DELETE delete-logs" do
let(:page) { Capybara::Node::Simple.new(response.body) }
let(:urban_chronotis) { create(:user, :data_provider, name: "Urban Chronotis") }
let(:log_1) { create(:lettings_log, :in_progress, created_by: urban_chronotis) }
let(:params) { { ids: [log_1.id, log_2.id] } }
before do
allow(urban_chronotis).to receive(:need_two_factor_authentication?).and_return(false)
sign_in urban_chronotis
end
context "when the user is authorized to delete the logs provided" do
let(:log_2) { create(:lettings_log, :completed, created_by: urban_chronotis) }
it "deletes the logs provided" do
delete delete_logs_lettings_logs_path, params: params
log_1.reload
expect(log_1.status).to eq "deleted"
expect(log_1.discarded_at).not_to be nil
log_2.reload
expect(log_2.status).to eq "deleted"
expect(log_2.discarded_at).not_to be nil
end
it "redirects to the lettings log index and displays a notice that the logs have been deleted" do
delete delete_logs_lettings_logs_path, params: params
expect(response).to redirect_to lettings_logs_path
follow_redirect!
expect(page).to have_selector(".govuk-notification-banner--success")
expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted")
end
end
context "when the user is not authorized to delete all the logs provided" do
let(:log_2) { create(:lettings_log, :completed) }
it "returns unauthorised and only deletes logs for which the user is authorised" do
delete delete_logs_lettings_logs_path, params: params
expect(response).to have_http_status(:unauthorized)
log_1.reload
expect(log_1.status).to eq "deleted"
expect(log_1.discarded_at).not_to be nil
log_2.reload
expect(log_2.discarded_at).to be nil
end
end
end
end

Loading…
Cancel
Save