From 3b1e21d6eb68950aa4fc1a72478c043b10da8f71 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= <andres@gitlab.eu>
Date: Wed, 15 Oct 2014 10:35:56 +0300
Subject: [PATCH 1/9] Basic archiving overhaul

---
 .../client/domain_versions_controller.rb      |  9 +-
 app/models/contact.rb                         | 25 +++++-
 app/models/domain.rb                          | 49 ++++++++++-
 app/models/domain_version.rb                  |  4 +
 app/models/nameserver.rb                      |  8 ++
 app/views/client/domain_versions/show.haml    | 73 +++++++++++-----
 app/views/client/domains/index.haml           |  2 +
 config/locales/en.yml                         |  3 +-
 .../20141010085152_add_snapshot_to_domain.rb  |  5 ++
 db/schema.rb                                  |  3 +-
 spec/models/domain_spec.rb                    | 25 ++++++
 spec/models/domain_version_spec.rb            | 84 +++++++++++++++++++
 12 files changed, 260 insertions(+), 30 deletions(-)
 create mode 100644 db/migrate/20141010085152_add_snapshot_to_domain.rb
 create mode 100644 spec/models/domain_version_spec.rb

diff --git a/app/controllers/client/domain_versions_controller.rb b/app/controllers/client/domain_versions_controller.rb
index 87f48cd45..f716b0a6e 100644
--- a/app/controllers/client/domain_versions_controller.rb
+++ b/app/controllers/client/domain_versions_controller.rb
@@ -1,5 +1,5 @@
 class Client::DomainVersionsController < ClientController
-  before_action :set_version, only: [:show]
+  before_action :set_domain, only: [:show]
 
   def index
     @versions = DomainVersion.registrar_events(current_registrar.id)
@@ -7,13 +7,12 @@ class Client::DomainVersionsController < ClientController
   end
 
   def show
-    @event = params[:event]
-    @domain = @version.reify(has_multiple: true) unless @event == 'create'
+    @versions = @domain.versions.reverse
   end
 
   private
 
-  def set_version
-    @version = DomainVersion.find(params[:id])
+  def set_domain
+    @domain = Domain.find(params[:id])
   end
 end
diff --git a/app/models/contact.rb b/app/models/contact.rb
index a5089d4e6..7ca8f40c7 100644
--- a/app/models/contact.rb
+++ b/app/models/contact.rb
@@ -5,8 +5,6 @@ class Contact < ActiveRecord::Base
 
   include EppErrors
 
-  #has_one :local_address, dependent: :destroy
-  #has_one :international_address, dependent: :destroy
   has_one :address, dependent: :destroy
   has_one :disclosure, class_name: 'ContactDisclosure'
 
@@ -35,6 +33,11 @@ class Contact < ActiveRecord::Base
   delegate :street, to: :address#, prefix: true
   delegate :zip, to: :address#, prefix: true
 
+  # callbacks
+  #after_commit :domains_snapshot
+  after_update :domains_snapshot
+  after_destroy :domains_snapshot
+
   #scopes
   scope :current_registrars, ->(id) { where(registrar_id: id) }
   # archiving
@@ -60,6 +63,13 @@ class Contact < ActiveRecord::Base
     errors.add(:ident, 'bad format') unless code.valid?
   end
 
+  def domains_snapshot
+    (domains + domains_owned).uniq.each do |domain|
+      next unless domain.is_a?(Domain)
+      domain.touch_with_version # Method from paper_trail
+    end
+  end
+
   def juridical?
     ident_type == IDENT_TYPE_ICO
   end
@@ -124,6 +134,17 @@ class Contact < ActiveRecord::Base
     name
   end
 
+  # for archiving
+  def snapshot
+    {
+      name: name,
+      phone: phone,
+      code: code,
+      ident: ident,
+      email: email
+    }
+  end
+
   class << self
     # non-EPP
 
diff --git a/app/models/domain.rb b/app/models/domain.rb
index 5d5338947..b22594a7a 100644
--- a/app/models/domain.rb
+++ b/app/models/domain.rb
@@ -57,10 +57,31 @@ class Domain < ActiveRecord::Base
   validate :validate_dnskeys_uniqueness
   validate :validate_nameserver_ips
 
-  attr_accessor :owner_contact_typeahead
+  attr_accessor :owner_contact_typeahead, :update_me
 
   # archiving
-  has_paper_trail class_name: 'DomainVersion'
+  has_paper_trail class_name: 'DomainVersion', meta: { snapshot: :create_snapshot }
+
+  # TODO Add touch_with_version hook to the DomainContact as well to track add/delete(?) of contact
+  # Not sure what hook to use since Contact.destroy fires update from here so possibly after_create
+
+  def create_snapshot
+    oc = owner_contact.snapshot if owner_contact.is_a?(Contact)
+    {
+      owner_contact: oc,
+      tech_contacts: tech_contacts.map{ |tc| tc.snapshot },
+      admin_contacts: admin_contacts.map{ |ac| ac.snapshot },
+      nameservers: nameservers.map{ |ns| ns.snapshot },
+      domain: make_snapshot
+    }.to_yaml
+  end
+
+  def make_snapshot
+    {
+      name: name,
+      status: status
+    }
+  end
 
   def name=(value)
     value.strip!
@@ -254,4 +275,28 @@ class Domain < ActiveRecord::Base
       return period.to_i.years if unit == 'y'
     end
   end
+
+  private
+
+  #for archiving
+  #def version_owner
+  #  return nil unless owner_contact
+  #  owner_contact.id
+  #end
+
+  #def version_admin_contacts
+  #  return nil unless admin_contacts
+  #  return admin_contacts.map { |ns| ns.id }
+  #end
+
+  #def version_tech_contacts
+  #  return nil unless tech_contacts
+  #  return tech_contacts.map { |ns| ns.id }
+  #end
+
+  #def version_nameservers
+  #  return nil unless nameservers
+  #  return nameservers.map { |ns| ns.id }
+  #end
+
 end
diff --git a/app/models/domain_version.rb b/app/models/domain_version.rb
index c4d1f4a40..e85df6b4d 100644
--- a/app/models/domain_version.rb
+++ b/app/models/domain_version.rb
@@ -5,4 +5,8 @@ class DomainVersion < PaperTrail::Version
 
   self.table_name = :domain_versions
   self.sequence_name = :domain_version_id_seq
+
+  def load_snapshot
+    YAML.load(snapshot)
+  end
 end
diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb
index 6ced6cbd2..bc5a0b61c 100644
--- a/app/models/nameserver.rb
+++ b/app/models/nameserver.rb
@@ -27,6 +27,14 @@ class Nameserver < ActiveRecord::Base
     }
   end
 
+  def snapshot
+    {
+      hostname: hostname,
+      ipv4: ipv4,
+      ipv6: ipv6
+    }
+  end
+
   def to_s
     hostname
   end
diff --git a/app/views/client/domain_versions/show.haml b/app/views/client/domain_versions/show.haml
index 3e2ccdfb0..48f650b7c 100644
--- a/app/views/client/domain_versions/show.haml
+++ b/app/views/client/domain_versions/show.haml
@@ -2,23 +2,58 @@
   .col-sm-6
     %h2.text-center-xs= t('shared.domains')
 %hr
-.row
-  - if @event != 'create'
-    .col-sm-6= render 'client/domains/partials/general'
-  - if @event != 'create' && @domain.owner_contact
-    .col-sm-6= render 'client/domains/partials/owner'
-  - if @event != 'create' && @domain.tech_contacts
-    .col-sm-6= render 'client/domains/partials/tech_contacts'
-  .col-sm-6
-    .panel.panel-default
-      .panel-heading
-        %h3.panel-title= t('shared.version')
-      .panel-body
-        %dl.dl-horizontal
-          %dt= t('shared.whodunnit')
-          %dd= @version.whodunnit
-          %dt= t('shared.event')
-          %dd= @version.event
-          %dt= t('shared.created_at')
-          %dd= l(@version.created_at, format: :short)
+
+.row
+  .col-md-12
+    .table-responsive
+      %table.table-hover.table-bordered.table-condensed
+        %thead
+          %tr
+            %th{class: 'col-xs-1'}= 'domain'
+            %th{class: 'col-xs-2'}= 'owner'
+            %th{class: 'col-xs-2'}= 'admins'
+            %th{class: 'col-xs-2'}= 'techs'
+            %th{class: 'col-xs-2'}= 'ns'
+            %th{class: 'col-xs-1'}= 'datetime'
+        %tbody
+          - @versions.each do |version|
+            - children = YAML.load(version.snapshot)
+            - next unless children.is_a?(Hash)
+            - children = HashWithIndifferentAccess.new(children)
+            %tr
+              %td
+                - if children[:domain]
+                  = children[:domain][:name]
+                  = children[:domain][:status]
+              %td
+                - if children[:owner_contact]
+                  %p{:style => "font-size:x-small;"}
+                    = children[:owner_contact][:name] + ","
+                    = children[:owner_contact][:phone] + ","
+                    = children[:owner_contact][:email] + ","
+                    = children[:owner_contact][:code]
+              %td
+                - if children[:admin_contacts]
+                  - children[:admin_contacts].each do |ac|
+                    %p{:style => "font-size:x-small;"}
+                      = ac[:name] + ","
+                      = ac[:phone] + ","
+                      = ac[:email] + ","
+                      = ac[:code]
+              %td
+                - if children[:tech_contacts]
+                  - children[:tech_contacts].each do |tc|
+                    %p{:style => "font-size:x-small;"}
+                      = tc[:name] + ","
+                      = tc[:phone] + ","
+                      = tc[:email] + ","
+                      = tc[:code]
+              %td
+                - if children[:nameservers]
+                  - children[:nameservers].each do |ns|
+                    %p{:style => "font-size:x-small;"}
+                      = ns[:hostname] + ","
+                      = ns[:ipv4] || ns[:ipv6]
+
+              %td= l(version.created_at, format: :short)
 
diff --git a/app/views/client/domains/index.haml b/app/views/client/domains/index.haml
index 7ee09354a..b1ecf3421 100644
--- a/app/views/client/domains/index.haml
+++ b/app/views/client/domains/index.haml
@@ -30,12 +30,14 @@
               = sort_link(@q, 'owner_contact_name', t('shared.owner'))
             %th{class: 'col-xs-2'}
               = sort_link(@q, 'valid_to', t('shared.valid_to'))
+            %th{class: 'col-xs-1'}
         %tbody
           - @domains.each do |x|
             %tr
               %td= link_to(x, client_domain_path(x))
               %td= link_to(x.owner_contact, [:client, x.owner_contact])
               %td= l(x.valid_to, format: :short)
+              %td= link_to t('shared.history'), client_domain_version_path(x.id), class: 'btn btn-primary'
 .row
   .col-md-12
     = paginate @domains
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 2a2e864bb..de0b819a7 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -375,6 +375,7 @@ en:
     contact_list: 'Contact list'
     create_new_contact: 'Create new contact'
     domain_pw: 'Domain password'
+    history: 'History'
 
     new_registrar: 'New registrar'
     registrar_added: 'Registrar added!'
@@ -417,7 +418,7 @@ en:
     record_deleted: 'Record deleted'
     failed_to_delete_record: 'Failed to delete record'
 
-    # sorry these need to be refactored - Andres
     authentication_error: 'Authentication error'
 
   registrar: Registrar
+
diff --git a/db/migrate/20141010085152_add_snapshot_to_domain.rb b/db/migrate/20141010085152_add_snapshot_to_domain.rb
new file mode 100644
index 000000000..3d995e195
--- /dev/null
+++ b/db/migrate/20141010085152_add_snapshot_to_domain.rb
@@ -0,0 +1,5 @@
+class AddSnapshotToDomain < ActiveRecord::Migration
+  def change
+    add_column :domain_versions, :snapshot, :text
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index a3e7eaba6..ce81b0b88 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20141006130306) do
+ActiveRecord::Schema.define(version: 20141010085152) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -143,6 +143,7 @@ ActiveRecord::Schema.define(version: 20141006130306) do
     t.string   "whodunnit"
     t.text     "object"
     t.datetime "created_at"
+    t.text     "snapshot"
   end
 
   add_index "domain_versions", ["item_type", "item_id"], name: "index_domain_versions_on_item_type_and_item_id", using: :btree
diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb
index 209a862fc..9686b5208 100644
--- a/spec/models/domain_spec.rb
+++ b/spec/models/domain_spec.rb
@@ -82,4 +82,29 @@ describe Domain do
       expect(d.auth_info).to_not be_empty
     end
   end
+
+  with_versioning do
+    context 'when not saved' do
+      it 'does not create domain version' do
+        Fabricate.build(:domain)
+        expect(DomainVersion.count).to eq(0)
+      end
+
+      it 'does not create child versions' do
+        Fabricate.build(:domain)
+        expect(ContactVersion.count).to eq(0)
+        expect(NameserverVersion.count).to eq(0)
+      end
+    end
+
+    context 'when saved' do
+      before(:each) { Fabricate(:domain_validation_setting_group); Fabricate(:domain) }
+      it 'creates domain version' do
+
+        expect(DomainVersion.count).to eq(1)
+        expect(ContactVersion.count).to eq(2)
+        expect(NameserverVersion.count).to eq(3)
+      end
+    end
+  end
 end
diff --git a/spec/models/domain_version_spec.rb b/spec/models/domain_version_spec.rb
new file mode 100644
index 000000000..f2585e036
--- /dev/null
+++ b/spec/models/domain_version_spec.rb
@@ -0,0 +1,84 @@
+require 'rails_helper'
+
+describe DomainVersion do
+  with_versioning do
+    before(:each) { Fabricate(:domain_validation_setting_group) }
+    before(:each) do
+      Fabricate(:domain, name: 'version.ee') do
+        owner_contact { Fabricate(:contact, name: 'owner_contact', code: 'asd', email: 'owner1@v.ee') }
+        nameservers(count: 1) { Fabricate(:nameserver, hostname: 'ns.test.ee')}
+        admin_contacts(count: 1) { Fabricate(:contact, name: 'admin_contact 1', code: 'qwe', email: 'admin1@v.ee') }
+        tech_contacts(count: 1) { Fabricate(:contact, name: 'tech_contact 1', code: 'zxc', email: 'tech1@v.ee') }
+      end
+    end
+
+    context 'when domain is created' do
+      it('creates a domain version') { expect(DomainVersion.count).to eq(1) }
+      it('has a snapshot') { expect(DomainVersion.first.snapshot).not_to be_empty }
+      it 'has a snapshot with correct info' do
+        expect(DomainVersion.last.load_snapshot).to eq({
+          :admin_contacts => [{:name=>"admin_contact 1", :phone=>"+372.12345678", :code=>"qwe", :ident=>"37605030299", :email=>"admin1@v.ee"}],
+          :domain => {:name=>"version.ee", :status=>nil},
+          :nameservers => [{:hostname=>"ns.test.ee", :ipv4=>nil, :ipv6=>nil}],
+          :owner_contact => {:name=>"owner_contact", :phone=>"+372.12345678", :code=>"asd", :ident=>"37605030299", :email=>"owner1@v.ee"},
+          :tech_contacts => [{:name=>"tech_contact 1", :phone=>"+372.12345678", :code=>"zxc", :ident=>"37605030299", :email=>"tech1@v.ee"}]
+        })
+      end
+    end
+
+    context 'when domain is deleted' do
+      it 'creates a version' do
+        expect(DomainVersion.count).to eq(1)
+        Domain.first.destroy
+        expect(DomainVersion.count).to eq(2)
+        expect(DomainVersion.last.load_snapshot).to eq({
+          :admin_contacts => [],
+          :domain => {:name=>"version.ee", :status=>nil},
+          :nameservers => [],
+          :owner_contact => {:name=>"owner_contact", :phone=>"+372.12345678", :code=>"asd", :ident=>"37605030299", :email=>"owner1@v.ee"},
+          :tech_contacts => []
+        })
+      end
+    end
+
+    context 'when deleting children' do
+      it 'creates a version' do
+        expect(DomainVersion.count).to eq(1)
+        Contact.find_by(name: 'tech_contact 1').destroy
+        expect(DomainVersion.count).to eq(2)
+        expect(DomainVersion.last.load_snapshot).to eq({
+          :admin_contacts => [{:name=>"admin_contact 1", :phone=>"+372.12345678", :code=>"qwe", :ident=>"37605030299", :email=>"admin1@v.ee"}],
+          :domain => {:name=>"version.ee", :status=>nil},
+          :nameservers => [{:hostname=>"ns.test.ee", :ipv4=>nil, :ipv6=>nil}],
+          :owner_contact => {:name=>"owner_contact", :phone=>"+372.12345678", :code=>"asd", :ident=>"37605030299", :email=>"owner1@v.ee"},
+          :tech_contacts => []
+        })
+      end
+    end
+
+    context 'when editing children' do
+      it 'creates a version' do
+        expect(DomainVersion.count).to eq(1)
+        Contact.find_by(name: 'owner_contact').update_attributes!(name: 'edited owner_contact')
+        expect(DomainVersion.count).to eq(2)
+      end
+
+      it 'creates 3 versions' do
+        expect(DomainVersion.count).to eq(1)
+        Contact.find_by(name: 'owner_contact').update_attributes!(name: 'edited owner_contact')
+        expect(DomainVersion.count).to eq(2)
+        Contact.find_by(name: 'tech_contact 1').update_attributes!(name: 'edited tech_contact')
+        expect(DomainVersion.count).to eq(3)
+        Contact.find_by(name: 'admin_contact 1').update_attributes!(name: 'edited admin_contact')
+        expect(DomainVersion.count).to eq(4)
+        expect(DomainVersion.last.load_snapshot).to eq({
+          :admin_contacts => [{:name=>"edited admin_contact", :phone=>"+372.12345678", :code=>"qwe", :ident=>"37605030299", :email=>"admin1@v.ee"}],
+          :domain => {:name=>"version.ee", :status=>nil},
+          :nameservers => [{:hostname=>"ns.test.ee", :ipv4=>nil, :ipv6=>nil}],
+          :owner_contact => {:name=>"edited owner_contact", :phone=>"+372.12345678", :code=>"asd", :ident=>"37605030299", :email=>"owner1@v.ee"},
+          :tech_contacts => [{:name=>"edited tech_contact", :phone=>"+372.12345678", :code=>"zxc", :ident=>"37605030299", :email=>"tech1@v.ee"}]
+        })
+      end
+    end
+  end
+end

From 7b05cef8eae5cfd162302d9ebb0449a9f19dbd03 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= <andres@gitlab.eu>
Date: Wed, 15 Oct 2014 12:15:30 +0300
Subject: [PATCH 2/9] Rubocop fixes

---
 app/models/domain.rb               | 25 +++++++-------
 spec/models/domain_spec.rb         |  7 +++-
 spec/models/domain_version_spec.rb | 53 +++++++++++++++++-------------
 3 files changed, 49 insertions(+), 36 deletions(-)

diff --git a/app/models/domain.rb b/app/models/domain.rb
index aa37461eb..8afe86671 100644
--- a/app/models/domain.rb
+++ b/app/models/domain.rb
@@ -69,9 +69,9 @@ class Domain < ActiveRecord::Base
     oc = owner_contact.snapshot if owner_contact.is_a?(Contact)
     {
       owner_contact: oc,
-      tech_contacts: tech_contacts.map{ |tc| tc.snapshot },
-      admin_contacts: admin_contacts.map{ |ac| ac.snapshot },
-      nameservers: nameservers.map{ |ns| ns.snapshot },
+      tech_contacts: tech_contacts.map(&:snapshot),
+      admin_contacts: admin_contacts.map(&:snapshot),
+      nameservers: nameservers.map(&:snapshot),
       domain: make_snapshot
     }.to_yaml
   end
@@ -293,25 +293,24 @@ class Domain < ActiveRecord::Base
 
   private
 
-  #for archiving
-  #def version_owner
+  # for archiving
+  # def version_owner
   #  return nil unless owner_contact
   #  owner_contact.id
-  #end
+  # end
 
-  #def version_admin_contacts
+  # def version_admin_contacts
   #  return nil unless admin_contacts
   #  return admin_contacts.map { |ns| ns.id }
-  #end
+  # end
 
-  #def version_tech_contacts
+  # def version_tech_contacts
   #  return nil unless tech_contacts
   #  return tech_contacts.map { |ns| ns.id }
-  #end
+  # end
 
-  #def version_nameservers
+  # def version_nameservers
   #  return nil unless nameservers
   #  return nameservers.map { |ns| ns.id }
-  #end
-
+  # end
 end
diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb
index 1d5dc70c2..edef63e71 100644
--- a/spec/models/domain_spec.rb
+++ b/spec/models/domain_spec.rb
@@ -102,7 +102,12 @@ describe Domain do
     end
 
     context 'when saved' do
-      before(:each) { Fabricate(:domain_validation_setting_group); Fabricate(:domain) }
+      before(:each) do
+        Fabricate(:domain_validation_setting_group)
+        Fabricate(:dnskeys_setting_group)
+        Fabricate(:domain)
+      end
+
       it 'creates domain version' do
 
         expect(DomainVersion.count).to eq(1)
diff --git a/spec/models/domain_version_spec.rb b/spec/models/domain_version_spec.rb
index f2585e036..7c69c9f18 100644
--- a/spec/models/domain_version_spec.rb
+++ b/spec/models/domain_version_spec.rb
@@ -2,11 +2,11 @@ require 'rails_helper'
 
 describe DomainVersion do
   with_versioning do
-    before(:each) { Fabricate(:domain_validation_setting_group) }
+    before(:each) { Fabricate(:domain_validation_setting_group); Fabricate(:dnskeys_setting_group) }
     before(:each) do
       Fabricate(:domain, name: 'version.ee') do
         owner_contact { Fabricate(:contact, name: 'owner_contact', code: 'asd', email: 'owner1@v.ee') }
-        nameservers(count: 1) { Fabricate(:nameserver, hostname: 'ns.test.ee')}
+        nameservers(count: 1) { Fabricate(:nameserver, hostname: 'ns.test.ee') }
         admin_contacts(count: 1) { Fabricate(:contact, name: 'admin_contact 1', code: 'qwe', email: 'admin1@v.ee') }
         tech_contacts(count: 1) { Fabricate(:contact, name: 'tech_contact 1', code: 'zxc', email: 'tech1@v.ee') }
       end
@@ -17,11 +17,14 @@ describe DomainVersion do
       it('has a snapshot') { expect(DomainVersion.first.snapshot).not_to be_empty }
       it 'has a snapshot with correct info' do
         expect(DomainVersion.last.load_snapshot).to eq({
-          :admin_contacts => [{:name=>"admin_contact 1", :phone=>"+372.12345678", :code=>"qwe", :ident=>"37605030299", :email=>"admin1@v.ee"}],
-          :domain => {:name=>"version.ee", :status=>nil},
-          :nameservers => [{:hostname=>"ns.test.ee", :ipv4=>nil, :ipv6=>nil}],
-          :owner_contact => {:name=>"owner_contact", :phone=>"+372.12345678", :code=>"asd", :ident=>"37605030299", :email=>"owner1@v.ee"},
-          :tech_contacts => [{:name=>"tech_contact 1", :phone=>"+372.12345678", :code=>"zxc", :ident=>"37605030299", :email=>"tech1@v.ee"}]
+          admin_contacts: [{ name: 'admin_contact 1', phone: '+372.12345678',
+                             code: 'qwe', ident: '37605030299', email: 'admin1@v.ee' }],
+          domain: { name: 'version.ee', status: nil },
+          nameservers: [{ hostname: 'ns.test.ee', ipv4: nil, ipv6: nil }],
+          owner_contact: { name: 'owner_contact', phone: '+372.12345678',
+                           code: 'asd', ident: '37605030299', email: 'owner1@v.ee' },
+          tech_contacts: [{ name: 'tech_contact 1', phone: '+372.12345678',
+                            code: 'zxc', ident: '37605030299', email: 'tech1@v.ee' }]
         })
       end
     end
@@ -32,11 +35,12 @@ describe DomainVersion do
         Domain.first.destroy
         expect(DomainVersion.count).to eq(2)
         expect(DomainVersion.last.load_snapshot).to eq({
-          :admin_contacts => [],
-          :domain => {:name=>"version.ee", :status=>nil},
-          :nameservers => [],
-          :owner_contact => {:name=>"owner_contact", :phone=>"+372.12345678", :code=>"asd", :ident=>"37605030299", :email=>"owner1@v.ee"},
-          :tech_contacts => []
+          admin_contacts: [],
+          domain: { name: 'version.ee', status: nil },
+          nameservers: [],
+          owner_contact: { name: 'owner_contact', phone: '+372.12345678',
+                           code: 'asd', ident: '37605030299', email: 'owner1@v.ee' },
+          tech_contacts: []
         })
       end
     end
@@ -47,11 +51,13 @@ describe DomainVersion do
         Contact.find_by(name: 'tech_contact 1').destroy
         expect(DomainVersion.count).to eq(2)
         expect(DomainVersion.last.load_snapshot).to eq({
-          :admin_contacts => [{:name=>"admin_contact 1", :phone=>"+372.12345678", :code=>"qwe", :ident=>"37605030299", :email=>"admin1@v.ee"}],
-          :domain => {:name=>"version.ee", :status=>nil},
-          :nameservers => [{:hostname=>"ns.test.ee", :ipv4=>nil, :ipv6=>nil}],
-          :owner_contact => {:name=>"owner_contact", :phone=>"+372.12345678", :code=>"asd", :ident=>"37605030299", :email=>"owner1@v.ee"},
-          :tech_contacts => []
+          admin_contacts: [{ name: 'admin_contact 1', phone: '+372.12345678',
+                             code: 'qwe', ident: '37605030299', email: 'admin1@v.ee' }],
+          domain: { name: 'version.ee', status: nil },
+          nameservers: [{ hostname: 'ns.test.ee', ipv4: nil, ipv6: nil }],
+          owner_contact: { name: 'owner_contact', phone: '+372.12345678',
+                           code: 'asd', ident: '37605030299', email: 'owner1@v.ee' },
+          tech_contacts: []
         })
       end
     end
@@ -72,11 +78,14 @@ describe DomainVersion do
         Contact.find_by(name: 'admin_contact 1').update_attributes!(name: 'edited admin_contact')
         expect(DomainVersion.count).to eq(4)
         expect(DomainVersion.last.load_snapshot).to eq({
-          :admin_contacts => [{:name=>"edited admin_contact", :phone=>"+372.12345678", :code=>"qwe", :ident=>"37605030299", :email=>"admin1@v.ee"}],
-          :domain => {:name=>"version.ee", :status=>nil},
-          :nameservers => [{:hostname=>"ns.test.ee", :ipv4=>nil, :ipv6=>nil}],
-          :owner_contact => {:name=>"edited owner_contact", :phone=>"+372.12345678", :code=>"asd", :ident=>"37605030299", :email=>"owner1@v.ee"},
-          :tech_contacts => [{:name=>"edited tech_contact", :phone=>"+372.12345678", :code=>"zxc", :ident=>"37605030299", :email=>"tech1@v.ee"}]
+          admin_contacts: [{ name: 'edited admin_contact', phone: '+372.12345678',
+                             code: 'qwe', ident: '37605030299', email: 'admin1@v.ee' }],
+          domain: { name: 'version.ee', status: nil },
+          nameservers: [{ hostname: 'ns.test.ee', ipv4: nil, ipv6: nil }],
+          owner_contact: { name: 'edited owner_contact', phone: '+372.12345678',
+                           code: 'asd', ident: '37605030299', email: 'owner1@v.ee' },
+          tech_contacts: [{ name: 'edited tech_contact', phone: '+372.12345678',
+                            code: 'zxc', ident: '37605030299', email: 'tech1@v.ee' }]
         })
       end
     end

From cb37ddf96de30d7031f1173918d693ac7967df3a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= <andres@gitlab.eu>
Date: Thu, 16 Oct 2014 11:38:29 +0300
Subject: [PATCH 3/9] DomainContact versioning and tests for versioning

---
 app/models/domain_contact.rb               | 11 +++++
 app/views/client/domain_versions/show.haml |  6 ++-
 spec/models/domain_version_spec.rb         | 47 ++++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/app/models/domain_contact.rb b/app/models/domain_contact.rb
index 9ea598b79..479ab5890 100644
--- a/app/models/domain_contact.rb
+++ b/app/models/domain_contact.rb
@@ -3,6 +3,10 @@ class DomainContact < ActiveRecord::Base
   belongs_to :contact
   belongs_to :domain
 
+  after_create :domain_snapshot
+  after_destroy :domain_snapshot
+#  after_save :domain_snapshot
+
   attr_accessor :value_typeahead
 
   def epp_code_map
@@ -33,4 +37,11 @@ class DomainContact < ActiveRecord::Base
   def value_typeahead
     @value_typeahead || contact.try(:name) || nil
   end
+
+  def domain_snapshot
+    # We don't create a version unless domain is valid, is that a good idea?
+    return true unless PaperTrail.enabled?
+    domain.touch_with_version if domain.valid?
+    true
+  end
 end
diff --git a/app/views/client/domain_versions/show.haml b/app/views/client/domain_versions/show.haml
index 48f650b7c..a355dbd5c 100644
--- a/app/views/client/domain_versions/show.haml
+++ b/app/views/client/domain_versions/show.haml
@@ -55,5 +55,9 @@
                       = ns[:hostname] + ","
                       = ns[:ipv4] || ns[:ipv6]
 
-              %td= l(version.created_at, format: :short)
+              %td
+                %p{ :style => 'font-size:x-small;' }
+                  = l(version.created_at, format: :short)
+                  = version.whodunnit
+                  = version.event
 
diff --git a/spec/models/domain_version_spec.rb b/spec/models/domain_version_spec.rb
index 7c69c9f18..5f7c782c3 100644
--- a/spec/models/domain_version_spec.rb
+++ b/spec/models/domain_version_spec.rb
@@ -45,6 +45,53 @@ describe DomainVersion do
       end
     end
 
+    context 'when adding child' do
+      it 'contact creates a version' do
+        expect(DomainVersion.count).to eq(1)
+        expect(Domain.last.tech_contacts.count).to eq(1)
+        Domain.last.tech_contacts << Fabricate(:contact, name: 'tech contact 2', phone: '+371.12345678',
+                                                code: '123', email: 'tech2@v.ee')
+        expect(Domain.last.tech_contacts.count).to eq(2)
+        expect(DomainVersion.count).to eq(2)
+      end
+
+      it 'nameserver creates a version' do
+        expect(DomainVersion.count).to eq(1)
+        expect(Domain.last.nameservers.count).to eq(1)
+        Domain.last.nameservers << Fabricate(:nameserver, hostname: 'ns.server.ee')
+        expect(DomainVersion.count).to eq(2)
+      end
+    end
+
+    context 'when removing child' do
+      it('has one domain version before events'){ expect(DomainVersion.count).to eq(1) }
+      before(:each) { Domain.last.nameservers << Fabricate(:nameserver) }
+
+      it 'contact creates a version' do
+        # FIXME For some reason nameservers disappeared mid-test, but randomly stopped happening
+        expect(DomainVersion.count).to eq(1)
+        DomainContact.last.destroy
+        expect(Domain.last.valid?).to be(true)
+        expect(DomainVersion.count).to eq(2)
+      end
+
+      it 'nameserver creates a version' do
+        Domain.last.nameservers.last.destroy
+        expect(DomainVersion.count).to eq(1)
+        expect(Domain.last.nameservers.count).to eq(1)
+        expect(DomainVersion.load_snapshot).to eq(
+          admin_contacts: [{ name: 'admin_contact 1', phone: '+372.12345678',
+                             code: 'qwe', ident: '37605030299', email: 'admin1@v.ee' }],
+          domain: { name: 'version.ee', status: nil },
+          nameservers: [{ hostname: 'ns.test.ee', ipv4: nil, ipv6: nil }],
+          owner_contact: { name: 'owner_contact', phone: '+372.12345678',
+                           code: 'asd', ident: '37605030299', email: 'owner1@v.ee' },
+          tech_contacts: [{ name: 'tech_contact 1', phone: '+372.12345678',
+                            code: 'zxc', ident: '37605030299', email: 'tech1@v.ee' }]
+        )
+      end
+    end
+
     context 'when deleting children' do
       it 'creates a version' do
         expect(DomainVersion.count).to eq(1)

From 58077a7989e861cc3ae7054ecdccc22c3334892e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= <andres@gitlab.eu>
Date: Thu, 16 Oct 2014 12:01:34 +0300
Subject: [PATCH 4/9] Style fixes

---
 app/models/domain.rb               | 26 --------------------------
 app/models/domain_contact.rb       |  2 +-
 spec/models/domain_version_spec.rb |  4 ++--
 3 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/app/models/domain.rb b/app/models/domain.rb
index 8afe86671..43f0629bf 100644
--- a/app/models/domain.rb
+++ b/app/models/domain.rb
@@ -62,9 +62,6 @@ class Domain < ActiveRecord::Base
   # archiving
   has_paper_trail class_name: 'DomainVersion', meta: { snapshot: :create_snapshot }
 
-  # TODO Add touch_with_version hook to the DomainContact as well to track add/delete(?) of contact
-  # Not sure what hook to use since Contact.destroy fires update from here so possibly after_create
-
   def create_snapshot
     oc = owner_contact.snapshot if owner_contact.is_a?(Contact)
     {
@@ -290,27 +287,4 @@ class Domain < ActiveRecord::Base
       return period.to_i.years if unit == 'y'
     end
   end
-
-  private
-
-  # for archiving
-  # def version_owner
-  #  return nil unless owner_contact
-  #  owner_contact.id
-  # end
-
-  # def version_admin_contacts
-  #  return nil unless admin_contacts
-  #  return admin_contacts.map { |ns| ns.id }
-  # end
-
-  # def version_tech_contacts
-  #  return nil unless tech_contacts
-  #  return tech_contacts.map { |ns| ns.id }
-  # end
-
-  # def version_nameservers
-  #  return nil unless nameservers
-  #  return nameservers.map { |ns| ns.id }
-  # end
 end
diff --git a/app/models/domain_contact.rb b/app/models/domain_contact.rb
index 479ab5890..472496d79 100644
--- a/app/models/domain_contact.rb
+++ b/app/models/domain_contact.rb
@@ -5,7 +5,7 @@ class DomainContact < ActiveRecord::Base
 
   after_create :domain_snapshot
   after_destroy :domain_snapshot
-#  after_save :domain_snapshot
+  #  after_save :domain_snapshot
 
   attr_accessor :value_typeahead
 
diff --git a/spec/models/domain_version_spec.rb b/spec/models/domain_version_spec.rb
index 5f7c782c3..270f60a4c 100644
--- a/spec/models/domain_version_spec.rb
+++ b/spec/models/domain_version_spec.rb
@@ -64,11 +64,11 @@ describe DomainVersion do
     end
 
     context 'when removing child' do
-      it('has one domain version before events'){ expect(DomainVersion.count).to eq(1) }
+      it('has one domain version before events') { expect(DomainVersion.count).to eq(1) }
       before(:each) { Domain.last.nameservers << Fabricate(:nameserver) }
 
       it 'contact creates a version' do
-        # FIXME For some reason nameservers disappeared mid-test, but randomly stopped happening
+        # FIXME: For some reason nameservers disappeared mid-test, but randomly stopped happening
         expect(DomainVersion.count).to eq(1)
         DomainContact.last.destroy
         expect(Domain.last.valid?).to be(true)

From 6b783fd45ecbe350b51f6895a3b5056f2e3440aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= <andres@gitlab.eu>
Date: Mon, 20 Oct 2014 11:23:05 +0300
Subject: [PATCH 5/9] nameserver change tracking

---
 app/models/contact.rb              |  1 +
 app/models/domain.rb               |  8 +++++-
 app/models/domain_contact.rb       |  3 ++
 app/models/nameserver.rb           |  5 ++++
 config/environments/test.rb        |  2 +-
 spec/models/address_spec.rb        |  2 +-
 spec/models/domain_spec.rb         |  5 ++--
 spec/models/domain_version_spec.rb | 44 ++++++++++++++++--------------
 8 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/app/models/contact.rb b/app/models/contact.rb
index f25729270..7ebf25478 100644
--- a/app/models/contact.rb
+++ b/app/models/contact.rb
@@ -66,6 +66,7 @@ class Contact < ActiveRecord::Base
   def domains_snapshot
     (domains + domains_owned).uniq.each do |domain|
       next unless domain.is_a?(Domain)
+      next if domain.versions.last == domain.create_snapshot
       domain.touch_with_version # Method from paper_trail
     end
   end
diff --git a/app/models/domain.rb b/app/models/domain.rb
index 29520575e..dcfd56e86 100644
--- a/app/models/domain.rb
+++ b/app/models/domain.rb
@@ -17,7 +17,8 @@ class Domain < ActiveRecord::Base
            -> { where(domain_contacts: { contact_type: DomainContact::ADMIN }) },
            through: :domain_contacts, source: :contact
 
-  has_many :nameservers, dependent: :delete_all
+  has_many :nameservers, dependent: :delete_all, after_add: :track_nameserver_add
+
   accepts_nested_attributes_for :nameservers, allow_destroy: true,
                                               reject_if: proc { |attrs| attrs[:hostname].blank? }
 
@@ -62,6 +63,11 @@ class Domain < ActiveRecord::Base
   # archiving
   has_paper_trail class_name: 'DomainVersion', meta: { snapshot: :create_snapshot }
 
+  def track_nameserver_add(nameserver)
+    # if we are not adding nameservers on create ( we don't care about ms so to_i )
+    touch_with_version if nameserver.created_at.to_i != created_at.to_i && valid?
+  end
+
   def create_snapshot
     oc = owner_contact.snapshot if owner_contact.is_a?(Contact)
     {
diff --git a/app/models/domain_contact.rb b/app/models/domain_contact.rb
index 472496d79..c334a58fc 100644
--- a/app/models/domain_contact.rb
+++ b/app/models/domain_contact.rb
@@ -41,6 +41,9 @@ class DomainContact < ActiveRecord::Base
   def domain_snapshot
     # We don't create a version unless domain is valid, is that a good idea?
     return true unless PaperTrail.enabled?
+    return true if domain.nil?
+    return true if contact.nil?
+    return true if domain.versions.last.try(:snapshot) == domain.try(:create_snapshot)
     domain.touch_with_version if domain.valid?
     true
   end
diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb
index 5417d191b..9c01b3cf2 100644
--- a/app/models/nameserver.rb
+++ b/app/models/nameserver.rb
@@ -12,6 +12,7 @@ class Nameserver < ActiveRecord::Base
 
   # archiving
   has_paper_trail class_name: 'NameserverVersion'
+  after_destroy :domain_version
 
   before_validation :normalize_attributes
 
@@ -45,6 +46,10 @@ class Nameserver < ActiveRecord::Base
     self.ipv6 = ipv6.try(:strip).try(:upcase)
   end
 
+  def domain_version
+    domain.touch_with_version if domain.valid?
+  end
+
   def to_s
     hostname
   end
diff --git a/config/environments/test.rb b/config/environments/test.rb
index dfc9384e2..f69df6cd3 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -41,6 +41,6 @@ Rails.application.configure do
   config.after_initialize do
     Bullet.enable = true
     Bullet.bullet_logger = true
-    Bullet.raise = true # raise an error if n+1 query occurs
+    Bullet.raise = false # raise an error if n+1 query occurs
   end
 end
diff --git a/spec/models/address_spec.rb b/spec/models/address_spec.rb
index c4b84dc60..668f2d964 100644
--- a/spec/models/address_spec.rb
+++ b/spec/models/address_spec.rb
@@ -13,7 +13,7 @@ describe Address, '.extract_params' do
     ph = { postalInfo: { name: 'fred', addr: { cc: 'EE', city: 'Village', street: %w(street1 street2) } }  }
     expect(Address.extract_attributes(ph[:postalInfo])).to eq({
       address_attributes: {
-        country_id: 1,
+        country_id: Country.find_by(iso: 'EE').id,
         city: 'Village',
         street: 'street1',
         street2: 'street2'
diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb
index 23eeb70ea..e1db0f069 100644
--- a/spec/models/domain_spec.rb
+++ b/spec/models/domain_spec.rb
@@ -113,13 +113,12 @@ describe Domain do
 
     context 'when saved' do
       before(:each) do
-        Fabricate(:domain_validation_setting_group)
-        Fabricate(:dnskeys_setting_group)
+        # Fabricate(:domain_validation_setting_group)
+        # Fabricate(:dnskeys_setting_group)
         Fabricate(:domain)
       end
 
       it 'creates domain version' do
-
         expect(DomainVersion.count).to eq(1)
         expect(ContactVersion.count).to eq(2)
         expect(NameserverVersion.count).to eq(3)
diff --git a/spec/models/domain_version_spec.rb b/spec/models/domain_version_spec.rb
index 270f60a4c..eca98bdf9 100644
--- a/spec/models/domain_version_spec.rb
+++ b/spec/models/domain_version_spec.rb
@@ -2,9 +2,10 @@ require 'rails_helper'
 
 describe DomainVersion do
   with_versioning do
-    before(:each) { Fabricate(:domain_validation_setting_group); Fabricate(:dnskeys_setting_group) }
+    # before(:each) { Fabricate(:domain_validation_setting_group); Fabricate(:dnskeys_setting_group) }
     before(:each) do
-      Fabricate(:domain, name: 'version.ee') do
+      Setting.ns_min_count = 1
+      Fabricate(:domain, name: 'version.ee', dnskeys: []) do
         owner_contact { Fabricate(:contact, name: 'owner_contact', code: 'asd', email: 'owner1@v.ee') }
         nameservers(count: 1) { Fabricate(:nameserver, hostname: 'ns.test.ee') }
         admin_contacts(count: 1) { Fabricate(:contact, name: 'admin_contact 1', code: 'qwe', email: 'admin1@v.ee') }
@@ -58,7 +59,7 @@ describe DomainVersion do
       it 'nameserver creates a version' do
         expect(DomainVersion.count).to eq(1)
         expect(Domain.last.nameservers.count).to eq(1)
-        Domain.last.nameservers << Fabricate(:nameserver, hostname: 'ns.server.ee')
+        Domain.last.nameservers << Fabricate(:nameserver, hostname: 'ns.server.ee', created_at: Time.now - 20)
         expect(DomainVersion.count).to eq(2)
       end
     end
@@ -75,25 +76,10 @@ describe DomainVersion do
         expect(DomainVersion.count).to eq(2)
       end
 
-      it 'nameserver creates a version' do
-        Domain.last.nameservers.last.destroy
-        expect(DomainVersion.count).to eq(1)
-        expect(Domain.last.nameservers.count).to eq(1)
-        expect(DomainVersion.load_snapshot).to eq(
-          admin_contacts: [{ name: 'admin_contact 1', phone: '+372.12345678',
-                             code: 'qwe', ident: '37605030299', email: 'admin1@v.ee' }],
-          domain: { name: 'version.ee', status: nil },
-          nameservers: [{ hostname: 'ns.test.ee', ipv4: nil, ipv6: nil }],
-          owner_contact: { name: 'owner_contact', phone: '+372.12345678',
-                           code: 'asd', ident: '37605030299', email: 'owner1@v.ee' },
-          tech_contacts: [{ name: 'tech_contact 1', phone: '+372.12345678',
-                            code: 'zxc', ident: '37605030299', email: 'tech1@v.ee' }]
-        )
-      end
     end
 
-    context 'when deleting children' do
-      it 'creates a version' do
+    context 'when deleting child' do
+      it 'contact creates a version' do
         expect(DomainVersion.count).to eq(1)
         Contact.find_by(name: 'tech_contact 1').destroy
         expect(DomainVersion.count).to eq(2)
@@ -107,6 +93,24 @@ describe DomainVersion do
           tech_contacts: []
         })
       end
+
+      it 'nameserver creates a version' do
+        Domain.last.nameservers << Fabricate(:nameserver, created_at: Time.now - 30)
+        Domain.last.nameservers.last.destroy
+        expect(DomainVersion.count).to eq(3)
+        expect(Domain.last.nameservers.count).to eq(1)
+        expect(DomainVersion.last.load_snapshot).to eq(
+          admin_contacts: [{ name: 'admin_contact 1', phone: '+372.12345678',
+                             code: 'qwe', ident: '37605030299', email: 'admin1@v.ee' }],
+          domain: { name: 'version.ee', status: nil },
+          nameservers: [{ hostname: 'ns.test.ee', ipv4: nil, ipv6: nil }],
+          owner_contact: { name: 'owner_contact', phone: '+372.12345678',
+                           code: 'asd', ident: '37605030299', email: 'owner1@v.ee' },
+          tech_contacts: [{ name: 'tech_contact 1', phone: '+372.12345678',
+                            code: 'zxc', ident: '37605030299', email: 'tech1@v.ee' }]
+        )
+      end
+
     end
 
     context 'when editing children' do

From 7c3740acba1ce65ef1203177b07a8a7de56c5b2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= <andres@gitlab.eu>
Date: Mon, 20 Oct 2014 13:50:17 +0300
Subject: [PATCH 6/9] Fixed double versioning on domains

---
 app/models/contact.rb              |  4 ++--
 app/models/domain.rb               | 17 ++++++++++++++++-
 app/models/domain_contact.rb       |  6 +-----
 app/models/nameserver.rb           |  2 +-
 spec/models/domain_version_spec.rb |  1 -
 5 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/app/models/contact.rb b/app/models/contact.rb
index 7ebf25478..3169cc4b8 100644
--- a/app/models/contact.rb
+++ b/app/models/contact.rb
@@ -66,8 +66,8 @@ class Contact < ActiveRecord::Base
   def domains_snapshot
     (domains + domains_owned).uniq.each do |domain|
       next unless domain.is_a?(Domain)
-      next if domain.versions.last == domain.create_snapshot
-      domain.touch_with_version # Method from paper_trail
+      #next if domain.versions.last == domain.create_snapshot
+      domain.create_version # Method from paper_trail
     end
   end
 
diff --git a/app/models/domain.rb b/app/models/domain.rb
index dcfd56e86..2f0aa4ce2 100644
--- a/app/models/domain.rb
+++ b/app/models/domain.rb
@@ -61,10 +61,25 @@ class Domain < ActiveRecord::Base
   attr_accessor :owner_contact_typeahead, :update_me
 
   # archiving
-  has_paper_trail class_name: 'DomainVersion', meta: { snapshot: :create_snapshot }
+  has_paper_trail class_name: 'DomainVersion', meta: { snapshot: :create_snapshot }, if: Proc.new{ |t| t.new_version }
+
+  def new_version
+    #versions.try(:last).try(:snapshot) != create_snapshot
+    return false if versions.try(:last).try(:snapshot) == create_snapshot
+    true
+  end
+
+  def create_version
+    return true unless PaperTrail.enabled?
+    # We don't create a version unless domain is valid, is that a good idea?
+    return true unless valid?
+    #return true if versions.try(:last).try(:snapshot) == create_snapshot
+    touch_with_version
+  end
 
   def track_nameserver_add(nameserver)
     # if we are not adding nameservers on create ( we don't care about ms so to_i )
+    #return true if versions.try(:last).try(:snapshot) == create_snapshot
     touch_with_version if nameserver.created_at.to_i != created_at.to_i && valid?
   end
 
diff --git a/app/models/domain_contact.rb b/app/models/domain_contact.rb
index c334a58fc..584b3571b 100644
--- a/app/models/domain_contact.rb
+++ b/app/models/domain_contact.rb
@@ -39,12 +39,8 @@ class DomainContact < ActiveRecord::Base
   end
 
   def domain_snapshot
-    # We don't create a version unless domain is valid, is that a good idea?
-    return true unless PaperTrail.enabled?
     return true if domain.nil?
-    return true if contact.nil?
-    return true if domain.versions.last.try(:snapshot) == domain.try(:create_snapshot)
-    domain.touch_with_version if domain.valid?
+    domain.create_version
     true
   end
 end
diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb
index 9c01b3cf2..e22429b65 100644
--- a/app/models/nameserver.rb
+++ b/app/models/nameserver.rb
@@ -47,7 +47,7 @@ class Nameserver < ActiveRecord::Base
   end
 
   def domain_version
-    domain.touch_with_version if domain.valid?
+    domain.create_version
   end
 
   def to_s
diff --git a/spec/models/domain_version_spec.rb b/spec/models/domain_version_spec.rb
index eca98bdf9..898ff7ac9 100644
--- a/spec/models/domain_version_spec.rb
+++ b/spec/models/domain_version_spec.rb
@@ -2,7 +2,6 @@ require 'rails_helper'
 
 describe DomainVersion do
   with_versioning do
-    # before(:each) { Fabricate(:domain_validation_setting_group); Fabricate(:dnskeys_setting_group) }
     before(:each) do
       Setting.ns_min_count = 1
       Fabricate(:domain, name: 'version.ee', dnskeys: []) do

From cfdafe7aac5665064421e5294da6171b724c7276 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= <andres@gitlab.eu>
Date: Mon, 20 Oct 2014 16:40:04 +0300
Subject: [PATCH 7/9] Version improvements

---
 app/models/domain.rb     | 6 +++---
 app/models/nameserver.rb | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/models/domain.rb b/app/models/domain.rb
index 2f0aa4ce2..0a7545bd0 100644
--- a/app/models/domain.rb
+++ b/app/models/domain.rb
@@ -61,10 +61,10 @@ class Domain < ActiveRecord::Base
   attr_accessor :owner_contact_typeahead, :update_me
 
   # archiving
+  # if proc works only on changes on domain sadly
   has_paper_trail class_name: 'DomainVersion', meta: { snapshot: :create_snapshot }, if: Proc.new{ |t| t.new_version }
 
   def new_version
-    #versions.try(:last).try(:snapshot) != create_snapshot
     return false if versions.try(:last).try(:snapshot) == create_snapshot
     true
   end
@@ -74,13 +74,13 @@ class Domain < ActiveRecord::Base
     # We don't create a version unless domain is valid, is that a good idea?
     return true unless valid?
     #return true if versions.try(:last).try(:snapshot) == create_snapshot
-    touch_with_version
+    touch_with_version if new_version
   end
 
   def track_nameserver_add(nameserver)
     # if we are not adding nameservers on create ( we don't care about ms so to_i )
     #return true if versions.try(:last).try(:snapshot) == create_snapshot
-    touch_with_version if nameserver.created_at.to_i != created_at.to_i && valid?
+    touch_with_version if nameserver.created_at.to_i != created_at.to_i && valid? && new_version
   end
 
   def create_snapshot
diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb
index e22429b65..d350bd1c6 100644
--- a/app/models/nameserver.rb
+++ b/app/models/nameserver.rb
@@ -47,7 +47,7 @@ class Nameserver < ActiveRecord::Base
   end
 
   def domain_version
-    domain.create_version
+    domain.create_version if domain
   end
 
   def to_s

From 76e19b7e90553aa14da9341ea25714d20965b30f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= <andres@gitlab.eu>
Date: Wed, 22 Oct 2014 10:51:32 +0300
Subject: [PATCH 8/9] Cleaned up unnecessary comments

---
 app/controllers/client/domain_versions_controller.rb | 1 +
 app/models/domain.rb                                 | 9 ++++-----
 app/views/client/domain_versions/show.haml           | 5 +++--
 app/views/layouts/client.haml                        | 2 +-
 spec/models/domain_version_spec.rb                   | 2 --
 5 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/app/controllers/client/domain_versions_controller.rb b/app/controllers/client/domain_versions_controller.rb
index f716b0a6e..adf4a75b0 100644
--- a/app/controllers/client/domain_versions_controller.rb
+++ b/app/controllers/client/domain_versions_controller.rb
@@ -1,4 +1,5 @@
 class Client::DomainVersionsController < ClientController
+  helper WhodunnitHelper
   before_action :set_domain, only: [:show]
 
   def index
diff --git a/app/models/domain.rb b/app/models/domain.rb
index 0a7545bd0..cc3a3eaa7 100644
--- a/app/models/domain.rb
+++ b/app/models/domain.rb
@@ -71,16 +71,15 @@ class Domain < ActiveRecord::Base
 
   def create_version
     return true unless PaperTrail.enabled?
-    # We don't create a version unless domain is valid, is that a good idea?
     return true unless valid?
-    #return true if versions.try(:last).try(:snapshot) == create_snapshot
     touch_with_version if new_version
   end
 
   def track_nameserver_add(nameserver)
-    # if we are not adding nameservers on create ( we don't care about ms so to_i )
-    #return true if versions.try(:last).try(:snapshot) == create_snapshot
-    touch_with_version if nameserver.created_at.to_i != created_at.to_i && valid? && new_version
+    return true unless valid? && new_version
+    ns_created = nameserver.created_at.to_i
+    return true if created_at.to_i.between?( ns_created, ns_created + 1 )
+    touch_with_version
   end
 
   def create_snapshot
diff --git a/app/views/client/domain_versions/show.haml b/app/views/client/domain_versions/show.haml
index a355dbd5c..debc481e6 100644
--- a/app/views/client/domain_versions/show.haml
+++ b/app/views/client/domain_versions/show.haml
@@ -58,6 +58,7 @@
               %td
                 %p{ :style => 'font-size:x-small;' }
                   = l(version.created_at, format: :short)
-                  = version.whodunnit
-                  = version.event
+                  = whodunnit_with_protocol(version.whodunnit)
+                  =# version.whodunnit
+                  =# version.event
 
diff --git a/app/views/layouts/client.haml b/app/views/layouts/client.haml
index 5ea9ff1a2..5e220f78d 100644
--- a/app/views/layouts/client.haml
+++ b/app/views/layouts/client.haml
@@ -51,7 +51,7 @@
                 %li
                   = link_to t('shared.contact_list'), client_contacts_path
 
-            %li.dropdown
+            -# %li.dropdown
               %a.dropdown-toggle{"data-toggle" => "dropdown", href: "#"}
                 = t('shared.history')
                 %span.caret
diff --git a/spec/models/domain_version_spec.rb b/spec/models/domain_version_spec.rb
index 898ff7ac9..68fc2f189 100644
--- a/spec/models/domain_version_spec.rb
+++ b/spec/models/domain_version_spec.rb
@@ -65,10 +65,8 @@ describe DomainVersion do
 
     context 'when removing child' do
       it('has one domain version before events') { expect(DomainVersion.count).to eq(1) }
-      before(:each) { Domain.last.nameservers << Fabricate(:nameserver) }
 
       it 'contact creates a version' do
-        # FIXME: For some reason nameservers disappeared mid-test, but randomly stopped happening
         expect(DomainVersion.count).to eq(1)
         DomainContact.last.destroy
         expect(Domain.last.valid?).to be(true)

From f5e7f23ddf687afc5628ab730d8e71a8762ff878 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= <andres@gitlab.eu>
Date: Wed, 22 Oct 2014 12:36:32 +0300
Subject: [PATCH 9/9] Fixed tests

---
 app/models/contact.rb        |  2 +-
 app/models/domain.rb         |  8 ++++----
 app/models/domain_contact.rb |  1 +
 config/environments/test.rb  |  2 +-
 spec/epp/contact_spec.rb     | 22 ----------------------
 spec/models/address_spec.rb  |  3 +--
 6 files changed, 8 insertions(+), 30 deletions(-)

diff --git a/app/models/contact.rb b/app/models/contact.rb
index 3169cc4b8..88be4e65f 100644
--- a/app/models/contact.rb
+++ b/app/models/contact.rb
@@ -66,7 +66,7 @@ class Contact < ActiveRecord::Base
   def domains_snapshot
     (domains + domains_owned).uniq.each do |domain|
       next unless domain.is_a?(Domain)
-      #next if domain.versions.last == domain.create_snapshot
+      # next if domain.versions.last == domain.create_snapshot
       domain.create_version # Method from paper_trail
     end
   end
diff --git a/app/models/domain.rb b/app/models/domain.rb
index cc3a3eaa7..4e3f6a937 100644
--- a/app/models/domain.rb
+++ b/app/models/domain.rb
@@ -62,7 +62,7 @@ class Domain < ActiveRecord::Base
 
   # archiving
   # if proc works only on changes on domain sadly
-  has_paper_trail class_name: 'DomainVersion', meta: { snapshot: :create_snapshot }, if: Proc.new{ |t| t.new_version }
+  has_paper_trail class_name: 'DomainVersion', meta: { snapshot: :create_snapshot }, if: proc(&:new_version)
 
   def new_version
     return false if versions.try(:last).try(:snapshot) == create_snapshot
@@ -75,10 +75,10 @@ class Domain < ActiveRecord::Base
     touch_with_version if new_version
   end
 
-  def track_nameserver_add(nameserver)
+  def track_nameserver_add(_nameserver)
+    return true if versions.count == 0
     return true unless valid? && new_version
-    ns_created = nameserver.created_at.to_i
-    return true if created_at.to_i.between?( ns_created, ns_created + 1 )
+
     touch_with_version
   end
 
diff --git a/app/models/domain_contact.rb b/app/models/domain_contact.rb
index 584b3571b..82332c0dc 100644
--- a/app/models/domain_contact.rb
+++ b/app/models/domain_contact.rb
@@ -40,6 +40,7 @@ class DomainContact < ActiveRecord::Base
 
   def domain_snapshot
     return true if domain.nil?
+    return true if domain.versions.count == 0 # avoid snapshot on creation
     domain.create_version
     true
   end
diff --git a/config/environments/test.rb b/config/environments/test.rb
index f69df6cd3..dfc9384e2 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -41,6 +41,6 @@ Rails.application.configure do
   config.after_initialize do
     Bullet.enable = true
     Bullet.bullet_logger = true
-    Bullet.raise = false # raise an error if n+1 query occurs
+    Bullet.raise = true # raise an error if n+1 query occurs
   end
 end
diff --git a/spec/epp/contact_spec.rb b/spec/epp/contact_spec.rb
index 64fd34e78..5f91c3e04 100644
--- a/spec/epp/contact_spec.rb
+++ b/spec/epp/contact_spec.rb
@@ -80,18 +80,6 @@ describe 'EPP Contact', epp: true do
         expect(id.text.length).to eq(8)
         # 5 seconds for what-ever weird lag reasons might happen
         expect(cr_date.text.to_time).to be_within(5).of(Time.now)
-
-      end
-
-      it 'does not create duplicate contact', pending: true do
-        Fabricate(:contact, code: 'sh8013')
-
-        response = epp_request(contact_create_xml, :xml)
-
-        expect(response[:result_code]).to eq('2302')
-        expect(response[:msg]).to eq('Contact id already exists')
-
-        expect(Contact.count).to eq(1)
       end
     end
 
@@ -115,16 +103,6 @@ describe 'EPP Contact', epp: true do
         expect(response[:result_code]).to eq('2201')
       end
 
-      it 'stamps updated_by succesfully', pending: true do
-        Fabricate(:contact, code: 'sh8013', created_by_id: zone.id)
-
-        expect(Contact.first.updated_by_id).to be nil
-
-        epp_request(contact_update_xml, :xml)
-
-        expect(Contact.first.updated_by_id).to eq 2
-      end
-
       it 'is succesful' do
         Fabricate(
           :contact,
diff --git a/spec/models/address_spec.rb b/spec/models/address_spec.rb
index 668f2d964..5eb80854b 100644
--- a/spec/models/address_spec.rb
+++ b/spec/models/address_spec.rb
@@ -7,8 +7,7 @@ end
 
 describe Address, '.extract_params' do
 
-  # TODO: please fix
-  it 'returns params hash', pending: true do
+  it 'returns params hash' do
     Fabricate(:country, iso: 'EE')
     ph = { postalInfo: { name: 'fred', addr: { cc: 'EE', city: 'Village', street: %w(street1 street2) } }  }
     expect(Address.extract_attributes(ph[:postalInfo])).to eq({