From 7a59bca9179f4d18669e72f015ac28aa8ac1d59a Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Fri, 2 Mar 2018 16:11:44 +0200 Subject: [PATCH 01/11] Improve readability #746 --- test/models/domain/transferable_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/domain/transferable_test.rb b/test/models/domain/transferable_test.rb index 42613aa45..35bf5f96b 100644 --- a/test/models/domain/transferable_test.rb +++ b/test/models/domain/transferable_test.rb @@ -34,7 +34,7 @@ class DomainTransferableTest < ActiveSupport::TestCase assert_equal '1bad4f', domain.transfer_code end - def test_changes_registrar + def test_assigns_new_registrar @domain.transfer(@new_registrar) assert_equal @new_registrar, @domain.registrar end From 998ef259bae623144a7db78178765065bb671719 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sat, 3 Mar 2018 14:33:42 +0200 Subject: [PATCH 02/11] Improve readability #746 --- test/models/contact/contact_transfer_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/models/contact/contact_transfer_test.rb b/test/models/contact/contact_transfer_test.rb index 7346a6375..d9379ecb8 100644 --- a/test/models/contact/contact_transfer_test.rb +++ b/test/models/contact/contact_transfer_test.rb @@ -55,12 +55,12 @@ class ContactTransferTest < ActiveSupport::TestCase end end - def test_changes_registrar + def test_assigns_new_registrar new_contact = @contact.transfer(@new_registrar) assert_equal @new_registrar, new_contact.registrar end - def test_links_to_original + def test_links_to_original_contact new_contact = @contact.transfer(@new_registrar) assert_equal @contact, new_contact.original end From 84bc0f891490f50103448d428c5e4b1e45152b21 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sat, 3 Mar 2018 15:44:03 +0200 Subject: [PATCH 03/11] Rename test class #746 --- .../models/contact/{contact_transfer_test.rb => transfer_test.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/models/contact/{contact_transfer_test.rb => transfer_test.rb} (100%) diff --git a/test/models/contact/contact_transfer_test.rb b/test/models/contact/transfer_test.rb similarity index 100% rename from test/models/contact/contact_transfer_test.rb rename to test/models/contact/transfer_test.rb From 53a34ee2d643b58f03bda1937274bdcd7c2b9c54 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 5 Mar 2018 10:59:14 +0200 Subject: [PATCH 04/11] Reuse identical contacts #746 --- app/models/concerns/contact/identical.rb | 18 +++++++++++ app/models/concerns/contact/transferable.rb | 2 ++ app/models/concerns/domain/transferable.rb | 2 +- app/models/contact.rb | 1 + test/fixtures/contacts.yml | 15 ++++++++++ test/integration/api/domain_transfers_test.rb | 18 ++++++++--- .../epp/domain/transfer/request_test.rb | 9 +++--- test/models/contact/identical_test.rb | 30 +++++++++++++++++++ test/models/contact/transfer_test.rb | 11 +++++-- 9 files changed, 94 insertions(+), 12 deletions(-) create mode 100644 app/models/concerns/contact/identical.rb create mode 100644 test/models/contact/identical_test.rb diff --git a/app/models/concerns/contact/identical.rb b/app/models/concerns/contact/identical.rb new file mode 100644 index 000000000..1c490b75d --- /dev/null +++ b/app/models/concerns/contact/identical.rb @@ -0,0 +1,18 @@ +module Concerns::Contact::Identical + extend ActiveSupport::Concern + + ATTRIBUTE_FILTER = %w[ + name + ident + ident_type + ident_country_code + phone + email + ] + private_constant :ATTRIBUTE_FILTER + + def identical(registrar) + self.class.where(attributes.slice(*ATTRIBUTE_FILTER)).where(registrar: registrar) + .where.not(id: id).take + end +end diff --git a/app/models/concerns/contact/transferable.rb b/app/models/concerns/contact/transferable.rb index 8da98fc57..14c1cac3c 100644 --- a/app/models/concerns/contact/transferable.rb +++ b/app/models/concerns/contact/transferable.rb @@ -7,6 +7,8 @@ module Concerns::Contact::Transferable end def transfer(new_registrar) + return identical(new_registrar) if identical(new_registrar) + new_contact = self.dup new_contact.registrar = new_registrar new_contact.original = self diff --git a/app/models/concerns/domain/transferable.rb b/app/models/concerns/domain/transferable.rb index f2e7736c2..56e77f34d 100644 --- a/app/models/concerns/domain/transferable.rb +++ b/app/models/concerns/domain/transferable.rb @@ -52,7 +52,7 @@ module Concerns::Domain::Transferable def transfer_registrant(new_registrar) return if registrant.registrar == new_registrar - self.registrant = registrant.transfer(new_registrar) + self.registrant = registrant.transfer(new_registrar).becomes(Registrant) end def transfer_domain_contacts(new_registrar) diff --git a/app/models/contact.rb b/app/models/contact.rb index 10b71b55b..d01d42634 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -3,6 +3,7 @@ class Contact < ActiveRecord::Base include EppErrors include UserEvents include Concerns::Contact::Transferable + include Concerns::Contact::Identical belongs_to :original, class_name: self.name belongs_to :registrar, required: true diff --git a/test/fixtures/contacts.yml b/test/fixtures/contacts.yml index eaf4401b7..4c82f6f56 100644 --- a/test/fixtures/contacts.yml +++ b/test/fixtures/contacts.yml @@ -19,6 +19,8 @@ william: registrar: bestnames code: william-001 auth_info: 6573d0 + statuses: + - ok jane: name: Jane @@ -42,6 +44,19 @@ acme_ltd: code: acme-ltd-001 auth_info: 720b3c +identical_to_william: + name: William + email: william@inbox.test + phone: '+555.555' + ident: 1234 + ident_type: priv + ident_country_code: US + registrar: goodnames + code: william-002 + auth_info: 5ab865 + statuses: + - ok + invalid: name: any code: any diff --git a/test/integration/api/domain_transfers_test.rb b/test/integration/api/domain_transfers_test.rb index b90b59be4..2e344e45e 100644 --- a/test/integration/api/domain_transfers_test.rb +++ b/test/integration/api/domain_transfers_test.rb @@ -3,6 +3,7 @@ require 'test_helper' class APIDomainTransfersTest < ActionDispatch::IntegrationTest def setup @domain = domains(:shop) + @new_registrar = registrars(:goodnames) Setting.transfer_wait_time = 0 # Auto-approval end @@ -29,10 +30,10 @@ class APIDomainTransfersTest < ActionDispatch::IntegrationTest assert @domain.transfers.last.approved? end - def test_changes_registrar + def test_assigns_new_registrar post '/repp/v1/domain_transfers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } @domain.reload - assert_equal registrars(:goodnames), @domain.registrar + assert_equal @new_registrar, @domain.registrar end def test_regenerates_transfer_code @@ -52,11 +53,20 @@ class APIDomainTransfersTest < ActionDispatch::IntegrationTest end def test_duplicates_registrant_admin_and_tech_contacts - assert_difference 'Contact.count', 3 do + assert_difference -> { @new_registrar.contacts.size }, 2 do post '/repp/v1/domain_transfers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } end end + def test_reuses_identical_contact + request_params = { format: :json, + data: { domainTransfers: [{ domainName: 'shop.test', transferCode: '65078d5' }, + { domainName: 'airport.test', transferCode: '55438j5' }, + { domainName: 'library.test', transferCode: '45118f5' }] } } + post '/repp/v1/domain_transfers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + assert_equal 1, @new_registrar.contacts.where(name: 'William').size + end + def test_fails_if_domain_does_not_exist request_params = { format: :json, data: { domainTransfers: [{ domainName: 'non-existent.test', transferCode: 'any' }] } } @@ -71,7 +81,7 @@ class APIDomainTransfersTest < ActionDispatch::IntegrationTest data: { domainTransfers: [{ domainName: 'shop.test', transferCode: 'wrong' }] } } post '/repp/v1/domain_transfers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } assert_response 400 - refute_equal registrars(:goodnames), @domain.registrar + refute_equal @new_registrar, @domain.registrar assert_equal ({ errors: [{ title: 'shop.test transfer code is wrong' }] }), JSON.parse(response.body, symbolize_names: true) end diff --git a/test/integration/epp/domain/transfer/request_test.rb b/test/integration/epp/domain/transfer/request_test.rb index 03c5e7daf..b516cbe76 100644 --- a/test/integration/epp/domain/transfer/request_test.rb +++ b/test/integration/epp/domain/transfer/request_test.rb @@ -3,6 +3,7 @@ require 'test_helper' class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest def setup @domain = domains(:shop) + @new_registrar = registrars(:goodnames) Setting.transfer_wait_time = 0 end @@ -24,10 +25,10 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest 'https://epp.tld.ee/schema/domain-eis-1.0.xsd').text end - def test_changes_registrar + def test_assigns_new_registrar post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' } @domain.reload - assert_equal registrars(:goodnames), @domain.registrar + assert_equal @new_registrar, @domain.registrar end def test_regenerates_transfer_code @@ -48,7 +49,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest end def test_duplicates_registrant_admin_and_tech_contacts - assert_difference 'Contact.count', 3 do + assert_difference -> { @new_registrar.contacts.size }, 2 do post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' } end end @@ -106,7 +107,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' } @domain.reload - refute_equal registrars(:goodnames), @domain.registrar + refute_equal @new_registrar, @domain.registrar assert_equal '2201', Nokogiri::XML(response.body).at_css('result')[:code] end diff --git a/test/models/contact/identical_test.rb b/test/models/contact/identical_test.rb new file mode 100644 index 000000000..10fbc3a66 --- /dev/null +++ b/test/models/contact/identical_test.rb @@ -0,0 +1,30 @@ +require 'test_helper' + +class ContactIdenticalTest < ActiveSupport::TestCase + def setup + @contact = contacts(:william) + @identical = contacts(:identical_to_william) + end + + def test_identical + assert_equal @identical, @contact.identical(@identical.registrar) + end + + def test_not_identical + filter_attributes = %i[ + name + ident + ident_type + ident_country_code + phone + email + ] + + filter_attributes.each do |attribute| + previous_value = @identical.public_send(attribute) + @identical.update_attribute(attribute, 'other') + assert_nil @contact.identical(@identical.registrar) + @identical.update_attribute(attribute, previous_value) + end + end +end diff --git a/test/models/contact/transfer_test.rb b/test/models/contact/transfer_test.rb index d9379ecb8..a695c112a 100644 --- a/test/models/contact/transfer_test.rb +++ b/test/models/contact/transfer_test.rb @@ -35,18 +35,23 @@ class ContactTransferTest < ActiveSupport::TestCase end def test_keeps_original_contact_untouched - original_hash = @contact.to_json + original_hash = @contact.attributes @contact.transfer(@new_registrar) @contact.reload - assert_equal original_hash, @contact.to_json + assert_equal original_hash, @contact.attributes end def test_creates_new_contact - assert_difference 'Contact.count' do + assert_difference -> { @new_registrar.contacts.count } do @contact.transfer(@new_registrar) end end + def test_reuses_identical_contact + identical = contacts(:identical_to_william) + assert_equal identical, contacts(:william).transfer(@new_registrar) + end + def test_bypasses_validation @contact = contacts(:invalid) From ecc68f083d43d7e296897cd3e49ecb552d92ab6c Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 6 Mar 2018 10:39:32 +0200 Subject: [PATCH 05/11] Enhance identical contact lookup #746 --- app/models/concerns/contact/identical.rb | 26 ++++++++++--- test/fixtures/contacts.yml | 17 ++++---- test/models/contact/identical_test.rb | 49 ++++++++++++++++++------ 3 files changed, 67 insertions(+), 25 deletions(-) diff --git a/app/models/concerns/contact/identical.rb b/app/models/concerns/contact/identical.rb index 1c490b75d..f529e09ac 100644 --- a/app/models/concerns/contact/identical.rb +++ b/app/models/concerns/contact/identical.rb @@ -1,18 +1,34 @@ module Concerns::Contact::Identical extend ActiveSupport::Concern - ATTRIBUTE_FILTER = %w[ + IDENTIFIABLE_ATTRIBUTES = %w[ name + email + phone + fax ident ident_type ident_country_code - phone - email + org_name ] - private_constant :ATTRIBUTE_FILTER + private_constant :IDENTIFIABLE_ATTRIBUTES def identical(registrar) - self.class.where(attributes.slice(*ATTRIBUTE_FILTER)).where(registrar: registrar) + self.class.where(identifiable_hash) + .where(["statuses = ?::character varying[]", "{#{read_attribute(:statuses).join(',')}}"]) + .where(registrar: registrar) .where.not(id: id).take end + + private + + def identifiable_hash + attributes = IDENTIFIABLE_ATTRIBUTES + + if self.class.address_processing? + attributes += self.class.address_attribute_names + end + + slice(*attributes) + end end diff --git a/test/fixtures/contacts.yml b/test/fixtures/contacts.yml index 5f6dc3e81..691570d21 100644 --- a/test/fixtures/contacts.yml +++ b/test/fixtures/contacts.yml @@ -9,16 +9,22 @@ john: code: john-001 auth_info: cacb5b -william: +william: &william name: William email: william@inbox.test phone: '+555.555' + fax: +555.555 ident: 1234 ident_type: priv ident_country_code: US registrar: bestnames code: william-001 auth_info: 6573d0 + street: Main Street + zip: 12345 + city: New York + state: New York + country_code: US statuses: - ok @@ -56,17 +62,10 @@ jack: auth_info: e2c440 identical_to_william: - name: William - email: william@inbox.test - phone: '+555.555' - ident: 1234 - ident_type: priv - ident_country_code: US + <<: *william registrar: goodnames code: william-002 auth_info: 5ab865 - statuses: - - ok invalid: name: any diff --git a/test/models/contact/identical_test.rb b/test/models/contact/identical_test.rb index 10fbc3a66..05802b7c2 100644 --- a/test/models/contact/identical_test.rb +++ b/test/models/contact/identical_test.rb @@ -1,30 +1,57 @@ require 'test_helper' class ContactIdenticalTest < ActiveSupport::TestCase + REGULAR_FILTER_ATTRIBUTES = %i[ + name + email + phone + fax + ident + ident_type + ident_country_code + org_name + ] + def setup @contact = contacts(:william) @identical = contacts(:identical_to_william) end - def test_identical + def test_returns_identical assert_equal @identical, @contact.identical(@identical.registrar) end - def test_not_identical - filter_attributes = %i[ - name - ident - ident_type - ident_country_code - phone - email - ] + def test_does_not_return_non_identical + REGULAR_FILTER_ATTRIBUTES.each do |attribute| + previous_value = @identical.public_send(attribute) + @identical.update_attribute(attribute, 'other') + assert_nil @contact.identical(@identical.registrar) + @identical.update_attribute(attribute, previous_value) + end - filter_attributes.each do |attribute| + @identical.update!({ statuses: %w[ok linked] }) + assert_nil @contact.identical(@identical.registrar) + end + + def test_takes_address_into_account_when_processing_enabled + Setting.address_processing = true + + Contact.address_attribute_names.each do |attribute| previous_value = @identical.public_send(attribute) @identical.update_attribute(attribute, 'other') assert_nil @contact.identical(@identical.registrar) @identical.update_attribute(attribute, previous_value) end end + + def test_ignores_address_when_processing_disabled + Setting.address_processing = false + + Contact.address_attribute_names.each do |attribute| + previous_value = @identical.public_send(attribute) + @identical.update_attribute(attribute, 'other') + assert_equal @identical, @contact.identical(@identical.registrar) + @identical.update_attribute(attribute, previous_value) + end + end end From d42bdfbc0fbc12d8d81b421c8a6f0b5a8b37e772 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 6 Mar 2018 11:05:08 +0200 Subject: [PATCH 06/11] Improve tests #746 --- test/integration/api/domain_transfers_test.rb | 4 ---- test/integration/epp/domain/transfer/request_test.rb | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/test/integration/api/domain_transfers_test.rb b/test/integration/api/domain_transfers_test.rb index 2e344e45e..08ce2e34c 100644 --- a/test/integration/api/domain_transfers_test.rb +++ b/test/integration/api/domain_transfers_test.rb @@ -59,10 +59,6 @@ class APIDomainTransfersTest < ActionDispatch::IntegrationTest end def test_reuses_identical_contact - request_params = { format: :json, - data: { domainTransfers: [{ domainName: 'shop.test', transferCode: '65078d5' }, - { domainName: 'airport.test', transferCode: '55438j5' }, - { domainName: 'library.test', transferCode: '45118f5' }] } } post '/repp/v1/domain_transfers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } assert_equal 1, @new_registrar.contacts.where(name: 'William').size end diff --git a/test/integration/epp/domain/transfer/request_test.rb b/test/integration/epp/domain/transfer/request_test.rb index b516cbe76..508ec4334 100644 --- a/test/integration/epp/domain/transfer/request_test.rb +++ b/test/integration/epp/domain/transfer/request_test.rb @@ -54,6 +54,11 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest end end + def test_reuses_identical_contact + post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' } + assert_equal 1, @new_registrar.contacts.where(name: 'William').size + end + def test_saves_legal_document assert_difference -> { @domain.legal_documents(true).size } do post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' } From a910165bc98bc62843fef32b2ecc9a6429ff5c4c Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 6 Mar 2018 12:13:21 +0200 Subject: [PATCH 07/11] Fix sporadic errors in tests #746 --- test/models/contact/identical_test.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/models/contact/identical_test.rb b/test/models/contact/identical_test.rb index 05802b7c2..61d0946bc 100644 --- a/test/models/contact/identical_test.rb +++ b/test/models/contact/identical_test.rb @@ -34,12 +34,14 @@ class ContactIdenticalTest < ActiveSupport::TestCase end def test_takes_address_into_account_when_processing_enabled - Setting.address_processing = true - Contact.address_attribute_names.each do |attribute| previous_value = @identical.public_send(attribute) @identical.update_attribute(attribute, 'other') - assert_nil @contact.identical(@identical.registrar) + + Contact.stub :address_processing?, true do + assert_nil @contact.identical(@identical.registrar) + end + @identical.update_attribute(attribute, previous_value) end end @@ -50,7 +52,11 @@ class ContactIdenticalTest < ActiveSupport::TestCase Contact.address_attribute_names.each do |attribute| previous_value = @identical.public_send(attribute) @identical.update_attribute(attribute, 'other') - assert_equal @identical, @contact.identical(@identical.registrar) + + Contact.stub :address_processing?, false do + assert_equal @identical, @contact.identical(@identical.registrar) + end + @identical.update_attribute(attribute, previous_value) end end From bfbee65a4987cf22b230d140c83a5fa5b49e4095 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 6 Mar 2018 12:26:35 +0200 Subject: [PATCH 08/11] Remove duplicate method #746 --- .reek | 1 - app/models/contact.rb | 18 +++--------------- spec/models/contact_spec.rb | 4 ---- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/.reek b/.reek index 6cdd58c75..0d5c66aa3 100644 --- a/.reek +++ b/.reek @@ -1066,7 +1066,6 @@ Attribute: - BankStatement#th6_file - Versions#version_loader - Contact#deliver_emails - - Contact#domains_present - Contact#legal_document_id - Counter#value - Deposit#amount diff --git a/app/models/contact.rb b/app/models/contact.rb index d01d42634..417c1f07b 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -70,10 +70,6 @@ class Contact < ActiveRecord::Base after_save :update_related_whois_records - - # for overwrite when doing children loop - attr_writer :domains_present - scope :current_registrars, ->(id) { where(registrar_id: id) } ORG = 'org' @@ -207,7 +203,7 @@ class Contact < ActiveRecord::Base ver_scope << "(children->'#{type}')::jsonb <@ json_build_array(#{contact.id})::jsonb" end next if DomainVersion.where("created_at > ?", Time.now - Setting.orphans_contacts_in_months.to_i.months).where(ver_scope.join(" OR ")).any? - next if contact.domains_present? + next if contact.used? contact.destroy counter.next @@ -280,7 +276,7 @@ class Contact < ActiveRecord::Base calculated.delete(Contact::OK) calculated.delete(Contact::LINKED) calculated << Contact::OK if calculated.empty?# && valid? - calculated << Contact::LINKED if domains_present? + calculated << Contact::LINKED if used? calculated.uniq end @@ -348,7 +344,7 @@ class Contact < ActiveRecord::Base # no need separate method # should use only in transaction def destroy_and_clean frame - if domains_present? + if used? errors.add(:domains, :exist) return false end @@ -406,14 +402,6 @@ class Contact < ActiveRecord::Base end end - # optimization under children loop, - # otherwise bullet will not be happy - def domains_present? - return @domains_present if @domains_present - domain_contacts.present? || registrant_domains.present? - end - - def search_name "#{code} #{name}" end diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 1703bf90a..7c9a7c833 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -76,10 +76,6 @@ RSpec.describe Contact do end end - it 'should not have relation with domains' do - @contact.domains_present?.should == false - end - it 'should not overwrite code' do old_code = @contact.code @contact.code = 'CID:REG1:should-not-overwrite-old-code-12345' From 5319243e09234a1503d086b56f8bc93df3c2f2c4 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 6 Mar 2018 13:08:18 +0200 Subject: [PATCH 09/11] Convert spec to test #746 --- spec/models/contact_spec.rb | 30 ----------------------------- test/fixtures/contacts.yml | 7 +++++++ test/models/contact/contact_test.rb | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 7c9a7c833..875042a27 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -413,36 +413,6 @@ RSpec.describe Contact do end end - describe '#used?' do - context 'when used as registrant' do - let(:registrant) { create(:registrant) } - - before :example do - create(:domain, registrant: registrant) - registrant.reload - end - - specify { expect(registrant).to be_used } - end - - context 'when used as contact' do - let(:contact) { create(:contact) } - - before :example do - domain = create(:domain) - domain.admin_domain_contacts << create(:admin_domain_contact, contact: contact) - contact.reload - end - - specify { expect(contact).to be_used } - end - - context 'when not used' do - let(:contact) { create(:contact) } - specify { expect(contact).to_not be_used } - end - end - describe '#domain_names_with_roles' do let(:contact) { create(:registrant) } subject(:domain_names) { contact.domain_names_with_roles } diff --git a/test/fixtures/contacts.yml b/test/fixtures/contacts.yml index 691570d21..50befdcf8 100644 --- a/test/fixtures/contacts.yml +++ b/test/fixtures/contacts.yml @@ -67,6 +67,13 @@ identical_to_william: code: william-002 auth_info: 5ab865 +not_in_use: + name: Useless + email: useless@inbox.test + registrar: bestnames + code: useless-001 + auth_info: e75a2a + invalid: name: any code: any diff --git a/test/models/contact/contact_test.rb b/test/models/contact/contact_test.rb index ef958e2a4..bae7a213b 100644 --- a/test/models/contact/contact_test.rb +++ b/test/models/contact/contact_test.rb @@ -12,4 +12,18 @@ class ContactTest < ActiveSupport::TestCase def test_invalid_fixture_is_invalid assert contacts(:invalid).invalid? end + + def test_in_use_if_acts_as_a_registrant + DomainContact.delete_all + assert @contact.used? + end + + def test_in_use_if_acts_as_a_domain_contact + Domain.update_all(registrant_id: contacts(:william)) + assert @contact.used? + end + + def test_not_in_use_if_acts_as_neither_registrant_nor_domain_contact + refute contacts(:not_in_use).used? + end end From 046b9e96ce608df547185e022b7a26e85b29e57b Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 6 Mar 2018 13:11:22 +0200 Subject: [PATCH 10/11] Improve readability #746 --- app/models/contact.rb | 8 ++++---- app/presenters/registrant_presenter.rb | 2 +- app/views/mailers/contact_mailer/email_updated.html.erb | 4 ++-- app/views/mailers/contact_mailer/email_updated.text.erb | 4 ++-- spec/presenters/registrant_presenter_spec.rb | 2 +- test/models/contact/contact_test.rb | 6 +++--- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/models/contact.rb b/app/models/contact.rb index 417c1f07b..8a95fa710 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -203,7 +203,7 @@ class Contact < ActiveRecord::Base ver_scope << "(children->'#{type}')::jsonb <@ json_build_array(#{contact.id})::jsonb" end next if DomainVersion.where("created_at > ?", Time.now - Setting.orphans_contacts_in_months.to_i.months).where(ver_scope.join(" OR ")).any? - next if contact.used? + next if contact.in_use? contact.destroy counter.next @@ -276,7 +276,7 @@ class Contact < ActiveRecord::Base calculated.delete(Contact::OK) calculated.delete(Contact::LINKED) calculated << Contact::OK if calculated.empty?# && valid? - calculated << Contact::LINKED if used? + calculated << Contact::LINKED if in_use? calculated.uniq end @@ -344,7 +344,7 @@ class Contact < ActiveRecord::Base # no need separate method # should use only in transaction def destroy_and_clean frame - if used? + if in_use? errors.add(:domains, :exist) return false end @@ -540,7 +540,7 @@ class Contact < ActiveRecord::Base Country.new(ident_country_code) end - def used? + def in_use? registrant_domains.any? || domain_contacts.any? end diff --git a/app/presenters/registrant_presenter.rb b/app/presenters/registrant_presenter.rb index 148c5d219..6a78f86c5 100644 --- a/app/presenters/registrant_presenter.rb +++ b/app/presenters/registrant_presenter.rb @@ -8,7 +8,7 @@ class RegistrantPresenter :reg_no, :street, :city, :state, :zip, :country, :ident_country, - :used?, + :in_use?, to: :registrant def initialize(registrant:, view:) diff --git a/app/views/mailers/contact_mailer/email_updated.html.erb b/app/views/mailers/contact_mailer/email_updated.html.erb index a4f6d8583..c3a22b063 100644 --- a/app/views/mailers/contact_mailer/email_updated.html.erb +++ b/app/views/mailers/contact_mailer/email_updated.html.erb @@ -10,7 +10,7 @@ uus aadress: <%= contact.email %>

E-posti aadressile saadetakse domeeni toimingutega seotud infot, sealhulgas kinnitustaotlused omanikuvahetuse ja domeeni kustutamise korral.

-<% if contact.used? %> +<% if contact.in_use? %> Muudatusega seotud domeenid:
<%= contact.domain_names_with_roles(locale: :et, line_break: '
') %> <% end %> @@ -34,7 +34,7 @@ new address: <%= contact.email %>

E-mail addresses are used to send important information regarding your registered domains including applications for approval of registrant change and domain deletion. Please make sure that the update and contact information are correct.

-<% if contact.used? %> +<% if contact.in_use? %> Domains affected by this update:
<%= contact.domain_names_with_roles(line_break: '
') %> <% end %> diff --git a/app/views/mailers/contact_mailer/email_updated.text.erb b/app/views/mailers/contact_mailer/email_updated.text.erb index 97e46a5eb..d847f9f57 100644 --- a/app/views/mailers/contact_mailer/email_updated.text.erb +++ b/app/views/mailers/contact_mailer/email_updated.text.erb @@ -10,7 +10,7 @@ uus aadress: <%= contact.email %> E-posti aadressile saadetakse domeeni toimingutega seotud infot, sealhulgas kinnitustaotlused omanikuvahetuse ja domeeni kustutamise korral. -<% if contact.used? %> +<% if contact.in_use? %> Muudatusega seotud domeenid: <%= contact.domain_names_with_roles(locale: :et) %> <% end %> @@ -34,7 +34,7 @@ new address: <%= contact.email %> E-mail addresses are used to send important information regarding your registered domains including applications for approval of registrant change and domain deletion. Please make sure that the update and contact information are correct. -<% if contact.used? %> +<% if contact.in_use? %> Domains affected by this update: <%= contact.domain_names_with_roles %> <% end %> diff --git a/spec/presenters/registrant_presenter_spec.rb b/spec/presenters/registrant_presenter_spec.rb index f9d112f1a..8c7c7b6d3 100644 --- a/spec/presenters/registrant_presenter_spec.rb +++ b/spec/presenters/registrant_presenter_spec.rb @@ -67,7 +67,7 @@ RSpec.describe RegistrantPresenter do zip id_code reg_no - used? + in_use? ) registrant_delegatable_attributes.each do |attr_name| diff --git a/test/models/contact/contact_test.rb b/test/models/contact/contact_test.rb index bae7a213b..286ded534 100644 --- a/test/models/contact/contact_test.rb +++ b/test/models/contact/contact_test.rb @@ -15,15 +15,15 @@ class ContactTest < ActiveSupport::TestCase def test_in_use_if_acts_as_a_registrant DomainContact.delete_all - assert @contact.used? + assert @contact.in_use? end def test_in_use_if_acts_as_a_domain_contact Domain.update_all(registrant_id: contacts(:william)) - assert @contact.used? + assert @contact.in_use? end def test_not_in_use_if_acts_as_neither_registrant_nor_domain_contact - refute contacts(:not_in_use).used? + refute contacts(:not_in_use).in_use? end end From afd135ff15ce1850da038f8367fe8e86b030818b Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 6 Mar 2018 18:05:30 +0200 Subject: [PATCH 11/11] Remove unused method #746 --- app/models/contact.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/contact.rb b/app/models/contact.rb index 8a95fa710..d54e7296d 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -70,7 +70,6 @@ class Contact < ActiveRecord::Base after_save :update_related_whois_records - scope :current_registrars, ->(id) { where(registrar_id: id) } ORG = 'org' PRIV = 'priv'