Merge pull request #1094 from internetee/update-paper-trail-gem

Update paper trail gem

Gemfile

@@ -20,12 +20,7 @@ gem 'figaro', '1.1.1'
 gem 'pg',                        '0.19.0'
 gem 'ransack',                   '1.5.1' # for searching
 gem 'validates_email_format_of', '1.6.3' # validates email against RFC 2822 and RFC 3696
-
+gem 'paper_trail', '~> 4.0'
-# with polymorphic fix
-gem 'paper_trail',
-  github: 'airblade/paper_trail',
-  ref: 'a453811226ec4ea59753ba6b827e390ced2fc140'
-# NB! if this gets upgraded, ensure Setting.reload_settings! still works correctly
 gem 'rails-settings-cached',     '0.4.1' # for settings
 
 # html-xml

Gemfile.lock

@@ -1,13 +1,3 @@
-GIT
-  remote: https://github.com/airblade/paper_trail.git
-  revision: a453811226ec4ea59753ba6b827e390ced2fc140
-  ref: a453811226ec4ea59753ba6b827e390ced2fc140
-  specs:
-    paper_trail (4.0.0.beta3)
-      activerecord (>= 3.0, < 6.0)
-      activesupport (>= 3.0, < 6.0)
-      request_store (~> 1.1.0)
-
 GIT
   remote: https://github.com/internetee/company_register.git
   revision: da7130542304fc543c90d54cd037d019a777c526
@@ -266,6 +256,10 @@ GEM
     nori (2.6.0)
     open4 (1.3.4)
     orm_adapter (0.5.0)
+    paper_trail (4.2.0)
+      activerecord (>= 3.0, < 6.0)
+      activesupport (>= 3.0, < 6.0)
+      request_store (~> 1.1)
     pdfkit (0.6.2)
     pg (0.19.0)
     polyamorous (1.3.1)
@@ -323,7 +317,8 @@ GEM
       i18n
       polyamorous (~> 1.1)
     rdoc (4.3.0)
-    request_store (1.1.0)
+    request_store (1.4.1)
+      rack (>= 1.4)
     responders (2.4.1)
       actionpack (>= 4.2.0, < 6.0)
       railties (>= 4.2.0, < 6.0)
@@ -476,7 +471,7 @@ DEPENDENCIES
   mina (= 0.3.1)
   money-rails
   nokogiri
-  paper_trail!
+  paper_trail (~> 4.0)
   pdfkit (= 0.6.2)
   pg (= 0.19.0)
   pry (= 0.10.1)

db/migrate/20190302091059_restore_versions.rb

@@ -0,0 +1,15 @@
+class RestoreVersions < ActiveRecord::Migration
+  def change
+    drop_table :versions
+
+    create_table :versions do |t|
+      t.string :item_type, :null => false
+      t.integer :item_id, :null => false
+      t.string :event, :null => false
+      t.string :whodunnit
+      t.text :object
+      t.datetime :created_at
+    end
+    add_index :versions, [:item_type, :item_id]
+  end
+end

db/migrate/20190302111152_add_object_changes_to_versions.rb

@@ -0,0 +1,5 @@
+class AddObjectChangesToVersions < ActiveRecord::Migration
+  def change
+    add_column :versions, :object_changes, :jsonb
+  end
+end

db/structure.sql

@@ -2363,7 +2363,13 @@ ALTER SEQUENCE public.users_id_seq OWNED BY public.users.id;
 
 CREATE TABLE public.versions (
     id integer NOT NULL,
-    depricated_table_but_somehow_paper_trail_tests_fails_without_it text
+    item_type character varying NOT NULL,
+    item_id integer NOT NULL,
+    event character varying NOT NULL,
+    whodunnit character varying,
+    object text,
+    created_at timestamp without time zone,
+    object_changes jsonb
 );
 
 
@@ -3980,6 +3986,13 @@ CREATE INDEX index_users_on_identity_code ON public.users USING btree (identity_
 CREATE INDEX index_users_on_registrar_id ON public.users USING btree (registrar_id);
 
 
+--
+-- Name: index_versions_on_item_type_and_item_id; Type: INDEX; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE INDEX index_versions_on_item_type_and_item_id ON public.versions USING btree (item_type, item_id);
+
+
 --
 -- Name: index_whois_records_on_domain_id; Type: INDEX; Schema: public; Owner: -; Tablespace: 
 --
@@ -4927,6 +4940,10 @@ INSERT INTO schema_migrations (version) VALUES ('20190102144032');
 
 INSERT INTO schema_migrations (version) VALUES ('20190209150026');
 
+INSERT INTO schema_migrations (version) VALUES ('20190302091059');
+
+INSERT INTO schema_migrations (version) VALUES ('20190302111152');
+
 INSERT INTO schema_migrations (version) VALUES ('20190311111718');
 
 INSERT INTO schema_migrations (version) VALUES ('20190312211614');

lib/gem_ext/paper_trail.rb

@@ -1,10 +1,3 @@
-# the following line is required for PaperTrail >= 4.0.0 with Rails
-PaperTrail::Rails::Engine.eager_load!
-
-PaperTrail::Version.module_eval do
-  self.abstract_class = true
-end
-
 # Store console and rake changes in versions
 if defined?(::Rails::Console)
   PaperTrail.whodunnit = "console-#{`whoami`.strip}"

spec/models/account_spec.rb

@@ -1,14 +1,6 @@
 require 'rails_helper'
 
 RSpec.describe Account do
-  it 'has versions' do
-    with_versioning do
-      price = build(:account)
-      price.save!
-      expect(price.versions.size).to be(1)
-    end
-  end
-
   describe 'registrar validation', db: false do
     subject(:account) { described_class.new }
 

spec/models/admin_user_spec.rb

@@ -1,16 +1,6 @@
 require 'rails_helper'
 
 RSpec.describe AdminUser do
-  context 'with invalid attribute' do
-    before do
-      @admin_user = described_class.new
-    end
-
-    it 'should not have any versions' do
-      @admin_user.versions.should == []
-    end
-  end
-
   context 'with valid attributes' do
     before do
       @admin_user = create(:admin_user)

spec/models/api_user_spec.rb

@@ -17,10 +17,6 @@ RSpec.describe ApiUser do
       ])
     end
 
-    it 'should not have any versions' do
-      @api_user.versions.should == []
-    end
-
     it 'should be active by default' do
       @api_user.active.should == true
     end
@@ -41,16 +37,6 @@ RSpec.describe ApiUser do
       @api_user.valid?
       @api_user.errors.full_messages.should match_array([])
     end
-
-    it 'should have one version' do
-      with_versioning do
-        @api_user.versions.should == []
-        @api_user.username = 'newusername'
-        @api_user.save
-        @api_user.errors.full_messages.should match_array([])
-        @api_user.versions.size.should == 1
-      end
-    end
   end
 
   describe '::min_password_length', db: false do

spec/models/contact_spec.rb

@@ -5,16 +5,6 @@ RSpec.describe Contact do
     create(:zone, origin: 'ee')
   end
 
-  context 'about class' do
-    it 'should have versioning enabled?' do
-      Contact.paper_trail_enabled_for_model?.should == true
-    end
-
-    it 'should have custom log prexied table name for versions table' do
-      ContactVersion.table_name.should == 'log_contacts'
-    end
-  end
-
   context 'with invalid attribute' do
     before :example do
       @contact = Contact.new
@@ -28,10 +18,6 @@ RSpec.describe Contact do
       @contact.updator.should == nil
     end
 
-    it 'should not have any versions' do
-      @contact.versions.should == []
-    end
-
     it 'should not accept long code' do
       @contact.code = 'verylongcode' * 100
       @contact.valid?
@@ -66,16 +52,6 @@ RSpec.describe Contact do
       @contact = create(:contact)
     end
 
-    it 'should have one version' do
-      with_versioning do
-        @contact.versions.reload.should == []
-        @contact.name = 'New name'
-        @contact.save
-        @contact.errors.full_messages.should match_array([])
-        @contact.versions.size.should == 1
-      end
-    end
-
     it 'should not overwrite code' do
       old_code = @contact.code
       @contact.code = 'CID:REG1:should-not-overwrite-old-code-12345'

spec/models/dnskey_spec.rb

@@ -34,10 +34,6 @@ describe Dnskey do
       @dnskey.errors.full_messages.should match_array([
       ])
     end
-
-    it 'should not have any versions' do
-      @dnskey.versions.should == []
-    end
   end
 
   context 'with valid attributes' do
@@ -66,15 +62,6 @@ describe Dnskey do
       @dnskey.errors.full_messages.should match_array([])
     end
 
-    # TODO: figure out why not working
-    # it 'should have one version' do
-      # with_versioning do
-        # @dnskey.versions.should == []
-        # @dnskey.touch_with_version
-        # @dnskey.versions.size.should == 1
-      # end
-    # end
-
     it 'generates correct DS digest and DS key tag for ria.ee' do
       d = create(:domain, name: 'ria.ee', dnskeys: [@dnskey])
       dk = d.dnskeys.last

spec/models/domain_contact_spec.rb

@@ -49,18 +49,6 @@ describe DomainContact do
     it 'should have Tech name' do
       @domain_contact.name.should == 'Tech'
     end
-
-    it 'should have one version' do
-      @domain_contact = create(:domain_contact)
-
-      with_versioning do
-        @domain_contact.versions.reload.should == []
-        @domain_contact.contact = create(:contact)
-        @domain_contact.save!
-        @domain_contact.errors.full_messages.should match_array([])
-        @domain_contact.versions.size.should == 1
-      end
-    end
   end
 
   context 'with valid attributes with tech domain contact' do
@@ -82,18 +70,6 @@ describe DomainContact do
     it 'should have Tech name' do
       @domain_contact.name.should == 'Tech'
     end
-
-    it 'should have one version' do
-      @domain_contact = create(:domain_contact)
-
-      with_versioning do
-        @domain_contact.versions.reload.should == []
-        @domain_contact.contact = create(:contact)
-        @domain_contact.save!
-        @domain_contact.errors.full_messages.should match_array([])
-        @domain_contact.versions.size.should == 1
-      end
-    end
   end
 
   context 'with valid attributes with admin domain contact' do
@@ -115,15 +91,5 @@ describe DomainContact do
     it 'should have Tech name' do
       @domain_contact.name.should == 'Admin'
     end
-
-    it 'should have one version' do
-      with_versioning do
-        @domain_contact.versions.reload.should == []
-        @domain_contact.contact = create(:contact)
-        @domain_contact.save
-        @domain_contact.errors.full_messages.should match_array([])
-        @domain_contact.versions.size.should == 1
-      end
-    end
   end
 end

spec/models/domain_spec.rb

@@ -33,10 +33,6 @@ RSpec.describe Domain do
       @domain = Domain.new
     end
 
-    it 'should not have any versions' do
-      @domain.versions.should == []
-    end
-
     it 'should not have whois body' do
       @domain.whois_record.should == nil
     end
@@ -489,21 +485,6 @@ RSpec.describe Domain do
     expect(d.statuses.count).to eq(1)
     expect(d.statuses.first).to eq(DomainStatus::CLIENT_DELETE_PROHIBITED)
   end
-
-  with_versioning do
-    context 'when saved' do
-      before(:each) do
-        domain = create(:domain, nameservers_attributes: [attributes_for(:nameserver),
-                                                          attributes_for(:nameserver)])
-      end
-
-      it 'creates domain version' do
-        expect(DomainVersion.count).to eq(1)
-        expect(ContactVersion.count).to eq(3)
-        expect(NameserverVersion.count).to eq(2)
-      end
-    end
-  end
 end
 
 RSpec.describe Domain do

spec/models/keyrelay_spec.rb

@@ -41,10 +41,6 @@ describe Keyrelay do
         "Only one parameter allowed: relative or absolute"
       ])
     end
-
-    it 'should not have any versions' do
-      @keyrelay.versions.should == []
-    end
   end
 
   context 'with valid attributes' do
@@ -63,16 +59,6 @@ describe Keyrelay do
       @keyrelay.errors.full_messages.should match_array([])
     end
 
-    it 'should have one version' do
-      with_versioning do
-        @keyrelay.versions.should == []
-        @keyrelay.auth_info_pw = 'newpw'
-        @keyrelay.save
-        @keyrelay.errors.full_messages.should match_array([])
-        @keyrelay.versions.size.should == 1
-      end
-    end
-
     it 'is in pending status' do
       @keyrelay.status.should == 'pending'
     end

spec/models/white_ip_spec.rb

@@ -23,22 +23,6 @@ describe WhiteIp do
     end
   end
 
-  context 'with valid attributes' do
-    before :all do
-      @white_ip = create(:white_ip)
-    end
-
-    it 'should have one version' do
-      with_versioning do
-        @white_ip.versions.should == []
-        @white_ip.ipv4 = '192.168.1.2'
-        @white_ip.save
-        @white_ip.errors.full_messages.should match_array([])
-        @white_ip.versions.size.should == 1
-      end
-    end
-  end
-
   describe '#include_ip?' do
     context 'when given ip v4 exists' do
       before do

test/fixtures/log_contacts.yml

@@ -0,0 +1,5 @@
+one:
+  item_id: <%= ActiveRecord::FixtureSet.identify(:john) %>
+  item_type: Contact
+  event: update
+  created_at: <%= Time.zone.parse('2010-07-05') %>

test/fixtures/log_domains.yml

@@ -0,0 +1,8 @@
+one:
+  item_id: <%= ActiveRecord::FixtureSet.identify(:shop) %>
+  item_type: Domain
+  event: update
+  object:
+    registrant_id: <%= ActiveRecord::FixtureSet.identify(:john) %>
+    updated_at: <%= Time.zone.parse('2010-07-05') %>
+  created_at: <%= Time.zone.parse('2010-07-05') %>

test/integration/contact/audit_log_test.rb

@@ -0,0 +1,16 @@
+require 'test_helper'
+
+class ContactAuditLogTest < ActionDispatch::IntegrationTest
+  def test_stores_metadata
+    contact = contacts(:john)
+
+    contact.legal_document_id = 1
+    assert_difference 'contact.versions.count' do
+      contact.save!
+    end
+
+    contact_version = contact.versions.last
+    assert_equal ({ legal_documents: [1] }).with_indifferent_access,
+                 contact_version.children.with_indifferent_access
+  end
+end

test/integration/domain/audit_log_test.rb

@@ -0,0 +1,26 @@
+require 'test_helper'
+
+class DomainAuditLogTest < ActionDispatch::IntegrationTest
+  def test_stores_metadata
+    domain = domains(:shop)
+    assert_equal [contacts(:jane).id], domain.admin_contacts.ids
+    assert_equal [contacts(:william).id, contacts(:acme_ltd).id].sort, domain.tech_contacts.ids.sort
+    assert_equal [nameservers(:shop_ns1).id, nameservers(:shop_ns2).id].sort, domain.nameservers.ids
+                                                                                .sort
+    assert_equal contacts(:john).id, domain.registrant_id
+
+    domain.legal_document_id = 1
+    assert_difference 'domain.versions.count' do
+      domain.save!
+    end
+
+    domain_version = domain.versions.last
+    assert_equal ({ admin_contacts: [contacts(:jane).id],
+                    tech_contacts: [contacts(:william).id, contacts(:acme_ltd).id],
+                    nameservers: [nameservers(:shop_ns1).id, nameservers(:shop_ns2).id],
+                    dnskeys: [],
+                    legal_documents: [1],
+                    registrant: [contacts(:john).id] }).with_indifferent_access,
+                 domain_version.children.with_indifferent_access
+  end
+end

test/learning/paper_trail_test.rb

@@ -0,0 +1,46 @@
+require 'test_helper'
+
+class Post < ActiveRecord::Base
+  has_paper_trail
+end
+
+class PaperTrailLearningTest < ActiveSupport::TestCase
+  setup do
+    ActiveRecord::Base.connection.create_table :posts do |t|
+      t.string :title
+
+      # Otherwise `touch_with_version` fails silently
+      t.datetime :updated_at
+    end
+  end
+
+  def test_returns_version_list
+    @record = Post.create!(title: 'any')
+
+    assert_equal 1, @record.versions.count
+    assert_respond_to @record.versions.first, :item_id
+  end
+
+  def test_creates_new_version_upon_update
+    @record = Post.create!(title: 'old title')
+    original_record = @record.clone
+
+    assert_difference -> { @record.versions.size } do
+      @record.update!(title: 'new title')
+    end
+    version = @record.versions.last
+    assert_equal @record.id, version.item_id
+    assert_equal @record.class.name, version.item_type
+    assert_equal version.reify, original_record
+    assert_equal ['old title', 'new title'], version.object_changes['title']
+    assert_equal 'update', version.event
+  end
+
+  def test_touch_with_version
+    @record = Post.create!(title: 'any')
+    
+    assert_difference -> { @record.versions.size } do
+      @record.touch_with_version
+    end
+  end
+end