Merge pull request #2375 from internetee/2374-deprecated-validation-e…

…vents Prevent deprecated validation events
internetee · May 17, 2022 · a580c22 · a580c22
2 parents 65ae476 + c02101a
commit a580c22
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 30 deletions.
diff --git a/app/interactions/actions/contact_update.rb b/app/interactions/actions/contact_update.rb
@@ -37,12 +37,11 @@ def maybe_change_email
     end
 
     def maybe_filtering_old_failed_records
-      if contact.validation_events.count > 1
-        contact.validation_events.order!(created_at: :asc)
-        while contact.validation_events.count >= 1
-          contact.validation_events.first.destroy
-        end
-      end
+      validation_events = contact.validation_events
+      return unless validation_events.count > 1
+
+      validation_events.order!(created_at: :asc)
+      validation_events.first.destroy while validation_events.count >= 1
     end
 
     def maybe_remove_address
@@ -116,8 +115,9 @@ def commit
       contact.email_history = old_email
       updated = contact.save
 
-      if updated && email_changed && contact.registrant?
-        ContactMailer.email_changed(contact: contact, old_email: old_email).deliver_now
+      if updated && email_changed
+        contact.validation_events.where('event_data @> ?', { 'email': old_email }.to_json).destroy_all
+        ContactMailer.email_changed(contact: contact, old_email: old_email).deliver_now if contact.registrant?
       end
 
       updated

diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb
@@ -26,33 +26,27 @@ def calculate_check_level
       Rails.env.test? && check_level == 'smtp' ? :mx : check_level.to_sym
     end
 
+    def destroy_old_validations(validation_events, minimum_size, check_level)
+      return unless validation_events.count > minimum_size && @check_level == check_level
+
+      validation_events.order!(created_at: :asc)
+      validation_events.first.destroy while validation_events.count > minimum_size
+    end
+
     def filtering_old_failed_records(result)
-      if @check_level == "mx" && !result.success && validation_eventable.validation_events.count > 3
-        validation_eventable.validation_events.order!(created_at: :asc)
-        while validation_eventable.validation_events.count > 3
-          validation_eventable.validation_events.first.destroy
-        end
-      end
+      events = validation_eventable.validation_events
 
-      if @check_level == "mx" && result.success && validation_eventable.validation_events.count > 1
-        validation_eventable.validation_events.order!(created_at: :asc)
-        while validation_eventable.validation_events.count > 1
-          validation_eventable.validation_events.first.destroy
-        end
-      end
+      destroy_old_validations(events, ValidationEvent::MX_CHECK, 'mx') unless result.success
 
-      if @check_level == "smtp" && validation_eventable.validation_events.count > 1
-        validation_eventable.validation_events.order!(created_at: :asc)
-        while validation_eventable.validation_events.count > 1
-          validation_eventable.validation_events.first.destroy
-        end
-      end
+      destroy_old_validations(events, ValidationEvent::REDEEM_EVENTS_COUNT_BY_LEVEL[:mx], 'mx') if result.success
+
+      destroy_old_validations(events, ValidationEvent::REDEEM_EVENTS_COUNT_BY_LEVEL[:smtp], 'smtp')
     end
 
     def save_result(result)
       contacts = Contact.where(email: email)
 
-      if !result.success && @check_level == "mx"
+      if !result.success && @check_level == 'mx'
         result_validation = Actions::AAndAaaaEmailValidation.call(email: @email, value: 'A')
         output_a_and_aaaa_validation_results(email: @email,
                                              result: result_validation,
@@ -96,8 +90,7 @@ def check_for_records_value(domain:, value:)
         when 'AAAA'
           ress = dns.getresources domain, Resolv::DNS::Resource::IN::AAAA
         end
-
-        result = ress.map { |r| r.address }
+        result = ress.map(&:address)
       end
 
       result

diff --git a/app/models/contact.rb b/app/models/contact.rb
@@ -15,7 +15,7 @@ class Contact < ApplicationRecord
   has_many :domain_contacts
   has_many :domains, through: :domain_contacts
   has_many :legal_documents, as: :documentable
-  has_many :validation_events, as: :validation_eventable
+  has_many :validation_events, as: :validation_eventable, dependent: :destroy
   has_many :registrant_domains, class_name: 'Domain', foreign_key: 'registrant_id'
   has_many :actions, dependent: :destroy
 

diff --git a/test/integration/epp/contact/update/base_test.rb b/test/integration/epp/contact/update/base_test.rb
@@ -80,6 +80,35 @@ def test_notifies_contact_by_email_when_email_is_changed
     assert_emails 1
   end
 
+  def test_destroy_old_validation_when_email_is_changed
+    @contact.verify_email
+    old_validation_event = @contact.validation_events.first
+    @contact.update_columns(code: @contact.code.upcase)
+
+    request_xml = <<-XML
+      <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+      <epp xmlns="#{Xsd::Schema.filename(for_prefix: 'epp-ee', for_version: '1.0')}">
+        <command>
+          <update>
+            <contact:update xmlns:contact="#{Xsd::Schema.filename(for_prefix: 'contact-ee', for_version: '1.1')}">
+              <contact:id>john-001</contact:id>
+              <contact:chg>
+                <contact:email>[email protected]</contact:email>
+              </contact:chg>
+            </contact:update>
+          </update>
+        </command>
+      </epp>
+    XML
+
+    post epp_update_path, params: { frame: request_xml },
+         headers: { 'HTTP_COOKIE' => 'session=api_bestnames' }
+
+    assert_raises(ActiveRecord::RecordNotFound) do
+      ValidationEvent.find(old_validation_event.id)
+    end
+  end
+
   def test_skips_notifying_contact_when_email_is_not_changed
     assert_equal 'john-001', @contact.code
     assert_equal '[email protected]', @contact.email