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.

Sign up for GitHub

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

Prevent deprecated validation events #2375

Merged
merged 5 commits into from
May 17, 2022
Merged

Prevent deprecated validation events #2375

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2803c73
Contact destroy dependent validation event
thiagoyoussef May 11, 2022
7a2256a
DRY code
thiagoyoussef May 11, 2022
ec71ae1
Prevent deprecated validations when email is changed
thiagoyoussef May 12, 2022
060416d
Use validation event constants
thiagoyoussef May 16, 2022
c02101a
Fix code climate
thiagoyoussef May 16, 2022
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
16 changes: 8 additions & 8 deletions app/interactions/actions/contact_update.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
35 changes: 14 additions & 21 deletions app/interactions/actions/email_check.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
return unless validation_events.count > minimum_size

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, 3) if @check_level == 'mx' && !result.success
Copy link
Contributor

Choose a reason for hiding this comment

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

What happen if we decide to change validation events limit? For example from 3 to 4. For this it will need to search code in whole project what responsibility for it. Also, if I didn't know nothing about validation, number 3 will be for me incomprehensible attribute. Please check validation_event model, there is should be constant where is assigned default validation limit and please change number 3 to this constant

(and yes, I know that I am too used numbers instead constants and this is also my fall :) )


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, 1) if @check_level == 'mx' && result.success

destroy_old_validations(events, 1) if @check_level == '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,
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/models/contact.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
29 changes: 29 additions & 0 deletions test/integration/epp/contact/update/base_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>john-new@inbox.test</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 'john@inbox.test', @contact.email
Expand Down