Browse Source
* Add LogPolicy * Add letting log delete flow * Add delete sales log flow * Test for presence of buttons * Use govuk_button_link_to * Move actions to helper * Allow deletion of delete and in progress logs * better handle 500 error * Soft delete logs * Move specs to shared spec file * Address comments * Add scoping for data coordinatorspull/1640/head
Jack
2 years ago
committed by
GitHub
25 changed files with 1112 additions and 627 deletions
@ -0,0 +1,46 @@ |
|||||||
|
module LogActionsHelper |
||||||
|
include GovukLinkHelper |
||||||
|
|
||||||
|
def edit_actions_for_log(log) |
||||||
|
back = back_button_for(log) |
||||||
|
delete = delete_button_for_log(log) |
||||||
|
|
||||||
|
return if back.nil? && delete.nil? |
||||||
|
|
||||||
|
content_tag(:div, class: "govuk-button-group") do |
||||||
|
safe_join([back, delete]) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
private |
||||||
|
|
||||||
|
def back_button_for(log) |
||||||
|
if log.completed? |
||||||
|
if log.bulk_uploaded? |
||||||
|
if log.lettings? |
||||||
|
govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_lettings_result_path(log.bulk_upload) |
||||||
|
else |
||||||
|
govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_sales_result_path(log.bulk_upload) |
||||||
|
end |
||||||
|
elsif log.lettings? |
||||||
|
govuk_button_link_to "Back to lettings logs", lettings_logs_path |
||||||
|
elsif log.sales? |
||||||
|
govuk_button_link_to "Back to sales logs", sales_logs_path |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
def policy_class_for(log) |
||||||
|
log.lettings? ? LettingsLogPolicy : SalesLogPolicy |
||||||
|
end |
||||||
|
|
||||||
|
def delete_button_for_log(log) |
||||||
|
if policy_class_for(log).new(current_user, log).destroy? |
||||||
|
govuk_button_link_to( |
||||||
|
"Delete log", |
||||||
|
lettings_log_delete_confirmation_path(log), |
||||||
|
warning: true, |
||||||
|
) |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
@ -0,0 +1,27 @@ |
|||||||
|
class LettingsLogPolicy |
||||||
|
attr_reader :user, :log |
||||||
|
|
||||||
|
def initialize(user, log) |
||||||
|
@user = user |
||||||
|
@log = log |
||||||
|
end |
||||||
|
|
||||||
|
def destroy? |
||||||
|
return false unless log && user |
||||||
|
|
||||||
|
# Can only delete editable logs |
||||||
|
return false unless log.collection_period_open? |
||||||
|
|
||||||
|
# Only delete logs with answered questions |
||||||
|
return false unless log.in_progress? || log.completed? |
||||||
|
|
||||||
|
# Support users can delete any log |
||||||
|
return true if user.support? |
||||||
|
|
||||||
|
# Data coordinators can delete any log visible to them |
||||||
|
return true if user.data_coordinator? && user.lettings_logs.visible.include?(log) |
||||||
|
|
||||||
|
# Data providers can only delete the log if it is assigned to them |
||||||
|
log.created_by == user |
||||||
|
end |
||||||
|
end |
||||||
@ -0,0 +1,27 @@ |
|||||||
|
class SalesLogPolicy |
||||||
|
attr_reader :user, :log |
||||||
|
|
||||||
|
def initialize(user, log) |
||||||
|
@user = user |
||||||
|
@log = log |
||||||
|
end |
||||||
|
|
||||||
|
def destroy? |
||||||
|
return false unless log && user |
||||||
|
|
||||||
|
# Can only delete editable logs |
||||||
|
return false unless log.collection_period_open? |
||||||
|
|
||||||
|
# Only delete logs with answered questions |
||||||
|
return false unless log.in_progress? || log.completed? |
||||||
|
|
||||||
|
# Support users can delete any log |
||||||
|
return true if user.support? |
||||||
|
|
||||||
|
# Data coordinators can delete any log visible to them |
||||||
|
return true if user.data_coordinator? && user.sales_logs.visible.include?(log) |
||||||
|
|
||||||
|
# Data providers can only delete the log if it is assigned to them |
||||||
|
log.created_by == user |
||||||
|
end |
||||||
|
end |
||||||
@ -0,0 +1,28 @@ |
|||||||
|
<% content_for :before_content do %> |
||||||
|
<% content_for :title, "Are you sure you want to delete this log?" %> |
||||||
|
<%= govuk_back_link href: @log.lettings? ? lettings_logs_path : sales_logs_path %> |
||||||
|
<% end %> |
||||||
|
|
||||||
|
<div class="govuk-grid-row"> |
||||||
|
<div class="govuk-grid-column-two-thirds-from-desktop"> |
||||||
|
<span class="govuk-caption-xl">Delete log <%= @log.id %></span> |
||||||
|
<h1 class="govuk-heading-xl"> |
||||||
|
<%= content_for(:title) %> |
||||||
|
</h1> |
||||||
|
|
||||||
|
<%= govuk_warning_text(text: "You will not be able to undo this action.") %> |
||||||
|
|
||||||
|
<div class="govuk-button-group"> |
||||||
|
<%= govuk_button_to( |
||||||
|
"Delete this log", |
||||||
|
@log.lettings? ? lettings_log_path(@log) : sales_log_path(@log), |
||||||
|
method: :delete, |
||||||
|
) %> |
||||||
|
<%= govuk_button_link_to( |
||||||
|
"Cancel", |
||||||
|
@log.lettings? ? lettings_log_path(@log) : sales_log_path(@log), |
||||||
|
secondary: true, |
||||||
|
) %> |
||||||
|
</div> |
||||||
|
</div> |
||||||
|
</div> |
||||||
@ -0,0 +1,6 @@ |
|||||||
|
class AddDiscardedAtToLogs < ActiveRecord::Migration[7.0] |
||||||
|
def change |
||||||
|
add_column :sales_logs, :discarded_at, :datetime |
||||||
|
add_column :lettings_logs, :discarded_at, :datetime |
||||||
|
end |
||||||
|
end |
||||||
|
|
@ -0,0 +1,114 @@ |
|||||||
|
require "rails_helper" |
||||||
|
|
||||||
|
RSpec.describe LettingsLogPolicy do |
||||||
|
subject(:policy) { described_class } |
||||||
|
|
||||||
|
permissions :destroy? do |
||||||
|
let(:log) { create(:lettings_log, :in_progress) } |
||||||
|
|
||||||
|
context "when log nil" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:collection_period_open?).and_return(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(policy).not_to permit(build(:user, :support), nil) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when user nil" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:collection_period_open?).and_return(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(policy).not_to permit(nil, build(:lettings_log, :in_progress)) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when collection period closed" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:collection_period_open?).and_return(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).not_to permit(build(:user, :support), log) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when collection period open" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:collection_period_open?).and_return(true) |
||||||
|
end |
||||||
|
|
||||||
|
context "when not started" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:in_progress?).and_return(false) |
||||||
|
allow(log).to receive(:completed?).and_return(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(log).to receive(:in_progress?) |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).not_to permit(build(:user, :support), log) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
[ |
||||||
|
%i[lettings_log in_progress], |
||||||
|
%i[lettings_log completed], |
||||||
|
].each do |type, status| |
||||||
|
let(:log) { create(type, status) } |
||||||
|
context "when #{type} status: #{status}" do |
||||||
|
context "when user is data coordinator" do |
||||||
|
let(:user) { create(:user, :data_coordinator) } |
||||||
|
let(:user_of_managing_org) { create(:user, :data_coordinator, organisation: log.managing_organisation) } |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).not_to permit(user, log) |
||||||
|
end |
||||||
|
|
||||||
|
it "allows deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).to permit(user_of_managing_org, log) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when user is support" do |
||||||
|
let(:user) { create(:user, :support) } |
||||||
|
|
||||||
|
it "does allow deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).to permit(user, log) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when user is data provider" do |
||||||
|
let(:user) { create(:user) } |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).not_to permit(user, log) |
||||||
|
end |
||||||
|
|
||||||
|
context "when the log is assigned to the user" do |
||||||
|
let(:log) { create(:lettings_log, :in_progress, created_by: user) } |
||||||
|
|
||||||
|
it "does allow deletion of log" do |
||||||
|
expect(policy).to permit(user, log) |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
@ -0,0 +1,114 @@ |
|||||||
|
require "rails_helper" |
||||||
|
|
||||||
|
RSpec.describe SalesLogPolicy do |
||||||
|
subject(:policy) { described_class } |
||||||
|
|
||||||
|
permissions :destroy? do |
||||||
|
let(:log) { create(:sales_log, :in_progress) } |
||||||
|
|
||||||
|
context "when log nil" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:collection_period_open?).and_return(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(policy).not_to permit(build(:user, :support), nil) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when user nil" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:collection_period_open?).and_return(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(policy).not_to permit(nil, build(:sales_log, :in_progress)) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when collection period closed" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:collection_period_open?).and_return(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).not_to permit(build(:user, :support), log) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when collection period open" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:collection_period_open?).and_return(true) |
||||||
|
end |
||||||
|
|
||||||
|
context "when not started" do |
||||||
|
before do |
||||||
|
allow(log).to receive(:in_progress?).and_return(false) |
||||||
|
allow(log).to receive(:completed?).and_return(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(log).to receive(:in_progress?) |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).not_to permit(build(:user, :support), log) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
[ |
||||||
|
%i[sales_log in_progress], |
||||||
|
%i[sales_log completed], |
||||||
|
].each do |type, status| |
||||||
|
let(:log) { create(type, status) } |
||||||
|
context "when #{type} status: #{status}" do |
||||||
|
context "when user is data coordinator" do |
||||||
|
let(:user) { create(:user, :data_coordinator) } |
||||||
|
let(:user_of_owning_org) { create(:user, :data_coordinator, organisation: log.owning_organisation) } |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).not_to permit(user, log) |
||||||
|
end |
||||||
|
|
||||||
|
it "allows deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).to permit(user_of_owning_org, log) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when user is support" do |
||||||
|
let(:user) { create(:user, :support) } |
||||||
|
|
||||||
|
it "does allow deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).to permit(user, log) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when user is data provider" do |
||||||
|
let(:user) { create(:user) } |
||||||
|
|
||||||
|
it "does not allow deletion of log" do |
||||||
|
expect(log).to receive(:collection_period_open?) |
||||||
|
|
||||||
|
expect(policy).not_to permit(user, log) |
||||||
|
end |
||||||
|
|
||||||
|
context "when the log is assigned to the user" do |
||||||
|
let(:log) { create(:sales_log, :in_progress, created_by: user) } |
||||||
|
|
||||||
|
it "does allow deletion of log" do |
||||||
|
expect(policy).to permit(user, log) |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
@ -0,0 +1,107 @@ |
|||||||
|
require "rails_helper" |
||||||
|
|
||||||
|
# rubocop:disable RSpec/AnyInstance |
||||||
|
RSpec.shared_examples "shared log examples" do |log_type| |
||||||
|
describe "status" do |
||||||
|
let(:empty_log) { create(log_type) } |
||||||
|
let(:in_progress_log) { create(log_type, :in_progress) } |
||||||
|
let(:completed_log) { create(log_type, :completed) } |
||||||
|
|
||||||
|
it "is set to not started for an empty #{log_type} log" do |
||||||
|
expect(empty_log.not_started?).to be(true) |
||||||
|
expect(empty_log.in_progress?).to be(false) |
||||||
|
expect(empty_log.completed?).to be(false) |
||||||
|
expect(empty_log.deleted?).to be(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "is set to in progress for a started #{log_type} log" do |
||||||
|
expect(in_progress_log.in_progress?).to be(true) |
||||||
|
expect(in_progress_log.not_started?).to be(false) |
||||||
|
expect(in_progress_log.completed?).to be(false) |
||||||
|
expect(in_progress_log.deleted?).to be(false) |
||||||
|
end |
||||||
|
|
||||||
|
it "is set to completed for a completed #{log_type} log" do |
||||||
|
expect(completed_log.in_progress?).to be(false) |
||||||
|
expect(completed_log.not_started?).to be(false) |
||||||
|
expect(completed_log.completed?).to be(true) |
||||||
|
expect(completed_log.deleted?).to be(false) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
describe "discard!" do |
||||||
|
around do |example| |
||||||
|
Timecop.freeze(Time.zone.local(2022, 1, 1)) do |
||||||
|
example.run |
||||||
|
end |
||||||
|
Timecop.return |
||||||
|
end |
||||||
|
|
||||||
|
let(:log) { create(log_type) } |
||||||
|
|
||||||
|
it "updates discarded at with current time" do |
||||||
|
expect { log.discard! }.to change { log.reload.discarded_at }.from(nil).to(Time.zone.now) |
||||||
|
end |
||||||
|
|
||||||
|
it "updates status to deleted" do |
||||||
|
expect { log.discard! }.to change { log.reload.status }.to("deleted") |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
describe "#process_uprn_change!" do |
||||||
|
context "when UPRN set to a value" do |
||||||
|
let(:log) do |
||||||
|
create( |
||||||
|
log_type, |
||||||
|
uprn: "123456789", |
||||||
|
uprn_confirmed: 1, |
||||||
|
county: "county", |
||||||
|
) |
||||||
|
end |
||||||
|
|
||||||
|
it "updates log fields" do |
||||||
|
log.uprn = "1111111" |
||||||
|
|
||||||
|
allow_any_instance_of(UprnClient).to receive(:call) |
||||||
|
allow_any_instance_of(UprnClient).to receive(:result).and_return({ |
||||||
|
"UPRN" => "UPRN", |
||||||
|
"UDPRN" => "UDPRN", |
||||||
|
"ADDRESS" => "full address", |
||||||
|
"SUB_BUILDING_NAME" => "0", |
||||||
|
"BUILDING_NAME" => "building name", |
||||||
|
"THOROUGHFARE_NAME" => "thoroughfare", |
||||||
|
"POST_TOWN" => "posttown", |
||||||
|
"POSTCODE" => "postcode", |
||||||
|
}) |
||||||
|
|
||||||
|
expect { log.process_uprn_change! }.to change(log, :address_line1).from(nil).to("0, Building Name, Thoroughfare") |
||||||
|
.and change(log, :town_or_city).from(nil).to("Posttown") |
||||||
|
.and change(log, :postcode_full).from(nil).to("POSTCODE") |
||||||
|
.and change(log, :uprn_confirmed).from(1).to(nil) |
||||||
|
.and change(log, :county).from("county").to(nil) |
||||||
|
.and change(log, :uprn_known).from(nil).to(1) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when UPRN nil" do |
||||||
|
let(:log) { create(log_type, uprn: nil) } |
||||||
|
|
||||||
|
it "does not update log" do |
||||||
|
expect { log.process_uprn_change! }.not_to change(log, :attributes) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when service errors" do |
||||||
|
let(:log) { create(log_type, uprn: "123456789", uprn_confirmed: 1) } |
||||||
|
let(:error_message) { "error" } |
||||||
|
|
||||||
|
it "adds error to log" do |
||||||
|
allow_any_instance_of(UprnClient).to receive(:call) |
||||||
|
allow_any_instance_of(UprnClient).to receive(:error).and_return(error_message) |
||||||
|
|
||||||
|
expect { log.process_uprn_change! }.to change { log.errors[:uprn] }.from([]).to([error_message]) |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
# rubocop:enable RSpec/AnyInstance |
||||||
Loading…
Reference in new issue