Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forms Refactoring -Add ability to reference single document in collection #512

Merged
merged 8 commits into from
Oct 15, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module WasteCarriersEngine
module CanHaveRegistrationAttributes
extend ActiveSupport::Concern
include Mongoid::Document
include CanReferenceSingleDocumentInCollection

# Rubocop sees a module as a block, and as such is not very forgiving in how
# many lines it allows. In the case of this concern we have to list out all
Expand All @@ -17,7 +18,10 @@ module CanHaveRegistrationAttributes
# for comments in some places, and putting them on the line above breaks
# the formatting we have in place.
# rubocop:disable Metrics/LineLength
embeds_many :addresses, class_name: "WasteCarriersEngine::Address"
embeds_many :addresses, class_name: "WasteCarriersEngine::Address"
reference_one :contact_address, collection: :addresses, find_by: { address_type: "POSTAL" }
cintamani marked this conversation as resolved.
Show resolved Hide resolved
reference_one :registered_address, collection: :addresses, find_by: { address_type: "REGISTERED" }

embeds_one :conviction_search_result, class_name: "WasteCarriersEngine::ConvictionSearchResult"
embeds_many :conviction_sign_offs, class_name: "WasteCarriersEngine::ConvictionSignOff"
embeds_one :finance_details, class_name: "WasteCarriersEngine::FinanceDetails", store_as: "financeDetails"
Expand Down Expand Up @@ -81,18 +85,6 @@ module CanHaveRegistrationAttributes
"addresses.postcode": /#{term}/i)
}

def contact_address
return nil unless addresses.present?

addresses.where(address_type: "POSTAL").first
end

def registered_address
return nil unless addresses.present?

addresses.where(address_type: "REGISTERED").first
end

def charity?
business_type == "charity"
end
Expand All @@ -109,14 +101,10 @@ def company_no_required?
end

def main_people
return [] unless key_people.present?
cintamani marked this conversation as resolved.
Show resolved Hide resolved

key_people.where(person_type: "KEY")
end

def relevant_people
return [] unless key_people.present?

key_people.where(person_type: "RELEVANT")
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# frozen_string_literal: true

module WasteCarriersEngine
module CanReferenceSingleDocumentInCollection
cintamani marked this conversation as resolved.
Show resolved Hide resolved
extend ActiveSupport::Concern

class_methods do
def reference_one(attribute_name, collection:, find_by:)
define_method(attribute_name) do
retrieve_attribute(attribute_name, collection, find_by)
end

define_method("#{attribute_name}=") do |new_object|
assign_attribute(attribute_name, collection, new_object)
end
end
end

included do
def retrieve_attribute(attribute_name, collection, find_by)
instance_variable_get("@#{attribute_name}") ||
fetch_attribute(collection, find_by)
end

# rubocop:disable Lint/UnusedMethodArgument
def assign_attribute(attribute_name, collection, new_object)
send(attribute_name)&.delete

instance_eval("#{collection} << new_object", __FILE__, __LINE__)

instance_variable_set("@#{attribute_name}", nil)
end
# rubocop:enable Lint/UnusedMethodArgument

def fetch_attribute(collection, find_by)
criteria = instance_eval("#{collection}.criteria", __FILE__, __LINE__)

criteria.where(find_by).first
end
end
end
end
10 changes: 5 additions & 5 deletions app/models/waste_carriers_engine/transient_registration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,11 @@ def registration_type_changed?
# Don't compare registration types if the new one hasn't been set
return false unless registration_type

original_registration_type = Registration.where(reg_identifier: reg_identifier).first.registration_type
original_registration_type != registration_type
registration.registration_type != registration_type
end

def registration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 ❤️

Registration.where(reg_identifier: reg_identifier).first
end

def fee_including_possible_type_change
Expand Down Expand Up @@ -76,7 +79,6 @@ def total_registration_card_charge
def company_no_changed?
return false unless company_no_required?

registration = Registration.where(reg_identifier: reg_identifier).first
# LLP is a new business type, so users who previously were forced to select 'partnership' would not have had the
# opportunity to enter a company_no. Therefore we have nothing to compare against and should allow users to
# continue the renewal journey.
Expand Down Expand Up @@ -156,8 +158,6 @@ def copy_data_from_registration
# Don't try to get Registration data with an invalid reg_identifier
return unless valid? && new_record?

registration = Registration.where(reg_identifier: reg_identifier).first

# Don't copy object IDs as Mongo should generate new unique ones
# Don't copy smart answers as we want users to use the latest version of the questions
attributes = registration.attributes.except("_id",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ module WasteCarriersEngine
end

it "does not modify the existing contact address" do
old_contact_address = transient_registration.contact_address
old_contact_address = transient_registration.reload.contact_address
post company_address_manual_forms_path, company_address_manual_form: valid_params
expect(transient_registration.reload.contact_address).to eq(old_contact_address)
end
Expand Down
29 changes: 29 additions & 0 deletions spec/support/shared_examples/can_have_registration_attributes.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
# frozen_string_literal: true

RSpec.shared_examples "Can have registration attributes" do
include_examples(
"Can reference single document in collection",
proc { create(:transient_registration, :has_required_data, :has_addresses) },
:contact_address,
proc { subject.addresses.find_by(address_type: "POSTAL") },
WasteCarriersEngine::Address.new,
:addresses
)

describe "#charity?" do
let(:transient_registration) { build(:transient_registration) }

Expand All @@ -19,6 +28,26 @@
end
end

describe "#contact_address" do
let(:contact_address) { build(:address, :contact) }
let(:transient_registration) { build(:transient_registration, addresses: [contact_address]) }

it "returns the address of type contact" do
expect(transient_registration.contact_address).to eq(contact_address)
end
end

describe "#contact_address=" do
let(:contact_address) { build(:address) }
let(:transient_registration) { build(:transient_registration, addresses: []) }

it "set an address of type contact" do
transient_registration.contact_address = contact_address

expect(transient_registration.addresses).to eq([contact_address])
end
end

describe "#company_no_required?" do
let(:transient_registration) { build(:transient_registration) }

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

RSpec.shared_examples "Can reference single document in collection" do |subject_lambda, attribute, object_from_collection, new_object_for_collection, collection|
subject { instance_eval(&subject_lambda) }

describe ".reference_one" do
it "defines an attr getter for the given attribute" do
expect(subject).to respond_to(attribute.to_s)
end

it "defines an attr setter for the given attribute" do
expect(subject).to respond_to("#{attribute}=")
end
end

describe "##{attribute}" do
it "returns the correct object from the collection" do
expect(subject.send(attribute)).to eq(instance_eval(&object_from_collection))
end
end

describe "##{attribute}=" do
it "updates the object's collection with the new object" do
size = subject.send(collection).size

expect(subject.send(collection)).to_not include(new_object_for_collection)

subject.send("#{attribute}=", new_object_for_collection)

expect(subject.send(collection)).to include(new_object_for_collection)
expect(subject.send(collection).size).to eq(size)
end
end
end