From 70252f9f87aabb11d4677b0065d3136ad72d43cc Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 12:58:11 +0200 Subject: [PATCH 01/44] Story #109367694 - changed xml text for elements crID, clID and upID --- app/views/epp/contacts/info.xml.builder | 12 +++++------- app/views/epp/domains/info.xml.builder | 13 ++++++------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 18019208a..3790544e1 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -46,15 +46,13 @@ xml.epp_head do xml.tag!('contact:email', 'No access') end - xml.tag!('contact:clID', @contact.registrar.try(:name)) - if @contact.creator.try(:registrar).blank? && Rails.env.test? - xml.tag!('contact:crID', 'TEST-CREATOR') - else - xml.tag!('contact:crID', @contact.creator.try(:registrar)) - end + xml.tag!('contact:clID', @contact.registrar.try(:code)) + + xml.tag!('contact:crID', @contact.creator.registrar.try(:code)) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) + if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.try(:registrar)) + xml.tag!('contact:upID', @contact.updator.registrar.code) xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index 9e1779921..fd6bfeeb6 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -36,19 +36,18 @@ xml.epp_head do ## TODO Find out what this domain:host is all about - xml.tag!('domain:clID', @domain.registrar_name) - - xml.tag!('domain:crID', @domain.creator.try(:registrar)) if @domain.creator + xml.tag!('domain:clID', @domain.registrar.code) + xml.tag!('domain:crID', @domain.creator.registrar.code) if @domain.creator xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) - xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) if @domain.updated_at != @domain.created_at + if @domain.updated_at != @domain.created_at + xml.tag!('domain:upID', @domain.updator.registrar.code) + xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) + end xml.tag!('domain:exDate', @domain.valid_to.try(:iso8601)) - # TODO Make domain stampable - #xml.tag!('domain:upID', @domain.updated_by) - # TODO Make domain transferrable #xml.tag!('domain:trDate', @domain.transferred_at) if @domain.transferred_at From 2921ccdaebced261a22e43eb129c9fb3c3016066 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 12:58:11 +0200 Subject: [PATCH 02/44] Story #109367694 - changed xml text for elements crID, clID and upID --- app/views/epp/contacts/info.xml.builder | 12 +++++------- app/views/epp/domains/info.xml.builder | 13 ++++++------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 18019208a..3790544e1 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -46,15 +46,13 @@ xml.epp_head do xml.tag!('contact:email', 'No access') end - xml.tag!('contact:clID', @contact.registrar.try(:name)) - if @contact.creator.try(:registrar).blank? && Rails.env.test? - xml.tag!('contact:crID', 'TEST-CREATOR') - else - xml.tag!('contact:crID', @contact.creator.try(:registrar)) - end + xml.tag!('contact:clID', @contact.registrar.try(:code)) + + xml.tag!('contact:crID', @contact.creator.registrar.try(:code)) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) + if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.try(:registrar)) + xml.tag!('contact:upID', @contact.updator.registrar.code) xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index 9e1779921..fd6bfeeb6 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -36,19 +36,18 @@ xml.epp_head do ## TODO Find out what this domain:host is all about - xml.tag!('domain:clID', @domain.registrar_name) - - xml.tag!('domain:crID', @domain.creator.try(:registrar)) if @domain.creator + xml.tag!('domain:clID', @domain.registrar.code) + xml.tag!('domain:crID', @domain.creator.registrar.code) if @domain.creator xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) - xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) if @domain.updated_at != @domain.created_at + if @domain.updated_at != @domain.created_at + xml.tag!('domain:upID', @domain.updator.registrar.code) + xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) + end xml.tag!('domain:exDate', @domain.valid_to.try(:iso8601)) - # TODO Make domain stampable - #xml.tag!('domain:upID', @domain.updated_by) - # TODO Make domain transferrable #xml.tag!('domain:trDate', @domain.transferred_at) if @domain.transferred_at From e15ab3421d4b814eec6e4ece051e8d22cacff3b8 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 18:29:02 +0200 Subject: [PATCH 03/44] Story #109367694 - refactor user log string to user for paper trail creator_str --- app/controllers/application_controller.rb | 3 +-- app/models/user.rb | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 318923e3d..de54dafc5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,8 +67,7 @@ class ApplicationController < ActionController::Base end def user_log_str(user) - return 'public' if user.nil? - "#{user.id}-#{user.class}: #{user.username}" + user.nil? ? 'public' : user.string end def comma_support_for(parent_key, key) diff --git a/app/models/user.rb b/app/models/user.rb index 0beb174f3..5d415230c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,4 +3,9 @@ class User < ActiveRecord::Base devise :trackable, :timeoutable attr_accessor :phone + + def string + "#{self.id}-#{self.class}: #{self.username}" + end + end From 13129214e5cc9af30ec4aafbc40cf6fbf0e3bce3 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 18:30:56 +0200 Subject: [PATCH 04/44] Story #109367694 - reset updator_str for pendingUpdate to update requestor requires prior commit e15ab3421d4b814eec6e4ece051e8d22cacff3b8 --- app/models/epp/domain.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index 613c57115..1a577218a 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -498,6 +498,7 @@ class Epp::Domain < Domain old_registrant_email = DomainMailer.registrant_updated_notification_for_old_registrant(id, deliver_emails) preclean_pendings user = ApiUser.find(pending_json['current_user_id']) + ::PaperTrail.whodunnit = user.string # updator str should be the request originator not the approval user frame = Nokogiri::XML(pending_json['frame']) statuses.delete(DomainStatus::PENDING_UPDATE) yield(self) if block_given? # need to skip statuses check here From 251a9e1666d2e0d6023cee6338cc6cc9a0e0d766 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 18:40:13 +0200 Subject: [PATCH 05/44] Story #109367694 - make upID optional to avoid possible error for AdminUser objects have no registar method --- app/views/epp/contacts/info.xml.builder | 2 +- app/views/epp/domains/info.xml.builder | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 3790544e1..965e2340a 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -52,7 +52,7 @@ xml.epp_head do xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.registrar.code) + xml.tag!('contact:upID', @contact.updator.registrar.code) if @contact.updator.try(:registrar).present? xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index fd6bfeeb6..181eabc67 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -42,7 +42,7 @@ xml.epp_head do xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) if @domain.updated_at != @domain.created_at - xml.tag!('domain:upID', @domain.updator.registrar.code) + xml.tag!('domain:upID', @domain.updator.registrar.code) if @domain.updator.try(:registrar).present? xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) end From 098c40efc159bc6ee6462891f20152f4614bc8e4 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 14:11:21 +0200 Subject: [PATCH 06/44] Story #109367694 - use creator or updator as string, when registrar can not be found --- app/views/epp/contacts/info.xml.builder | 10 ++++++++-- app/views/epp/domains/info.xml.builder | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 965e2340a..9410368bb 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -48,11 +48,17 @@ xml.epp_head do xml.tag!('contact:clID', @contact.registrar.try(:code)) - xml.tag!('contact:crID', @contact.creator.registrar.try(:code)) + # EPP requires a creator ID, which should be registrar code if we have one + crID = contact.creator.try(:registrar) + crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? + crID = contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only + xml.tag!('contact:crID', crID) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.registrar.code) if @contact.updator.try(:registrar).present? + upID = contact.updator.try(:registrar) + upID = upID.code if upID.present? # Did updator return a kind of User that has a registrar? + xml.tag!('contact:upID', upID) if upID.present? # optional upID xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index 181eabc67..11ab2441f 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -38,11 +38,17 @@ xml.epp_head do xml.tag!('domain:clID', @domain.registrar.code) - xml.tag!('domain:crID', @domain.creator.registrar.code) if @domain.creator + # EPP requires a creator ID, which should be registrar code if we have one + crID = @domain.creator.try(:registrar) + crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? + crID = @domain.creator unless crID.present? # Fallback if we failed, maybe this is a string only + xml.tag!('domain:crID', crID) xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) if @domain.updated_at != @domain.created_at - xml.tag!('domain:upID', @domain.updator.registrar.code) if @domain.updator.try(:registrar).present? + upID = @domain.updator.try(:registrar) + upID = upID.code if upID.present? # Did updator return a kind of User that has a registrar? + xml.tag!('domain:upID', upID) if upID.present? # optional upID xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) end From 238440de5260efb010c83092e5f62825d7829ef3 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 14:30:04 +0200 Subject: [PATCH 07/44] Story #109367694 - restore missing punctuation --- app/views/epp/contacts/info.xml.builder | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 9410368bb..5b501fc75 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -49,14 +49,14 @@ xml.epp_head do xml.tag!('contact:clID', @contact.registrar.try(:code)) # EPP requires a creator ID, which should be registrar code if we have one - crID = contact.creator.try(:registrar) + crID = @contact.creator.try(:registrar) crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? - crID = contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only + crID = @contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only xml.tag!('contact:crID', crID) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at - upID = contact.updator.try(:registrar) + upID = @contact.updator.try(:registrar) upID = upID.code if upID.present? # Did updator return a kind of User that has a registrar? xml.tag!('contact:upID', upID) if upID.present? # optional upID xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) From da11d9e7cba10ea2ec2b20c86114971a22c3abc4 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 15:27:42 +0200 Subject: [PATCH 08/44] Story #109367694 - add support for imported data, name was used --- app/models/concerns/versions.rb | 34 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index 2cbdca838..b0ac97434 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -22,32 +22,30 @@ module Versions def creator return nil if creator_str.blank? - - if creator_str =~ /^\d+-AdminUser:/ - creator = AdminUser.find_by(id: creator_str) - elsif creator_str =~ /^\d+-ApiUser:/ - creator = ApiUser.find_by(id: creator_str) - elsif creator_str =~ /^\d+-api-/ # depricated - creator = ApiUser.find_by(id: creator_str) - end - + creator = user_from_id_class_name creator_str creator.present? ? creator : creator_str end def updator return nil if updator_str.blank? - - if updator_str =~ /^\d+-AdminUser:/ - updator = AdminUser.find_by(id: updator_str) - elsif updator_str =~ /^\d+-ApiUser:/ - updator = ApiUser.find_by(id: updator_str) - elsif updator_str =~ /^\d+-api-/ # depricated - updator = ApiUser.find_by(id: updator_str) - end - + updator = user_from_id_class_name updator_str updator.present? ? updator : updator_str end + def user_from_id_class_name(str) + user = ApiUser.find_by(id: $1) if str =~ /^(\d+)-(ApiUser:|api-)/ + unless user.present? + user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ + unless user.present? + # on import we copied Registrar name, which may eql code + registrar = Registrar.find_by(name: str).first + # assume each registrar has only one user + user = registrar.api_users.first if registrar + end + end + user + end + # callbacks def touch_domain_version domain.try(:touch_with_version) From 5b47f21dbe4430cc6a7d89cee117be024ef52659 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 15:38:48 +0200 Subject: [PATCH 09/44] Story 109367694 - refactor method name --- app/controllers/application_controller.rb | 2 +- app/models/epp/domain.rb | 2 +- app/models/user.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index de54dafc5..5d051377d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,7 +67,7 @@ class ApplicationController < ActionController::Base end def user_log_str(user) - user.nil? ? 'public' : user.string + user.nil? ? 'public' : user.id_role_username end def comma_support_for(parent_key, key) diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index 1a577218a..c41adc245 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -498,7 +498,7 @@ class Epp::Domain < Domain old_registrant_email = DomainMailer.registrant_updated_notification_for_old_registrant(id, deliver_emails) preclean_pendings user = ApiUser.find(pending_json['current_user_id']) - ::PaperTrail.whodunnit = user.string # updator str should be the request originator not the approval user + ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user frame = Nokogiri::XML(pending_json['frame']) statuses.delete(DomainStatus::PENDING_UPDATE) yield(self) if block_given? # need to skip statuses check here diff --git a/app/models/user.rb b/app/models/user.rb index 5d415230c..b69e0250c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,7 +4,7 @@ class User < ActiveRecord::Base attr_accessor :phone - def string + def id_role_username "#{self.id}-#{self.class}: #{self.username}" end From 1f0e8f86e91b5e7e7661ae8e4ad3fd2814dc08d2 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 15:40:33 +0200 Subject: [PATCH 10/44] Story 109367694 - refactor method name also --- app/models/concerns/versions.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index b0ac97434..ae2dabf6a 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -22,17 +22,17 @@ module Versions def creator return nil if creator_str.blank? - creator = user_from_id_class_name creator_str + creator = user_from_id_role_username creator_str creator.present? ? creator : creator_str end def updator return nil if updator_str.blank? - updator = user_from_id_class_name updator_str + updator = user_from_id_role_username updator_str updator.present? ? updator : updator_str end - def user_from_id_class_name(str) + def user_from_id_role_username(str) user = ApiUser.find_by(id: $1) if str =~ /^(\d+)-(ApiUser:|api-)/ unless user.present? user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ From 380b86b9bdcee618a9433743d81213be8162291c Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 17:57:19 +0200 Subject: [PATCH 11/44] Story #109367694 - misplaced first --- app/models/concerns/versions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index ae2dabf6a..768cf021e 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -38,7 +38,7 @@ module Versions user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ unless user.present? # on import we copied Registrar name, which may eql code - registrar = Registrar.find_by(name: str).first + registrar = Registrar.find_by(name: str) # assume each registrar has only one user user = registrar.api_users.first if registrar end From 6c38f3e5a58e6915ec7e4fc9f1f157f95bc4290d Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 12:58:11 +0200 Subject: [PATCH 12/44] Story #109367694 - changed xml text for elements crID, clID and upID --- app/views/epp/contacts/info.xml.builder | 12 +++++------- app/views/epp/domains/info.xml.builder | 13 ++++++------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 18019208a..3790544e1 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -46,15 +46,13 @@ xml.epp_head do xml.tag!('contact:email', 'No access') end - xml.tag!('contact:clID', @contact.registrar.try(:name)) - if @contact.creator.try(:registrar).blank? && Rails.env.test? - xml.tag!('contact:crID', 'TEST-CREATOR') - else - xml.tag!('contact:crID', @contact.creator.try(:registrar)) - end + xml.tag!('contact:clID', @contact.registrar.try(:code)) + + xml.tag!('contact:crID', @contact.creator.registrar.try(:code)) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) + if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.try(:registrar)) + xml.tag!('contact:upID', @contact.updator.registrar.code) xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index 9e1779921..fd6bfeeb6 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -36,19 +36,18 @@ xml.epp_head do ## TODO Find out what this domain:host is all about - xml.tag!('domain:clID', @domain.registrar_name) - - xml.tag!('domain:crID', @domain.creator.try(:registrar)) if @domain.creator + xml.tag!('domain:clID', @domain.registrar.code) + xml.tag!('domain:crID', @domain.creator.registrar.code) if @domain.creator xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) - xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) if @domain.updated_at != @domain.created_at + if @domain.updated_at != @domain.created_at + xml.tag!('domain:upID', @domain.updator.registrar.code) + xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) + end xml.tag!('domain:exDate', @domain.valid_to.try(:iso8601)) - # TODO Make domain stampable - #xml.tag!('domain:upID', @domain.updated_by) - # TODO Make domain transferrable #xml.tag!('domain:trDate', @domain.transferred_at) if @domain.transferred_at From 04b807120689a1729f43dd61918a0dd40174236e Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 18:29:02 +0200 Subject: [PATCH 13/44] Story #109367694 - refactor user log string to user for paper trail creator_str --- app/controllers/application_controller.rb | 3 +-- app/models/user.rb | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 318923e3d..de54dafc5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,8 +67,7 @@ class ApplicationController < ActionController::Base end def user_log_str(user) - return 'public' if user.nil? - "#{user.id}-#{user.class}: #{user.username}" + user.nil? ? 'public' : user.string end def comma_support_for(parent_key, key) diff --git a/app/models/user.rb b/app/models/user.rb index 0beb174f3..5d415230c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,4 +3,9 @@ class User < ActiveRecord::Base devise :trackable, :timeoutable attr_accessor :phone + + def string + "#{self.id}-#{self.class}: #{self.username}" + end + end From 74c73b78cbe5bbc2691360d84475365290e27c5d Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 18:30:56 +0200 Subject: [PATCH 14/44] Story #109367694 - reset updator_str for pendingUpdate to update requestor requires prior commit e15ab3421d4b814eec6e4ece051e8d22cacff3b8 --- app/models/epp/domain.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index 739c55bec..5339b8695 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -503,6 +503,7 @@ class Epp::Domain < Domain old_registrant_email = DomainMailer.registrant_updated_notification_for_old_registrant(id, deliver_emails) preclean_pendings user = ApiUser.find(pending_json['current_user_id']) + ::PaperTrail.whodunnit = user.string # updator str should be the request originator not the approval user frame = Nokogiri::XML(pending_json['frame']) statuses.delete(DomainStatus::PENDING_UPDATE) yield(self) if block_given? # need to skip statuses check here From 2e52c2d102630ec95a05d3557a2963a5ca736773 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 18:40:13 +0200 Subject: [PATCH 15/44] Story #109367694 - make upID optional to avoid possible error for AdminUser objects have no registar method --- app/views/epp/contacts/info.xml.builder | 2 +- app/views/epp/domains/info.xml.builder | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 3790544e1..965e2340a 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -52,7 +52,7 @@ xml.epp_head do xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.registrar.code) + xml.tag!('contact:upID', @contact.updator.registrar.code) if @contact.updator.try(:registrar).present? xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index fd6bfeeb6..181eabc67 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -42,7 +42,7 @@ xml.epp_head do xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) if @domain.updated_at != @domain.created_at - xml.tag!('domain:upID', @domain.updator.registrar.code) + xml.tag!('domain:upID', @domain.updator.registrar.code) if @domain.updator.try(:registrar).present? xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) end From 66799c8681bd2fa08c9dc290841a3e2508475f3c Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 14:11:21 +0200 Subject: [PATCH 16/44] Story #109367694 - use creator or updator as string, when registrar can not be found --- app/views/epp/contacts/info.xml.builder | 10 ++++++++-- app/views/epp/domains/info.xml.builder | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 965e2340a..9410368bb 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -48,11 +48,17 @@ xml.epp_head do xml.tag!('contact:clID', @contact.registrar.try(:code)) - xml.tag!('contact:crID', @contact.creator.registrar.try(:code)) + # EPP requires a creator ID, which should be registrar code if we have one + crID = contact.creator.try(:registrar) + crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? + crID = contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only + xml.tag!('contact:crID', crID) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.registrar.code) if @contact.updator.try(:registrar).present? + upID = contact.updator.try(:registrar) + upID = upID.code if upID.present? # Did updator return a kind of User that has a registrar? + xml.tag!('contact:upID', upID) if upID.present? # optional upID xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index 181eabc67..11ab2441f 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -38,11 +38,17 @@ xml.epp_head do xml.tag!('domain:clID', @domain.registrar.code) - xml.tag!('domain:crID', @domain.creator.registrar.code) if @domain.creator + # EPP requires a creator ID, which should be registrar code if we have one + crID = @domain.creator.try(:registrar) + crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? + crID = @domain.creator unless crID.present? # Fallback if we failed, maybe this is a string only + xml.tag!('domain:crID', crID) xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) if @domain.updated_at != @domain.created_at - xml.tag!('domain:upID', @domain.updator.registrar.code) if @domain.updator.try(:registrar).present? + upID = @domain.updator.try(:registrar) + upID = upID.code if upID.present? # Did updator return a kind of User that has a registrar? + xml.tag!('domain:upID', upID) if upID.present? # optional upID xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) end From f927e7c79f9469bf364b8b745138bdc8278c42a5 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 14:30:04 +0200 Subject: [PATCH 17/44] Story #109367694 - restore missing punctuation --- app/views/epp/contacts/info.xml.builder | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 9410368bb..5b501fc75 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -49,14 +49,14 @@ xml.epp_head do xml.tag!('contact:clID', @contact.registrar.try(:code)) # EPP requires a creator ID, which should be registrar code if we have one - crID = contact.creator.try(:registrar) + crID = @contact.creator.try(:registrar) crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? - crID = contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only + crID = @contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only xml.tag!('contact:crID', crID) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at - upID = contact.updator.try(:registrar) + upID = @contact.updator.try(:registrar) upID = upID.code if upID.present? # Did updator return a kind of User that has a registrar? xml.tag!('contact:upID', upID) if upID.present? # optional upID xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) From 5979cc4b8b1c2c94529b92daf7f9dbc90ea2c4cb Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 15:27:42 +0200 Subject: [PATCH 18/44] Story #109367694 - add support for imported data, name was used --- app/models/concerns/versions.rb | 34 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index 2cbdca838..b0ac97434 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -22,32 +22,30 @@ module Versions def creator return nil if creator_str.blank? - - if creator_str =~ /^\d+-AdminUser:/ - creator = AdminUser.find_by(id: creator_str) - elsif creator_str =~ /^\d+-ApiUser:/ - creator = ApiUser.find_by(id: creator_str) - elsif creator_str =~ /^\d+-api-/ # depricated - creator = ApiUser.find_by(id: creator_str) - end - + creator = user_from_id_class_name creator_str creator.present? ? creator : creator_str end def updator return nil if updator_str.blank? - - if updator_str =~ /^\d+-AdminUser:/ - updator = AdminUser.find_by(id: updator_str) - elsif updator_str =~ /^\d+-ApiUser:/ - updator = ApiUser.find_by(id: updator_str) - elsif updator_str =~ /^\d+-api-/ # depricated - updator = ApiUser.find_by(id: updator_str) - end - + updator = user_from_id_class_name updator_str updator.present? ? updator : updator_str end + def user_from_id_class_name(str) + user = ApiUser.find_by(id: $1) if str =~ /^(\d+)-(ApiUser:|api-)/ + unless user.present? + user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ + unless user.present? + # on import we copied Registrar name, which may eql code + registrar = Registrar.find_by(name: str).first + # assume each registrar has only one user + user = registrar.api_users.first if registrar + end + end + user + end + # callbacks def touch_domain_version domain.try(:touch_with_version) From 054058416fffa592c22f4cd77258c86c41e2f5b7 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 15:38:48 +0200 Subject: [PATCH 19/44] Story 109367694 - refactor method name --- app/controllers/application_controller.rb | 2 +- app/models/epp/domain.rb | 2 +- app/models/user.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index de54dafc5..5d051377d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,7 +67,7 @@ class ApplicationController < ActionController::Base end def user_log_str(user) - user.nil? ? 'public' : user.string + user.nil? ? 'public' : user.id_role_username end def comma_support_for(parent_key, key) diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index 5339b8695..ec519faf5 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -503,7 +503,7 @@ class Epp::Domain < Domain old_registrant_email = DomainMailer.registrant_updated_notification_for_old_registrant(id, deliver_emails) preclean_pendings user = ApiUser.find(pending_json['current_user_id']) - ::PaperTrail.whodunnit = user.string # updator str should be the request originator not the approval user + ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user frame = Nokogiri::XML(pending_json['frame']) statuses.delete(DomainStatus::PENDING_UPDATE) yield(self) if block_given? # need to skip statuses check here diff --git a/app/models/user.rb b/app/models/user.rb index 5d415230c..b69e0250c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,7 +4,7 @@ class User < ActiveRecord::Base attr_accessor :phone - def string + def id_role_username "#{self.id}-#{self.class}: #{self.username}" end From e77a84a4333136d8ce081d0d53f33c29912d48de Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 15:40:33 +0200 Subject: [PATCH 20/44] Story 109367694 - refactor method name also --- app/models/concerns/versions.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index b0ac97434..ae2dabf6a 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -22,17 +22,17 @@ module Versions def creator return nil if creator_str.blank? - creator = user_from_id_class_name creator_str + creator = user_from_id_role_username creator_str creator.present? ? creator : creator_str end def updator return nil if updator_str.blank? - updator = user_from_id_class_name updator_str + updator = user_from_id_role_username updator_str updator.present? ? updator : updator_str end - def user_from_id_class_name(str) + def user_from_id_role_username(str) user = ApiUser.find_by(id: $1) if str =~ /^(\d+)-(ApiUser:|api-)/ unless user.present? user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ From 782c06caddeb2427f4b76aeb7b092c7c5fc5abe0 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 17:57:19 +0200 Subject: [PATCH 21/44] Story #109367694 - misplaced first --- app/models/concerns/versions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index ae2dabf6a..768cf021e 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -38,7 +38,7 @@ module Versions user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ unless user.present? # on import we copied Registrar name, which may eql code - registrar = Registrar.find_by(name: str).first + registrar = Registrar.find_by(name: str) # assume each registrar has only one user user = registrar.api_users.first if registrar end From 5aa0ae64239bcf1f6f4f131a9d6efa1dcd5bc549 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Sun, 13 Dec 2015 13:41:44 +0200 Subject: [PATCH 22/44] Story #109367694 - add better support for cr_id --- app/models/concerns/user_events.rb | 12 ++++++++++++ app/models/version/contact_version.rb | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/user_events.rb b/app/models/concerns/user_events.rb index 12ff18444..9d87b4d71 100644 --- a/app/models/concerns/user_events.rb +++ b/app/models/concerns/user_events.rb @@ -1,6 +1,18 @@ module UserEvents extend ActiveSupport::Concern + def cr_id + if versions.first.object.nil? + cr_registrar_id =versions.first.object_changes['registrar_id'].second + else + # untested, expected never to execute + cr_registrar_id = versions.first.object['registrar_id'] + end + if cr_registrar_id.present? + Registrar.find(cr_registrar_id).code + end + end + # TODO: remove old # module ClassMethods # def registrar_events(id) diff --git a/app/models/version/contact_version.rb b/app/models/version/contact_version.rb index 987dbc1fd..4b7e38364 100644 --- a/app/models/version/contact_version.rb +++ b/app/models/version/contact_version.rb @@ -3,7 +3,7 @@ class ContactVersion < PaperTrail::Version self.table_name = :log_contacts self.sequence_name = :log_contacts_id_seq - # include UserEvents + include UserEvents # scope :deleted, -> { where(event: 'destroy') } end From 026c50b3ecb28df42f33a1e2a56fb2f1ded6f201 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 09:51:27 +0200 Subject: [PATCH 23/44] Story #109367694 - remove dead code, unused since 30.1.15 --- app/models/concerns/user_events.rb | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/app/models/concerns/user_events.rb b/app/models/concerns/user_events.rb index 9d87b4d71..322bda66f 100644 --- a/app/models/concerns/user_events.rb +++ b/app/models/concerns/user_events.rb @@ -13,23 +13,4 @@ module UserEvents end end - # TODO: remove old - # module ClassMethods - # def registrar_events(id) - # registrar = Registrar.find(id) - # return [] unless registrar - # @events = [] - # registrar.users.each { |user| @events << user_events(user.id) } - # registrar.epp_users.each { |user| @events << epp_user_events(user.id) } - # @events - # end - - # def user_events(id) - # where(whodunnit: id.to_s) - # end - - # def epp_user_events(id) - # where(whodunnit: "#{id}-EppUser") - # end - # end end From 6cf2474837b210cf52c152b39c37db551389594b Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 09:58:09 +0200 Subject: [PATCH 24/44] Story #109367694 - relocate whodunit to capture save of admin approves update --- app/models/epp/domain.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index ec519faf5..d23f91013 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -503,10 +503,11 @@ class Epp::Domain < Domain old_registrant_email = DomainMailer.registrant_updated_notification_for_old_registrant(id, deliver_emails) preclean_pendings user = ApiUser.find(pending_json['current_user_id']) - ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user frame = Nokogiri::XML(pending_json['frame']) statuses.delete(DomainStatus::PENDING_UPDATE) yield(self) if block_given? # need to skip statuses check here + ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user + self.save return unless update(frame, user, false) clean_pendings! From 92a6285ef57be06b62886f2fdcd9de8fd40b9065 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 10:20:31 +0200 Subject: [PATCH 25/44] Story #109367694 - let cr_id try creator string before searching history, because it did that before. TODO: performance test and maybe reverse that order --- app/models/concerns/user_events.rb | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/app/models/concerns/user_events.rb b/app/models/concerns/user_events.rb index 322bda66f..6cf8e9ac6 100644 --- a/app/models/concerns/user_events.rb +++ b/app/models/concerns/user_events.rb @@ -1,15 +1,26 @@ module UserEvents extend ActiveSupport::Concern + # EPP requires a server defined creator ID, which should be registrar code if we have one def cr_id - if versions.first.object.nil? - cr_registrar_id =versions.first.object_changes['registrar_id'].second + # try this, rebuild user for registrar before searching history? really? + registrar = self.creator.try(:registrar) + if registrar.present? # Did creator return a kind of User that has a registrar? + registrar.code else - # untested, expected never to execute - cr_registrar_id = versions.first.object['registrar_id'] - end - if cr_registrar_id.present? - Registrar.find(cr_registrar_id).code + if versions.first.object.nil? + cr_registrar_id =versions.first.object_changes['registrar_id'].second + else + # untested, expected never to execute + cr_registrar_id = versions.first.object['registrar_id'] + end + + if cr_registrar_id.present? + Registrar.find(cr_registrar_id).code + else + # cr_id optional for domain, but required for contact; but we want something here anyway + self.creator_str # Fallback if we failed, maybe we can find a string here + end end end From b4f834e33a9aea202e8b667538c1455e249976b4 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 11:27:18 +0200 Subject: [PATCH 26/44] Story #109367694 - use cr_id in EPP info response --- app/views/epp/contacts/info.xml.builder | 6 +----- app/views/epp/domains/info.xml.builder | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 5b501fc75..39aa91b39 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -48,11 +48,7 @@ xml.epp_head do xml.tag!('contact:clID', @contact.registrar.try(:code)) - # EPP requires a creator ID, which should be registrar code if we have one - crID = @contact.creator.try(:registrar) - crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? - crID = @contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only - xml.tag!('contact:crID', crID) + xml.tag!('contact:crID', @contact.cr_id) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index 11ab2441f..ec5947b13 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -38,11 +38,7 @@ xml.epp_head do xml.tag!('domain:clID', @domain.registrar.code) - # EPP requires a creator ID, which should be registrar code if we have one - crID = @domain.creator.try(:registrar) - crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? - crID = @domain.creator unless crID.present? # Fallback if we failed, maybe this is a string only - xml.tag!('domain:crID', crID) + xml.tag!('domain:crID', @domain.cr_id) xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) if @domain.updated_at != @domain.created_at From 1cefe4ed5fa710371974bb90ed93ef7aca9c1c66 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 11:28:26 +0200 Subject: [PATCH 27/44] Story #109367694 - refactor: move UserEvents include up, wrap method in included block, say self --- app/models/concerns/user_events.rb | 37 +++++++++++++++------------ app/models/contact.rb | 1 + app/models/domain.rb | 1 + app/models/version/contact_version.rb | 4 +-- app/models/version/domain_version.rb | 2 -- 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/app/models/concerns/user_events.rb b/app/models/concerns/user_events.rb index 6cf8e9ac6..25a225e8b 100644 --- a/app/models/concerns/user_events.rb +++ b/app/models/concerns/user_events.rb @@ -1,25 +1,28 @@ module UserEvents extend ActiveSupport::Concern - # EPP requires a server defined creator ID, which should be registrar code if we have one - def cr_id - # try this, rebuild user for registrar before searching history? really? - registrar = self.creator.try(:registrar) - if registrar.present? # Did creator return a kind of User that has a registrar? - registrar.code - else - if versions.first.object.nil? - cr_registrar_id =versions.first.object_changes['registrar_id'].second + included do + # EPP requires a server defined creator ID, which should be registrar code if we have one + def cr_id + # try this, rebuild user for registrar before searching history? really? + registrar = self.creator.try(:registrar) + if registrar.present? # Did creator return a kind of User that has a registrar? + registrar.code else - # untested, expected never to execute - cr_registrar_id = versions.first.object['registrar_id'] - end + if self.versions.first.try(:object).nil? + changes = self.versions.first.try(:object_changes) + cr_registrar_id = changes['registrar_id'].second if changes.present? + else + # untested, expected never to execute + cr_registrar_id = self.versions.first.object['registrar_id'] + end - if cr_registrar_id.present? - Registrar.find(cr_registrar_id).code - else - # cr_id optional for domain, but required for contact; but we want something here anyway - self.creator_str # Fallback if we failed, maybe we can find a string here + if cr_registrar_id.present? + Registrar.find(cr_registrar_id).code + else + # cr_id optional for domain, but required for contact; but we want something here anyway + self.creator_str # Fallback if we failed, maybe we can find a string here + end end end end diff --git a/app/models/contact.rb b/app/models/contact.rb index 1d6f069e5..d17ec2354 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -1,6 +1,7 @@ class Contact < ActiveRecord::Base include Versions # version/contact_version.rb include EppErrors + include UserEvents belongs_to :registrar has_many :domain_contacts diff --git a/app/models/domain.rb b/app/models/domain.rb index 780ff3b7a..22595cd3a 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -1,5 +1,6 @@ # rubocop: disable Metrics/ClassLength class Domain < ActiveRecord::Base + include UserEvents include Versions # version/domain_version.rb include Statuses has_paper_trail class_name: "DomainVersion", meta: { children: :children_log } diff --git a/app/models/version/contact_version.rb b/app/models/version/contact_version.rb index 4b7e38364..781813302 100644 --- a/app/models/version/contact_version.rb +++ b/app/models/version/contact_version.rb @@ -2,8 +2,6 @@ class ContactVersion < PaperTrail::Version include VersionSession self.table_name = :log_contacts self.sequence_name = :log_contacts_id_seq - - include UserEvents - + # scope :deleted, -> { where(event: 'destroy') } end diff --git a/app/models/version/domain_version.rb b/app/models/version/domain_version.rb index 5bc25f1e1..2986d7811 100644 --- a/app/models/version/domain_version.rb +++ b/app/models/version/domain_version.rb @@ -4,7 +4,5 @@ class DomainVersion < PaperTrail::Version self.table_name = :log_domains self.sequence_name = :log_domains_id_seq - include UserEvents - scope :deleted, -> { where(event: 'destroy') } end From ac04a94b6a4054cd428406166ddd3e8e0bbb4ebe Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 12:58:11 +0200 Subject: [PATCH 28/44] Story #109367694 - changed xml text for elements crID, clID and upID --- app/views/epp/contacts/info.xml.builder | 12 +++++------- app/views/epp/domains/info.xml.builder | 13 ++++++------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 18019208a..3790544e1 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -46,15 +46,13 @@ xml.epp_head do xml.tag!('contact:email', 'No access') end - xml.tag!('contact:clID', @contact.registrar.try(:name)) - if @contact.creator.try(:registrar).blank? && Rails.env.test? - xml.tag!('contact:crID', 'TEST-CREATOR') - else - xml.tag!('contact:crID', @contact.creator.try(:registrar)) - end + xml.tag!('contact:clID', @contact.registrar.try(:code)) + + xml.tag!('contact:crID', @contact.creator.registrar.try(:code)) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) + if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.try(:registrar)) + xml.tag!('contact:upID', @contact.updator.registrar.code) xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index 9e1779921..fd6bfeeb6 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -36,19 +36,18 @@ xml.epp_head do ## TODO Find out what this domain:host is all about - xml.tag!('domain:clID', @domain.registrar_name) - - xml.tag!('domain:crID', @domain.creator.try(:registrar)) if @domain.creator + xml.tag!('domain:clID', @domain.registrar.code) + xml.tag!('domain:crID', @domain.creator.registrar.code) if @domain.creator xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) - xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) if @domain.updated_at != @domain.created_at + if @domain.updated_at != @domain.created_at + xml.tag!('domain:upID', @domain.updator.registrar.code) + xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) + end xml.tag!('domain:exDate', @domain.valid_to.try(:iso8601)) - # TODO Make domain stampable - #xml.tag!('domain:upID', @domain.updated_by) - # TODO Make domain transferrable #xml.tag!('domain:trDate', @domain.transferred_at) if @domain.transferred_at From 18c643ee8d75f5e2adb529a82387836ed489d5e5 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 18:29:02 +0200 Subject: [PATCH 29/44] Story #109367694 - refactor user log string to user for paper trail creator_str --- app/controllers/application_controller.rb | 3 +-- app/models/user.rb | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 318923e3d..de54dafc5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,8 +67,7 @@ class ApplicationController < ActionController::Base end def user_log_str(user) - return 'public' if user.nil? - "#{user.id}-#{user.class}: #{user.username}" + user.nil? ? 'public' : user.string end def comma_support_for(parent_key, key) diff --git a/app/models/user.rb b/app/models/user.rb index 0beb174f3..5d415230c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,4 +3,9 @@ class User < ActiveRecord::Base devise :trackable, :timeoutable attr_accessor :phone + + def string + "#{self.id}-#{self.class}: #{self.username}" + end + end From c74de1b2bd22a4f7b5a9fdfa4ae47787e534143b Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 18:30:56 +0200 Subject: [PATCH 30/44] Story #109367694 - reset updator_str for pendingUpdate to update requestor requires prior commit e15ab3421d4b814eec6e4ece051e8d22cacff3b8 --- app/models/epp/domain.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index 739c55bec..5339b8695 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -503,6 +503,7 @@ class Epp::Domain < Domain old_registrant_email = DomainMailer.registrant_updated_notification_for_old_registrant(id, deliver_emails) preclean_pendings user = ApiUser.find(pending_json['current_user_id']) + ::PaperTrail.whodunnit = user.string # updator str should be the request originator not the approval user frame = Nokogiri::XML(pending_json['frame']) statuses.delete(DomainStatus::PENDING_UPDATE) yield(self) if block_given? # need to skip statuses check here From 01ed0cedb9f1973463b9125b00f306ef1bd5fca0 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 18:40:13 +0200 Subject: [PATCH 31/44] Story #109367694 - make upID optional to avoid possible error for AdminUser objects have no registar method --- app/views/epp/contacts/info.xml.builder | 2 +- app/views/epp/domains/info.xml.builder | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 3790544e1..965e2340a 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -52,7 +52,7 @@ xml.epp_head do xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.registrar.code) + xml.tag!('contact:upID', @contact.updator.registrar.code) if @contact.updator.try(:registrar).present? xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index fd6bfeeb6..181eabc67 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -42,7 +42,7 @@ xml.epp_head do xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) if @domain.updated_at != @domain.created_at - xml.tag!('domain:upID', @domain.updator.registrar.code) + xml.tag!('domain:upID', @domain.updator.registrar.code) if @domain.updator.try(:registrar).present? xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) end From 3749c1e1b8b555dc93824cfade6178838a92bd6f Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 14:11:21 +0200 Subject: [PATCH 32/44] Story #109367694 - use creator or updator as string, when registrar can not be found --- app/views/epp/contacts/info.xml.builder | 10 ++++++++-- app/views/epp/domains/info.xml.builder | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 965e2340a..9410368bb 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -48,11 +48,17 @@ xml.epp_head do xml.tag!('contact:clID', @contact.registrar.try(:code)) - xml.tag!('contact:crID', @contact.creator.registrar.try(:code)) + # EPP requires a creator ID, which should be registrar code if we have one + crID = contact.creator.try(:registrar) + crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? + crID = contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only + xml.tag!('contact:crID', crID) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at - xml.tag!('contact:upID', @contact.updator.registrar.code) if @contact.updator.try(:registrar).present? + upID = contact.updator.try(:registrar) + upID = upID.code if upID.present? # Did updator return a kind of User that has a registrar? + xml.tag!('contact:upID', upID) if upID.present? # optional upID xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) end # xml.tag!('contact:trDate', '123') if false diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index 181eabc67..11ab2441f 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -38,11 +38,17 @@ xml.epp_head do xml.tag!('domain:clID', @domain.registrar.code) - xml.tag!('domain:crID', @domain.creator.registrar.code) if @domain.creator + # EPP requires a creator ID, which should be registrar code if we have one + crID = @domain.creator.try(:registrar) + crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? + crID = @domain.creator unless crID.present? # Fallback if we failed, maybe this is a string only + xml.tag!('domain:crID', crID) xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) if @domain.updated_at != @domain.created_at - xml.tag!('domain:upID', @domain.updator.registrar.code) if @domain.updator.try(:registrar).present? + upID = @domain.updator.try(:registrar) + upID = upID.code if upID.present? # Did updator return a kind of User that has a registrar? + xml.tag!('domain:upID', upID) if upID.present? # optional upID xml.tag!('domain:upDate', @domain.updated_at.try(:iso8601)) end From 9332de407978c5f8a4c52b10303c984d6cc2ad11 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 14:30:04 +0200 Subject: [PATCH 33/44] Story #109367694 - restore missing punctuation --- app/views/epp/contacts/info.xml.builder | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 9410368bb..5b501fc75 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -49,14 +49,14 @@ xml.epp_head do xml.tag!('contact:clID', @contact.registrar.try(:code)) # EPP requires a creator ID, which should be registrar code if we have one - crID = contact.creator.try(:registrar) + crID = @contact.creator.try(:registrar) crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? - crID = contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only + crID = @contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only xml.tag!('contact:crID', crID) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at - upID = contact.updator.try(:registrar) + upID = @contact.updator.try(:registrar) upID = upID.code if upID.present? # Did updator return a kind of User that has a registrar? xml.tag!('contact:upID', upID) if upID.present? # optional upID xml.tag!('contact:upDate', @contact.updated_at.try(:iso8601)) From 3f906b5ac6a42d2a31b0c6654df9ca1010292ef7 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 15:27:42 +0200 Subject: [PATCH 34/44] Story #109367694 - add support for imported data, name was used --- app/models/concerns/versions.rb | 34 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index 2cbdca838..b0ac97434 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -22,32 +22,30 @@ module Versions def creator return nil if creator_str.blank? - - if creator_str =~ /^\d+-AdminUser:/ - creator = AdminUser.find_by(id: creator_str) - elsif creator_str =~ /^\d+-ApiUser:/ - creator = ApiUser.find_by(id: creator_str) - elsif creator_str =~ /^\d+-api-/ # depricated - creator = ApiUser.find_by(id: creator_str) - end - + creator = user_from_id_class_name creator_str creator.present? ? creator : creator_str end def updator return nil if updator_str.blank? - - if updator_str =~ /^\d+-AdminUser:/ - updator = AdminUser.find_by(id: updator_str) - elsif updator_str =~ /^\d+-ApiUser:/ - updator = ApiUser.find_by(id: updator_str) - elsif updator_str =~ /^\d+-api-/ # depricated - updator = ApiUser.find_by(id: updator_str) - end - + updator = user_from_id_class_name updator_str updator.present? ? updator : updator_str end + def user_from_id_class_name(str) + user = ApiUser.find_by(id: $1) if str =~ /^(\d+)-(ApiUser:|api-)/ + unless user.present? + user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ + unless user.present? + # on import we copied Registrar name, which may eql code + registrar = Registrar.find_by(name: str).first + # assume each registrar has only one user + user = registrar.api_users.first if registrar + end + end + user + end + # callbacks def touch_domain_version domain.try(:touch_with_version) From e5a0149d1eece940c0b6bb329e2bfbc1136fe3db Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 15:38:48 +0200 Subject: [PATCH 35/44] Story 109367694 - refactor method name --- app/controllers/application_controller.rb | 2 +- app/models/epp/domain.rb | 2 +- app/models/user.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index de54dafc5..5d051377d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,7 +67,7 @@ class ApplicationController < ActionController::Base end def user_log_str(user) - user.nil? ? 'public' : user.string + user.nil? ? 'public' : user.id_role_username end def comma_support_for(parent_key, key) diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index 5339b8695..ec519faf5 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -503,7 +503,7 @@ class Epp::Domain < Domain old_registrant_email = DomainMailer.registrant_updated_notification_for_old_registrant(id, deliver_emails) preclean_pendings user = ApiUser.find(pending_json['current_user_id']) - ::PaperTrail.whodunnit = user.string # updator str should be the request originator not the approval user + ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user frame = Nokogiri::XML(pending_json['frame']) statuses.delete(DomainStatus::PENDING_UPDATE) yield(self) if block_given? # need to skip statuses check here diff --git a/app/models/user.rb b/app/models/user.rb index 5d415230c..b69e0250c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,7 +4,7 @@ class User < ActiveRecord::Base attr_accessor :phone - def string + def id_role_username "#{self.id}-#{self.class}: #{self.username}" end From 4bc31bb285b75f1f7ac96b0e84d6630730f3ff29 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 15:40:33 +0200 Subject: [PATCH 36/44] Story 109367694 - refactor method name also --- app/models/concerns/versions.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index b0ac97434..ae2dabf6a 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -22,17 +22,17 @@ module Versions def creator return nil if creator_str.blank? - creator = user_from_id_class_name creator_str + creator = user_from_id_role_username creator_str creator.present? ? creator : creator_str end def updator return nil if updator_str.blank? - updator = user_from_id_class_name updator_str + updator = user_from_id_role_username updator_str updator.present? ? updator : updator_str end - def user_from_id_class_name(str) + def user_from_id_role_username(str) user = ApiUser.find_by(id: $1) if str =~ /^(\d+)-(ApiUser:|api-)/ unless user.present? user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ From a2a4db0732b0a0b754c71ff2d87e7eb8e9bab844 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Fri, 4 Dec 2015 17:57:19 +0200 Subject: [PATCH 37/44] Story #109367694 - misplaced first --- app/models/concerns/versions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index ae2dabf6a..768cf021e 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -38,7 +38,7 @@ module Versions user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ unless user.present? # on import we copied Registrar name, which may eql code - registrar = Registrar.find_by(name: str).first + registrar = Registrar.find_by(name: str) # assume each registrar has only one user user = registrar.api_users.first if registrar end From ceaa70fcfc802f443575eaacd58a8660be8af7dd Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Sun, 13 Dec 2015 13:41:44 +0200 Subject: [PATCH 38/44] Story #109367694 - add better support for cr_id --- app/models/concerns/user_events.rb | 12 ++++++++++++ app/models/version/contact_version.rb | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/user_events.rb b/app/models/concerns/user_events.rb index 12ff18444..9d87b4d71 100644 --- a/app/models/concerns/user_events.rb +++ b/app/models/concerns/user_events.rb @@ -1,6 +1,18 @@ module UserEvents extend ActiveSupport::Concern + def cr_id + if versions.first.object.nil? + cr_registrar_id =versions.first.object_changes['registrar_id'].second + else + # untested, expected never to execute + cr_registrar_id = versions.first.object['registrar_id'] + end + if cr_registrar_id.present? + Registrar.find(cr_registrar_id).code + end + end + # TODO: remove old # module ClassMethods # def registrar_events(id) diff --git a/app/models/version/contact_version.rb b/app/models/version/contact_version.rb index 987dbc1fd..4b7e38364 100644 --- a/app/models/version/contact_version.rb +++ b/app/models/version/contact_version.rb @@ -3,7 +3,7 @@ class ContactVersion < PaperTrail::Version self.table_name = :log_contacts self.sequence_name = :log_contacts_id_seq - # include UserEvents + include UserEvents # scope :deleted, -> { where(event: 'destroy') } end From b9f6380e4e2a059f51a23a96a2f903ecb25eb759 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 09:51:27 +0200 Subject: [PATCH 39/44] Story #109367694 - remove dead code, unused since 30.1.15 --- app/models/concerns/user_events.rb | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/app/models/concerns/user_events.rb b/app/models/concerns/user_events.rb index 9d87b4d71..322bda66f 100644 --- a/app/models/concerns/user_events.rb +++ b/app/models/concerns/user_events.rb @@ -13,23 +13,4 @@ module UserEvents end end - # TODO: remove old - # module ClassMethods - # def registrar_events(id) - # registrar = Registrar.find(id) - # return [] unless registrar - # @events = [] - # registrar.users.each { |user| @events << user_events(user.id) } - # registrar.epp_users.each { |user| @events << epp_user_events(user.id) } - # @events - # end - - # def user_events(id) - # where(whodunnit: id.to_s) - # end - - # def epp_user_events(id) - # where(whodunnit: "#{id}-EppUser") - # end - # end end From 27482495d8a21daac3e375eb1c9fd27290e8e38d Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 09:58:09 +0200 Subject: [PATCH 40/44] Story #109367694 - relocate whodunit to capture save of admin approves update --- app/models/epp/domain.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index ec519faf5..d23f91013 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -503,10 +503,11 @@ class Epp::Domain < Domain old_registrant_email = DomainMailer.registrant_updated_notification_for_old_registrant(id, deliver_emails) preclean_pendings user = ApiUser.find(pending_json['current_user_id']) - ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user frame = Nokogiri::XML(pending_json['frame']) statuses.delete(DomainStatus::PENDING_UPDATE) yield(self) if block_given? # need to skip statuses check here + ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user + self.save return unless update(frame, user, false) clean_pendings! From d63129059816f2a46cd51596d84b607fdeb7e5da Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 10:20:31 +0200 Subject: [PATCH 41/44] Story #109367694 - let cr_id try creator string before searching history, because it did that before. TODO: performance test and maybe reverse that order --- app/models/concerns/user_events.rb | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/app/models/concerns/user_events.rb b/app/models/concerns/user_events.rb index 322bda66f..6cf8e9ac6 100644 --- a/app/models/concerns/user_events.rb +++ b/app/models/concerns/user_events.rb @@ -1,15 +1,26 @@ module UserEvents extend ActiveSupport::Concern + # EPP requires a server defined creator ID, which should be registrar code if we have one def cr_id - if versions.first.object.nil? - cr_registrar_id =versions.first.object_changes['registrar_id'].second + # try this, rebuild user for registrar before searching history? really? + registrar = self.creator.try(:registrar) + if registrar.present? # Did creator return a kind of User that has a registrar? + registrar.code else - # untested, expected never to execute - cr_registrar_id = versions.first.object['registrar_id'] - end - if cr_registrar_id.present? - Registrar.find(cr_registrar_id).code + if versions.first.object.nil? + cr_registrar_id =versions.first.object_changes['registrar_id'].second + else + # untested, expected never to execute + cr_registrar_id = versions.first.object['registrar_id'] + end + + if cr_registrar_id.present? + Registrar.find(cr_registrar_id).code + else + # cr_id optional for domain, but required for contact; but we want something here anyway + self.creator_str # Fallback if we failed, maybe we can find a string here + end end end From b7e560aacc013d7d2a7b2d9aa39e0d79e9158c27 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 11:27:18 +0200 Subject: [PATCH 42/44] Story #109367694 - use cr_id in EPP info response --- app/views/epp/contacts/info.xml.builder | 6 +----- app/views/epp/domains/info.xml.builder | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 5b501fc75..39aa91b39 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -48,11 +48,7 @@ xml.epp_head do xml.tag!('contact:clID', @contact.registrar.try(:code)) - # EPP requires a creator ID, which should be registrar code if we have one - crID = @contact.creator.try(:registrar) - crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? - crID = @contact.creator unless crID.present? # Fallback if we failed, maybe this is a string only - xml.tag!('contact:crID', crID) + xml.tag!('contact:crID', @contact.cr_id) xml.tag!('contact:crDate', @contact.created_at.try(:iso8601)) if @contact.updated_at != @contact.created_at diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder index 11ab2441f..ec5947b13 100644 --- a/app/views/epp/domains/info.xml.builder +++ b/app/views/epp/domains/info.xml.builder @@ -38,11 +38,7 @@ xml.epp_head do xml.tag!('domain:clID', @domain.registrar.code) - # EPP requires a creator ID, which should be registrar code if we have one - crID = @domain.creator.try(:registrar) - crID = crID.code if crID.present? # Did creator return a kind of User that has a registrar? - crID = @domain.creator unless crID.present? # Fallback if we failed, maybe this is a string only - xml.tag!('domain:crID', crID) + xml.tag!('domain:crID', @domain.cr_id) xml.tag!('domain:crDate', @domain.created_at.try(:iso8601)) if @domain.updated_at != @domain.created_at From 7b2767eb085010535bcf1a1965189f9c20b78aca Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 11:28:26 +0200 Subject: [PATCH 43/44] Story #109367694 - refactor: move UserEvents include up, wrap method in included block, say self --- app/models/concerns/user_events.rb | 37 +++++++++++++++------------ app/models/contact.rb | 1 + app/models/domain.rb | 1 + app/models/version/contact_version.rb | 4 +-- app/models/version/domain_version.rb | 2 -- 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/app/models/concerns/user_events.rb b/app/models/concerns/user_events.rb index 6cf8e9ac6..25a225e8b 100644 --- a/app/models/concerns/user_events.rb +++ b/app/models/concerns/user_events.rb @@ -1,25 +1,28 @@ module UserEvents extend ActiveSupport::Concern - # EPP requires a server defined creator ID, which should be registrar code if we have one - def cr_id - # try this, rebuild user for registrar before searching history? really? - registrar = self.creator.try(:registrar) - if registrar.present? # Did creator return a kind of User that has a registrar? - registrar.code - else - if versions.first.object.nil? - cr_registrar_id =versions.first.object_changes['registrar_id'].second + included do + # EPP requires a server defined creator ID, which should be registrar code if we have one + def cr_id + # try this, rebuild user for registrar before searching history? really? + registrar = self.creator.try(:registrar) + if registrar.present? # Did creator return a kind of User that has a registrar? + registrar.code else - # untested, expected never to execute - cr_registrar_id = versions.first.object['registrar_id'] - end + if self.versions.first.try(:object).nil? + changes = self.versions.first.try(:object_changes) + cr_registrar_id = changes['registrar_id'].second if changes.present? + else + # untested, expected never to execute + cr_registrar_id = self.versions.first.object['registrar_id'] + end - if cr_registrar_id.present? - Registrar.find(cr_registrar_id).code - else - # cr_id optional for domain, but required for contact; but we want something here anyway - self.creator_str # Fallback if we failed, maybe we can find a string here + if cr_registrar_id.present? + Registrar.find(cr_registrar_id).code + else + # cr_id optional for domain, but required for contact; but we want something here anyway + self.creator_str # Fallback if we failed, maybe we can find a string here + end end end end diff --git a/app/models/contact.rb b/app/models/contact.rb index 1d6f069e5..d17ec2354 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -1,6 +1,7 @@ class Contact < ActiveRecord::Base include Versions # version/contact_version.rb include EppErrors + include UserEvents belongs_to :registrar has_many :domain_contacts diff --git a/app/models/domain.rb b/app/models/domain.rb index 780ff3b7a..22595cd3a 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -1,5 +1,6 @@ # rubocop: disable Metrics/ClassLength class Domain < ActiveRecord::Base + include UserEvents include Versions # version/domain_version.rb include Statuses has_paper_trail class_name: "DomainVersion", meta: { children: :children_log } diff --git a/app/models/version/contact_version.rb b/app/models/version/contact_version.rb index 4b7e38364..781813302 100644 --- a/app/models/version/contact_version.rb +++ b/app/models/version/contact_version.rb @@ -2,8 +2,6 @@ class ContactVersion < PaperTrail::Version include VersionSession self.table_name = :log_contacts self.sequence_name = :log_contacts_id_seq - - include UserEvents - + # scope :deleted, -> { where(event: 'destroy') } end diff --git a/app/models/version/domain_version.rb b/app/models/version/domain_version.rb index 5bc25f1e1..2986d7811 100644 --- a/app/models/version/domain_version.rb +++ b/app/models/version/domain_version.rb @@ -4,7 +4,5 @@ class DomainVersion < PaperTrail::Version self.table_name = :log_domains self.sequence_name = :log_domains_id_seq - include UserEvents - scope :deleted, -> { where(event: 'destroy') } end From 6fe813e6850cf15c989d778e8b40d551a4d27d68 Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Mon, 14 Dec 2015 12:00:05 +0200 Subject: [PATCH 44/44] Story #109367694 - fix merge error --- app/models/epp/domain.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index d3699b118..40b374003 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -503,13 +503,12 @@ class Epp::Domain < Domain old_registrant_email = DomainMailer.registrant_updated_notification_for_old_registrant(id, deliver_emails) preclean_pendings user = ApiUser.find(pending_json['current_user_id']) - ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user frame = Nokogiri::XML(pending_json['frame']) statuses.delete(DomainStatus::PENDING_UPDATE) yield(self) if block_given? # need to skip statuses check here - ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user self.save + ::PaperTrail.whodunnit = user.id_role_username # updator str should be the request originator not the approval user return unless update(frame, user, false) clean_pendings! self.deliver_emails = true # turn on email delivery