Browse Source

Merge branch 'main' into CLDC-2262-clear-validations-data-when-correcting

pull/1570/head
natdeanlewissoftwire 3 years ago
parent
commit
317d935b32
  1. 3
      Gemfile
  2. 13
      Gemfile.lock
  3. 8
      app/controllers/application_controller.rb
  4. 8
      app/controllers/bulk_upload_lettings_results_controller.rb
  5. 10
      app/controllers/bulk_upload_sales_results_controller.rb
  6. 69
      app/models/derived_variables/sales_log_variables.rb
  7. 12
      app/models/form/sales/pages/buyer1_live_in_property.rb
  8. 14
      app/models/form/sales/pages/buyer2_live_in_property.rb
  9. 2
      app/models/form/sales/pages/buyer_company.rb
  10. 26
      app/models/sales_log.rb
  11. 10
      app/models/validations/sales/household_validations.rb
  12. 26
      app/policies/bulk_upload_policy.rb
  13. 3
      config/locales/en.yml
  14. 23
      spec/controllers/application_controller_spec.rb
  15. 13
      spec/controllers/bulk_upload_lettings_results_controller_spec.rb
  16. 33
      spec/controllers/bulk_upload_sales_results_controller_spec.rb
  17. 21
      spec/models/form/sales/pages/buyer1_live_in_property_spec.rb
  18. 14
      spec/models/form/sales/pages/buyer2_live_in_property_spec.rb
  19. 4
      spec/models/form/sales/pages/buyer_company_spec.rb
  20. 64
      spec/models/sales_log_spec.rb
  21. 57
      spec/models/validations/sales/household_validations_spec.rb
  22. 39
      spec/policies/bulk_upload_policy_spec.rb
  23. 1
      spec/rails_helper.rb
  24. 41
      spec/requests/bulk_upload_lettings_results_controller_spec.rb

3
Gemfile

@ -48,6 +48,9 @@ gem "aws-sdk-s3"
gem "paper_trail"
# Store active record objects in version whodunnits
gem "paper_trail-globalid"
gem "pundit"
# Request rate limiting
gem "rack", ">= 2.2.6.3"
gem "rack-attack"

13
Gemfile.lock

@ -135,7 +135,7 @@ GEM
childprocess (4.1.0)
coderay (1.1.3)
concurrent-ruby (1.2.2)
connection_pool (2.3.0)
connection_pool (2.4.0)
crack (0.4.5)
rexml
crass (1.0.6)
@ -277,9 +277,11 @@ GEM
public_suffix (5.0.1)
puma (5.6.5)
nio4r (~> 2.0)
pundit (2.3.0)
activesupport (>= 3.0.0)
raabro (1.4.0)
racc (1.6.2)
rack (2.2.6.4)
rack (2.2.7)
rack-attack (6.6.1)
rack (>= 1.0, < 3)
rack-mini-profiler (2.3.4)
@ -320,7 +322,7 @@ GEM
ffi (~> 1.0)
redcarpet (3.6.0)
redis (4.8.1)
redis-client (0.12.2)
redis-client (0.14.1)
connection_pool
regexp_parser (2.7.0)
request_store (1.5.1)
@ -390,11 +392,11 @@ GEM
sentry-ruby (~> 5.8.0)
sentry-ruby (5.8.0)
concurrent-ruby (~> 1.0, >= 1.0.2)
sidekiq (7.0.5)
sidekiq (7.1.0)
concurrent-ruby (< 2)
connection_pool (>= 2.3.0)
rack (>= 2.2.4)
redis-client (>= 0.11.0)
redis-client (>= 0.14.0)
sidekiq-cron (1.9.1)
fugit (~> 1.8)
sidekiq (>= 4.2.1)
@ -478,6 +480,7 @@ DEPENDENCIES
propshaft
pry-byebug
puma (~> 5.0)
pundit
rack (>= 2.2.6.3)
rack-attack
rack-mini-profiler (~> 2.0)

8
app/controllers/application_controller.rb

@ -1,10 +1,18 @@
class ApplicationController < ActionController::Base
include Pundit::Authorization
rescue_from Pundit::NotAuthorizedError, with: :render_not_authorized
before_action :set_paper_trail_whodunnit
def render_not_found
render "errors/not_found", status: :not_found
end
def render_not_authorized
render "errors/not_found", status: :unauthorized
end
def render_not_found_json(class_name, id)
render json: { error: "#{class_name} #{id} not found" }, status: :not_found
end

8
app/controllers/bulk_upload_lettings_results_controller.rb

@ -4,7 +4,9 @@ class BulkUploadLettingsResultsController < ApplicationController
rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
def show
@bulk_upload = current_user.bulk_uploads.lettings.find(params[:id])
@bulk_upload = BulkUpload.lettings.find(params[:id])
authorize @bulk_upload
end
def resume
@ -20,7 +22,9 @@ class BulkUploadLettingsResultsController < ApplicationController
end
def summary
@bulk_upload = current_user.bulk_uploads.lettings.find(params[:id])
@bulk_upload = BulkUpload.lettings.find(params[:id])
authorize @bulk_upload
end
private

10
app/controllers/bulk_upload_sales_results_controller.rb

@ -4,7 +4,9 @@ class BulkUploadSalesResultsController < ApplicationController
rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
def show
@bulk_upload = current_user.bulk_uploads.sales.find(params[:id])
@bulk_upload = BulkUpload.sales.find(params[:id])
authorize @bulk_upload
end
def resume
@ -20,9 +22,13 @@ class BulkUploadSalesResultsController < ApplicationController
end
def summary
@bulk_upload = current_user.bulk_uploads.sales.find(params[:id])
@bulk_upload = BulkUpload.sales.find(params[:id])
authorize @bulk_upload
end
private
def reset_logs_filters
session["logs_filters"] = {}.to_json
end

69
app/models/derived_variables/sales_log_variables.rb

@ -1,5 +1,7 @@
module DerivedVariables::SalesLogVariables
def set_derived_fields!
reset_invalidated_derived_values!
self.ethnic = 17 if ethnic_refused?
self.mscharge = nil if no_monthly_leasehold_charges?
if exdate.present?
@ -13,9 +15,6 @@ module DerivedVariables::SalesLogVariables
self.hoyear = hodate.year
end
self.deposit = value if outright_sale? && mortgage_not_used?
if mortgage_not_used?
self.mortgage = 0
end
self.pcode1, self.pcode2 = postcode_full.split(" ") if postcode_full.present?
self.totchild = total_child
self.totadult = total_adult + total_elder
@ -30,10 +29,74 @@ module DerivedVariables::SalesLogVariables
self.uprn = nil
self.uprn_known = 0
end
set_encoded_derived_values!
end
private
DEPENDENCIES = [
{
conditions: {
buylivein: 2,
},
derived_values: {
buy1livein: 2,
},
},
{
conditions: {
buylivein: 2,
jointpur: 1,
},
derived_values: {
buy1livein: 2,
buy2livein: 2,
},
},
{
conditions: {
buylivein: 1,
jointpur: 2,
},
derived_values: {
buy1livein: 1,
},
},
{
conditions: {
mortgageused: 2,
},
derived_values: {
mortgage: 0,
},
},
].freeze
def reset_invalidated_derived_values!
DEPENDENCIES.each do |dependency|
any_conditions_changed = dependency[:conditions].any? { |attribute, _value| send("#{attribute}_changed?") }
next unless any_conditions_changed
previously_in_derived_state = dependency[:conditions].all? { |attribute, value| send("#{attribute}_was") == value }
next unless previously_in_derived_state
dependency[:derived_values].each do |derived_attribute, _derived_value|
Rails.logger.debug("Cleared derived #{derived_attribute} value")
send("#{derived_attribute}=", nil)
end
end
end
def set_encoded_derived_values!
DEPENDENCIES.each do |dependency|
derivation_applies = dependency[:conditions].all? { |attribute, value| send(attribute) == value }
if derivation_applies
dependency[:derived_values].each { |attribute, value| send("#{attribute}=", value) }
end
end
end
def number_of_household_members
return unless hholdcount.present? && jointpur.present?

12
app/models/form/sales/pages/buyer1_live_in_property.rb

@ -5,9 +5,21 @@ class Form::Sales::Pages::Buyer1LiveInProperty < ::Form::Page
@depends_on = [
{
"buyer_has_seen_privacy_notice?" => true,
"outright_sale?" => false,
},
{
"buyer_not_interviewed?" => true,
"outright_sale?" => false,
},
{
"buyer_has_seen_privacy_notice?" => true,
"joint_purchase?" => true,
"buyers_will_live_in?" => true,
},
{
"buyer_not_interviewed?" => true,
"joint_purchase?" => true,
"buyers_will_live_in?" => true,
},
]
end

14
app/models/form/sales/pages/buyer2_live_in_property.rb

@ -4,12 +4,24 @@ class Form::Sales::Pages::Buyer2LiveInProperty < ::Form::Page
@id = "buyer_2_live_in_property"
@depends_on = [
{
"joint_purchase?" => true,
"buyer_has_seen_privacy_notice?" => true,
"outright_sale?" => false,
"joint_purchase?" => true,
},
{
"buyer_not_interviewed?" => true,
"outright_sale?" => false,
"joint_purchase?" => true,
},
{
"buyer_has_seen_privacy_notice?" => true,
"joint_purchase?" => true,
"buyers_will_live_in?" => true,
},
{
"buyer_not_interviewed?" => true,
"joint_purchase?" => true,
"buyers_will_live_in?" => true,
},
]
end

2
app/models/form/sales/pages/buyer_company.rb

@ -3,7 +3,7 @@ class Form::Sales::Pages::BuyerCompany < ::Form::Page
super
@id = "buyer_company"
@depends_on = [{
"ownershipsch" => 3,
"outright_sale?" => true,
}]
end

26
app/models/sales_log.rb

@ -25,7 +25,6 @@ class SalesLog < Log
validates_with SalesLogValidator
before_validation :recalculate_start_year!, if: :saledate_changed?
before_validation :reset_invalidated_dependent_fields!
before_validation :process_postcode_changes!, if: :postcode_full_changed?
before_validation :process_previous_postcode_changes!, if: :ppostcode_full_changed?
before_validation :reset_location_fields!, unless: :postcode_known?
@ -160,10 +159,26 @@ class SalesLog < Log
inc1mort == 1
end
def buyers_will_live_in?
buylivein == 1
end
def buyers_will_not_live_in?
buylivein == 2
end
def buyer_two_will_live_in_property?
buy2livein == 1
end
def buyer_two_will_not_live_in_property?
buy2livein == 2
end
def buyer_one_will_not_live_in_property?
buy1livein == 2
end
def buyer_two_not_already_living_in_property?
buy2living == 2
end
@ -351,12 +366,9 @@ class SalesLog < Log
def ownership_scheme
case ownershipsch
when 1
"shared ownership"
when 2
"discounted ownership"
when 3
"outright sale"
when 1 then "shared ownership"
when 2 then "discounted ownership"
when 3 then "outright sale"
end
end

10
app/models/validations/sales/household_validations.rb

@ -20,6 +20,16 @@ module Validations::Sales::HouseholdValidations
end
end
def validate_buyers_living_in_property(record)
return unless record.form.start_date.year >= 2023
if record.buyers_will_live_in? && record.buyer_one_will_not_live_in_property? && record.buyer_two_will_not_live_in_property?
record.errors.add :buylivein, I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent_setup")
record.errors.add :buy1livein, I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent")
record.errors.add :buy2livein, I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent")
end
end
private
def validate_person_age_matches_relationship(record, person_num)

26
app/policies/bulk_upload_policy.rb

@ -0,0 +1,26 @@
class BulkUploadPolicy
attr_reader :user, :bulk_upload
def initialize(user, bulk_upload)
@user = user
@bulk_upload = bulk_upload
end
def summary?
owner? || same_org? || user.support?
end
def show?
owner? || same_org? || user.support?
end
private
def owner?
bulk_upload.user == user
end
def same_org?
bulk_upload.user.organisation.users.include?(user)
end
end

3
config/locales/en.yml

@ -443,6 +443,9 @@ en:
no_choices: "You cannot answer this question as you told us nobody in the household has a physical or mental health condition (or other illness) expected to last 12 months or more"
postcode:
discounted_ownership: "Last settled accommodation and discounted ownership property postcodes must match"
buylivein:
buyers_will_live_in_property_values_inconsistent_setup: "You have already told us that both buyer 1 and buyer 2 will not live in the property"
buyers_will_live_in_property_values_inconsistent: "You have already told us that the buyers will live in the property. Either buyer 1 or buyer 2 must live in the property"
tenancy:
length:

23
spec/controllers/application_controller_spec.rb

@ -0,0 +1,23 @@
require "rails_helper"
RSpec.describe ApplicationController do
describe "when Pundit::NotAuthorizedError raised" do
render_views
controller do
def index
raise Pundit::NotAuthorizedError, "error goes here"
end
end
it "returns status 401 unauthorized" do
get :index
expect(response).to be_unauthorized
end
it "renders page not found" do
get :index
expect(response.body).to have_content("Page not found")
end
end
end

13
spec/controllers/bulk_upload_lettings_results_controller_spec.rb

@ -5,6 +5,19 @@ RSpec.describe BulkUploadLettingsResultsController do
sign_in user
end
describe "#show" do
let(:user) { create(:user) }
let(:bulk_upload) { create(:bulk_upload, :lettings, user:) }
it "passes thru pundit" do
allow(controller).to receive(:authorize)
get :show, params: { id: bulk_upload.id }
expect(controller).to have_received(:authorize)
end
end
describe "GET #resume /lettings-logs/bulk-upload-results/:ID/resume" do
let(:user) { create(:user) }
let(:bulk_upload) { create(:bulk_upload, :lettings, user:) }

33
spec/controllers/bulk_upload_sales_results_controller_spec.rb

@ -0,0 +1,33 @@
require "rails_helper"
RSpec.describe BulkUploadSalesResultsController do
before do
sign_in user
end
describe "#show" do
let(:user) { create(:user) }
let(:bulk_upload) { create(:bulk_upload, :sales, user:) }
it "passes thru pundit" do
allow(controller).to receive(:authorize)
get :show, params: { id: bulk_upload.id }
expect(controller).to have_received(:authorize)
end
end
describe "#summary" do
let(:user) { create(:user) }
let(:bulk_upload) { create(:bulk_upload, :sales, user:) }
it "passes thru pundit" do
allow(controller).to receive(:authorize)
get :summary, params: { id: bulk_upload.id }
expect(controller).to have_received(:authorize)
end
end
end

21
spec/models/form/sales/pages/buyer1_live_in_property_spec.rb

@ -28,6 +28,25 @@ RSpec.describe Form::Sales::Pages::Buyer1LiveInProperty, type: :model do
end
it "has correct depends_on" do
expect(page.depends_on).to eq([{ "buyer_has_seen_privacy_notice?" => true }, { "buyer_not_interviewed?" => true }])
expect(page.depends_on).to eq([
{
"buyer_has_seen_privacy_notice?" => true,
"outright_sale?" => false,
},
{
"buyer_not_interviewed?" => true,
"outright_sale?" => false,
},
{
"buyer_has_seen_privacy_notice?" => true,
"joint_purchase?" => true,
"buyers_will_live_in?" => true,
},
{
"buyer_not_interviewed?" => true,
"joint_purchase?" => true,
"buyers_will_live_in?" => true,
},
])
end
end

14
spec/models/form/sales/pages/buyer2_live_in_property_spec.rb

@ -30,12 +30,24 @@ RSpec.describe Form::Sales::Pages::Buyer2LiveInProperty, type: :model do
it "has correct depends_on" do
expect(page.depends_on).to eq([
{
"joint_purchase?" => true,
"buyer_has_seen_privacy_notice?" => true,
"outright_sale?" => false,
"joint_purchase?" => true,
},
{
"buyer_not_interviewed?" => true,
"outright_sale?" => false,
"joint_purchase?" => true,
},
{
"buyer_has_seen_privacy_notice?" => true,
"joint_purchase?" => true,
"buyers_will_live_in?" => true,
},
{
"buyer_not_interviewed?" => true,
"joint_purchase?" => true,
"buyers_will_live_in?" => true,
},
])
end

4
spec/models/form/sales/pages/buyer_company_spec.rb

@ -28,8 +28,6 @@ RSpec.describe Form::Sales::Pages::BuyerCompany, type: :model do
end
it "has correct depends_on" do
expect(page.depends_on).to eq([{
"ownershipsch" => 3,
}])
expect(page.depends_on).to eq([{ "outright_sale?" => true }])
end
end

64
spec/models/sales_log_spec.rb

@ -220,6 +220,70 @@ RSpec.describe SalesLog, type: :model do
record_from_db = ActiveRecord::Base.connection.execute("select mortgage from sales_logs where id=#{sales_log.id}").to_a[0]
expect(record_from_db["mortgage"]).to eq(0.0)
end
it "clears mortgage value if mortgage used is changed from no to yes" do
# to avoid log failing validations when mortgage value is removed:
new_grant_value = sales_log.grant + sales_log.mortgage
sales_log.update!(mortgageused: 2, grant: new_grant_value)
sales_log.update!(mortgageused: 1)
record_from_db = ActiveRecord::Base.connection.execute("select mortgage from sales_logs where id=#{sales_log.id}").to_a[0]
expect(record_from_db["mortgage"]).to eq(nil)
end
context "when outright sale and buyers will live in the property" do
let(:sales_log) { create(:sales_log, :outright_sale_setup_complete, buylivein: 1, jointpur:) }
context "and the sale is not a joint purchase" do
let(:jointpur) { 2 }
it "derives that buyer 1 will live in the property" do
expect(sales_log.buy1livein).to be 1
end
it "does not derive a value for whether buyer 2 will live in the property" do
expect(sales_log.buy2livein).to be nil
end
it "clears that buyer 1 will live in the property if joint purchase is updated" do
sales_log.update!(jointpur: 1)
expect(sales_log.buy1livein).to be nil
end
end
context "and the sale is a joint purchase" do
let(:jointpur) { 1 }
it "does not derive values for whether buyer 1 or buyer 2 will live in the property" do
expect(sales_log.buy1livein).to be nil
expect(sales_log.buy2livein).to be nil
end
end
end
context "when outright sale and buyers will not live in the property" do
let(:sales_log) { create(:sales_log, :outright_sale_setup_complete, buylivein: 2, jointpur:) }
context "and the sale is not a joint purchase" do
let(:jointpur) { 2 }
it "derives that buyer 1 will not live in the property" do
expect(sales_log.buy1livein).to be 2
end
it "does not derive a value for whether buyer 2 will live in the property" do
expect(sales_log.buy2livein).to be nil
end
end
context "and the sale is a joint purchase" do
let(:jointpur) { 1 }
it "derives that neither buyer 1 nor buyer 2 will live in the property" do
expect(sales_log.buy1livein).to be 2
expect(sales_log.buy2livein).to be 2
end
end
end
end
context "when saving addresses" do

57
spec/models/validations/sales/household_validations_spec.rb

@ -201,4 +201,61 @@ RSpec.describe Validations::Sales::HouseholdValidations do
end
end
end
describe "validating fields about buyers living in the property" do
let(:sales_log) { FactoryBot.create(:sales_log, :outright_sale_setup_complete, noint: 1, companybuy: 2, buylivein:, jointpur:, jointmore:, buy1livein:) }
context "when buyers will live in the property and the sale is a joint purchase" do
let(:buylivein) { 1 }
let(:jointpur) { 1 }
let(:jointmore) { 2 }
context "and buyer one will live in the property" do
let(:buy1livein) { 1 }
it "does not add validations regardless of whether buyer two will live in the property" do
sales_log.buy2livein = 1
household_validator.validate_buyers_living_in_property(sales_log)
expect(sales_log.errors).to be_empty
sales_log.buy2livein = 2
household_validator.validate_buyers_living_in_property(sales_log)
expect(sales_log.errors).to be_empty
end
end
context "and buyer one will not live in the property" do
let(:buy1livein) { 2 }
it "does not add validations if buyer two will live in the property or if we do not yet know" do
sales_log.buy2livein = 1
household_validator.validate_buyers_living_in_property(sales_log)
expect(sales_log.errors).to be_empty
sales_log.buy2livein = nil
household_validator.validate_buyers_living_in_property(sales_log)
expect(sales_log.errors).to be_empty
end
it "triggers a validation if buyer two will also not live in the property" do
sales_log.buy2livein = 2
household_validator.validate_buyers_living_in_property(sales_log)
expect(sales_log.errors[:buylivein]).to include I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent_setup")
expect(sales_log.errors[:buy2livein]).to include I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent")
expect(sales_log.errors[:buy1livein]).to include I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent")
end
end
context "and we don't know whether buyer one will live in the property" do
let(:buy1livein) { nil }
it "does not add validations regardless of whether buyer two will live in the property" do
sales_log.buy2livein = 1
household_validator.validate_buyers_living_in_property(sales_log)
expect(sales_log.errors).to be_empty
sales_log.buy2livein = 2
household_validator.validate_buyers_living_in_property(sales_log)
expect(sales_log.errors).to be_empty
end
end
end
end
end

39
spec/policies/bulk_upload_policy_spec.rb

@ -0,0 +1,39 @@
require "rails_helper"
RSpec.describe BulkUploadPolicy do
subject(:policy) { described_class }
permissions :summary?, :show? do
it "grants access to owner" do
user = build(:user)
bulk_upload = build(:bulk_upload, user:)
expect(policy).to permit(user, bulk_upload)
end
it "grants access to user from same org as uploader" do
user = create(:user)
organisation = user.organisation
other_user = create(:user, organisation:)
bulk_upload = create(:bulk_upload, user:)
expect(policy).to permit(other_user, bulk_upload)
end
it "grants access to support" do
user = create(:user)
support_user = create(:user, :support)
bulk_upload = create(:bulk_upload, user:)
expect(policy).to permit(support_user, bulk_upload)
end
it "denies access to random users" do
user = create(:user)
other_user = create(:user)
bulk_upload = create(:bulk_upload, user:)
expect(policy).not_to permit(other_user, bulk_upload)
end
end
end

1
spec/rails_helper.rb

@ -9,6 +9,7 @@ require "capybara/rspec"
require "capybara-screenshot/rspec"
require "selenium-webdriver"
require "view_component/test_helpers"
require "pundit/rspec"
Capybara.register_driver :headless do |app|
options = Selenium::WebDriver::Firefox::Options.new

41
spec/requests/bulk_upload_lettings_results_controller_spec.rb

@ -2,11 +2,13 @@ require "rails_helper"
RSpec.describe BulkUploadLettingsResultsController, type: :request do
let(:user) { create(:user) }
let(:support_user) { create(:user, :support) }
let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:) }
let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2) }
let(:viewing_user) { user }
before do
sign_in user
sign_in viewing_user
end
describe "GET /lettings-logs/bulk-upload-results/:ID/summary" do
@ -22,6 +24,43 @@ RSpec.describe BulkUploadLettingsResultsController, type: :request do
expect(response.body).to include(bulk_upload.filename)
end
context "when viewed by support user" do
before do
allow(support_user).to receive(:need_two_factor_authentication?).and_return(false)
end
let(:viewing_user) { support_user }
it "is accessible" do
get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary"
expect(response).to be_successful
expect(response.body).to include(bulk_upload.filename)
end
end
context "when viewed by some other random user" do
let(:other_user) { create(:user) }
let(:viewing_user) { other_user }
it "is not accessible" do
get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary"
expect(response).to be_unauthorized
end
end
context "when viewed by another user in the same org" do
let(:other_user) { create(:user, organisation: user.organisation) }
let(:viewing_user) { other_user }
it "is accessible" do
get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary"
expect(response).to be_successful
expect(response.body).to include(bulk_upload.filename)
end
end
end
describe "GET /lettings-logs/bulk-upload-results/:ID" do

Loading…
Cancel
Save