From 70252f9f87aabb11d4677b0065d3136ad72d43cc Mon Sep 17 00:00:00 2001 From: Matt Farnsworth Date: Thu, 3 Dec 2015 12:58:11 +0200 Subject: [PATCH 01/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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