From 4cfa3d4b27e6ea127c5b97699e0a35d8e6ee036c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20Cosoroab=C4=83?= Date: Fri, 31 Mar 2017 01:08:14 +0300 Subject: [PATCH 1/5] added failing jsonb integration test (cherry picked from commit 7908cf75fc315dad870cfae2150323548f5d48da) --- .../db/migrate/20160415194001_create_posts.rb | 1 + spec/dummy/db/schema.rb | 1 + spec/integrations/triggers_spec.rb | 17 ++++++++++++++++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/spec/dummy/db/migrate/20160415194001_create_posts.rb b/spec/dummy/db/migrate/20160415194001_create_posts.rb index 818aa894..cb2eb326 100644 --- a/spec/dummy/db/migrate/20160415194001_create_posts.rb +++ b/spec/dummy/db/migrate/20160415194001_create_posts.rb @@ -4,6 +4,7 @@ def change t.string :title t.integer :rating t.boolean :active + t.jsonb :meta t.timestamps null: false end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index be023b27..06bbf1bc 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -38,6 +38,7 @@ t.string "title" t.integer "rating" t.boolean "active" + t.jsonb "meta" t.datetime "created_at", null: false t.datetime "updated_at", null: false end diff --git a/spec/integrations/triggers_spec.rb b/spec/integrations/triggers_spec.rb index 13922b81..c8003348 100644 --- a/spec/integrations/triggers_spec.rb +++ b/spec/integrations/triggers_spec.rb @@ -30,7 +30,7 @@ after(:all) { @old_post.destroy! } - let(:params) { { title: 'Triggers', rating: 10, active: false } } + let(:params) { { title: 'Triggers', rating: 10, active: false, meta: { tags: %w(some tag) } } } describe "backfill" do let(:post) { Post.find(@old_post.id) } @@ -58,6 +58,21 @@ end end + describe "diff" do + let(:post) { Post.create!(params).reload } + + it "generates the correct diff", :aggregate_failures do + post.update!(meta: { tags: ['other'] }) + diff = post.reload.log_data.diff_from version: (post.reload.log_version - 1) + expected_diff_meta = { + "old" => { "tags" => %w(some tag) }, + "new" => { "tags" => %w(other) } + } + expect(diff["meta"]["new"].class).to eq diff["meta"]["old"].class + expect(diff["meta"]).to eq expected_diff_meta + end + end + describe "update" do before(:all) { @post = Post.create! } after(:all) { @post.destroy! } From fa93e14029be7e11d364284c2e99472890821168 Mon Sep 17 00:00:00 2001 From: Vladimir Dementyev Date: Wed, 27 Dec 2017 14:36:41 +0300 Subject: [PATCH 2/5] Suppress migrations output in tests --- spec/acceptance_helper.rb | 9 +++++--- spec/support/acceptance_helpers.rb | 23 +++++++++++++++++-- .../shared_contexts/cleanup_files_contexts.rb | 8 ++++--- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/spec/acceptance_helper.rb b/spec/acceptance_helper.rb index 0befe2e7..dd58903f 100644 --- a/spec/acceptance_helper.rb +++ b/spec/acceptance_helper.rb @@ -13,9 +13,12 @@ config.after(:suite) do Dir.chdir("#{File.dirname(__FILE__)}/dummy") do ActiveRecord::Base.connection_pool.disconnect! - system <<-CMD - rake db:drop db:create db:migrate - CMD + + Logidze::AcceptanceHelpers.suppress_output do + system <<-CMD + rake db:drop db:create db:migrate + CMD + end end end end diff --git a/spec/support/acceptance_helpers.rb b/spec/support/acceptance_helpers.rb index a4395c7b..ce35725a 100644 --- a/spec/support/acceptance_helpers.rb +++ b/spec/support/acceptance_helpers.rb @@ -2,12 +2,12 @@ module Logidze module AcceptanceHelpers #:nodoc: def successfully(command) - expect(system("RAILS_ENV=test #{command}")) + expect(suppress_output { system("RAILS_ENV=test #{command}") }) .to eq(true), "'#{command}' was unsuccessful" end def unsuccessfully(command) - expect(system("RAILS_ENV=test #{command}")) + expect(suppress_output { system("RAILS_ENV=test #{command}") }) .to eq(false), "'#{command}' was successful" end @@ -20,5 +20,24 @@ def verify_file_not_contain(path, statement) expect(File.readlines(path).grep(/#{statement}/)) .to be_empty, "File #{path} should not contain '#{statement}'" end + + def suppress_output + return yield if ENV['LOG'].present? + retval = nil + begin + original_stderr = $stderr.clone + original_stdout = $stdout.clone + $stderr.reopen(IO::NULL) + $stdout.reopen(IO::NULL) + retval = yield + ensure + $stdout.reopen(original_stdout) + $stderr.reopen(original_stderr) + end + + retval + end + + extend self end end diff --git a/spec/support/shared_contexts/cleanup_files_contexts.rb b/spec/support/shared_contexts/cleanup_files_contexts.rb index 80125f97..0e3a78c6 100644 --- a/spec/support/shared_contexts/cleanup_files_contexts.rb +++ b/spec/support/shared_contexts/cleanup_files_contexts.rb @@ -7,9 +7,11 @@ version = path.match(%r{\/(\d+)\_[^\.]+\.rb$})[1] if all_versions.include?(version.to_i) Dir.chdir("#{File.dirname(__FILE__)}/../../dummy") do - system <<-CMD - VERSION=#{version} rake db:migrate:down - CMD + suppress_output do + system <<-CMD + VERSION=#{version} rake db:migrate:down + CMD + end end end FileUtils.rm(path) From 281199ec47f54ff4c70c6ab387c59a5a826d0c94 Mon Sep 17 00:00:00 2001 From: Vladimir Dementyev Date: Wed, 27 Dec 2017 15:00:16 +0300 Subject: [PATCH 3/5] Use keyword args for at/diff_from methods --- CHANGELOG.md | 9 ++++ README.md | 14 +++--- lib/logidze/history.rb | 4 +- lib/logidze/model.rb | 74 +++++++++++++++++++--------- lib/logidze/versioned_association.rb | 4 +- spec/integrations/triggers_spec.rb | 3 +- spec/logidze/model_spec.rb | 46 ++++++++--------- 7 files changed, 97 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eeb2e271..e3bff9da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## master +## 0.6.0 (2017-12-27) + +- Use positional arguments in `at`/`diff_from` methods and allow passing version. ([@palkan][]) + + Now you can write `post.diff_from(time: ts)`, `post.diff_from(version: x)`, Post.at(time: 1.day.ago)`, etc. + + NOTE: the previous behaviour is still supported (but gonna be deprecated), + i.e. you still can use `post.diff_from(ts)` if you don't mind the deprecation warning. + ## 0.5.3 (2017-08-22) - Add `--update` flag to model migration. ([@palkan][]) diff --git a/README.md b/README.md index 29d6968d..e924cdba 100644 --- a/README.md +++ b/README.md @@ -134,35 +134,35 @@ post.log_version #=> 3 post.log_size #=> 3 # Get copy of a record at a given time -old_post = post.at(2.days.ago) +old_post = post.at(time: 2.days.ago) # or revert the record itself to the previous state (without committing to DB) -post.at!('201-04-15 12:00:00') +post.at!(time: '201-04-15 12:00:00') # If no version found -post.at('1945-05-09 09:00:00') #=> nil +post.at(time: '1945-05-09 09:00:00') #=> nil ``` You can also get revision by version number: ```ruby -post.at_version(2) +post.at(version: 2) ``` It is also possible to get version for relations: ```ruby -Post.where(active: true).at(1.month.ago) +Post.where(active: true).at(time: 1.month.ago) ``` You can also get diff from specified time: ```ruby -post.diff_from(1.hour.ago) +post.diff_from(time: 1.hour.ago) #=> { "id" => 27, "changes" => { "title" => { "old" => "Logidze sucks!", "new" => "Logidze rulz!" } } } # the same for relations -Post.where(created_at: Time.zone.today.all_day).diff_from(1.hour.ago) +Post.where(created_at: Time.zone.today.all_day).diff_from(time: 1.hour.ago) ``` There are also `#undo!` and `#redo!` options (and more general `#switch_to!`): diff --git a/lib/logidze/history.rb b/lib/logidze/history.rb index be53e4f0..7bf4f5da 100644 --- a/lib/logidze/history.rb +++ b/lib/logidze/history.rb @@ -124,8 +124,8 @@ def as_json(options = {}) private def build_changes(a, b) - b.each_with_object({}) do |kv, acc| - acc[kv.first] = { "old" => a[kv.first], "new" => kv.last } unless kv.last == a[kv.first] + b.each_with_object({}) do |(k, v), acc| + acc[k] = { "old" => a[k], "new" => v } unless v == a[k] end end diff --git a/lib/logidze/model.rb b/lib/logidze/model.rb index f2a5e0e0..9701dd4a 100644 --- a/lib/logidze/model.rb +++ b/lib/logidze/model.rb @@ -2,6 +2,15 @@ require 'active_support' module Logidze + module Deprecations # :nodoc: + def self.show_ts_deprecation_for(meth) + warn( + "[Deprecation] Usage of #{meth}(time) will be removed in the future releases, "\ + "use #{meth}(time: ts) instead" + ) + end + end + # Extends model with methods to browse history module Model require 'logidze/history/type' if Rails::VERSION::MAJOR >= 5 @@ -20,13 +29,17 @@ module Model module ClassMethods # :nodoc: # Return records reverted to specified time - def at(ts) - all.map { |record| record.at(ts) }.compact + def at(ts = nil, time: nil, version: nil) + Deprecations.show_ts_deprecation_for(".at") if ts + time ||= ts + all.map { |record| record.at(time: time, version: version) }.compact end # Return changes made to records since specified time - def diff_from(ts) - all.map { |record| record.diff_from(ts) } + def diff_from(ts = nil, time: nil, version: nil) + Deprecations.show_ts_deprecation_for(".diff_from") if ts + time ||= ts + all.map { |record| record.diff_from(time: time, version: version) } end # Alias for Logidze.without_logging @@ -45,35 +58,46 @@ def has_logidze? attr_accessor :logidze_requested_ts # Return a dirty copy of record at specified time - # If time is less then the first version, then return nil. - # If time is greater then the last version, then return self. - def at(ts) - ts = parse_time(ts) + # If time/version is less then the first version, then return nil. + # If time/version is greater then the last version, then return self. + def at(ts = nil, time: nil, version: nil) + Deprecations.show_ts_deprecation_for("#at") if ts + + return at_version(version) if version + + time ||= ts + time = parse_time(time) - return nil unless log_data.exists_ts?(ts) + return nil unless log_data.exists_ts?(time) - if log_data.current_ts?(ts) - self.logidze_requested_ts = ts + if log_data.current_ts?(time) + self.logidze_requested_ts = time return self end - version = log_data.find_by_time(ts).version + version = log_data.find_by_time(time).version object_at = dup object_at.apply_diff(version, log_data.changes_to(version: version)) object_at.id = id - object_at.logidze_requested_ts = ts + object_at.logidze_requested_ts = time object_at end # Revert record to the version at specified time (without saving to DB) - def at!(ts) - ts = parse_time(ts) - return self if log_data.current_ts?(ts) - return false unless log_data.exists_ts?(ts) + def at!(ts = nil, time: nil, version: nil) + Deprecations.show_ts_deprecation_for("#at!") if ts - version = log_data.find_by_time(ts).version + return at_version!(version) if version + + time ||= ts + time = parse_time(time) + + return self if log_data.current_ts?(time) + return false unless log_data.exists_ts?(time) + + version = log_data.find_by_time(time).version apply_diff(version, log_data.changes_to(version: version)) end @@ -99,11 +123,17 @@ def at_version!(version) # # @example # - # post.diff_from(2.days.ago) + # post.diff_from(time: 2.days.ago) # or post.diff_from(version: 2) # #=> { "id" => 1, "changes" => { "title" => { "old" => "Hello!", "new" => "World" } } } - def diff_from(ts) - ts = parse_time(ts) - { "id" => id, "changes" => log_data.diff_from(time: ts) } + def diff_from(ts = nil, version: nil, time: nil) + Deprecations.show_ts_deprecation_for("#diff_from") if ts + time ||= ts + time = parse_time(time) if time + changes = log_data.diff_from(time: time, version: version).tap do |v| + deserialize_changes!(v) + end + + { "id" => id, "changes" => changes } end # Restore record to the previous version. diff --git a/lib/logidze/versioned_association.rb b/lib/logidze/versioned_association.rb index cfc5c88d..4f227c87 100644 --- a/lib/logidze/versioned_association.rb +++ b/lib/logidze/versioned_association.rb @@ -10,10 +10,10 @@ def load_target if target.is_a? Array target.map! do |object| - object.at(time) + object.at(time: time) end.compact! else - target.at!(time) + target.at!(time: time) end target diff --git a/spec/integrations/triggers_spec.rb b/spec/integrations/triggers_spec.rb index c8003348..6301f569 100644 --- a/spec/integrations/triggers_spec.rb +++ b/spec/integrations/triggers_spec.rb @@ -58,12 +58,13 @@ end end + # See https://github.com/palkan/logidze/pull/30 describe "diff" do let(:post) { Post.create!(params).reload } it "generates the correct diff", :aggregate_failures do post.update!(meta: { tags: ['other'] }) - diff = post.reload.log_data.diff_from version: (post.reload.log_version - 1) + diff = post.reload.diff_from(version: (post.reload.log_version - 1))["changes"] expected_diff_meta = { "old" => { "tags" => %w(some tag) }, "new" => { "tags" => %w(other) } diff --git a/spec/logidze/model_spec.rb b/spec/logidze/model_spec.rb index ae46f54c..1cff469c 100644 --- a/spec/logidze/model_spec.rb +++ b/spec/logidze/model_spec.rb @@ -21,24 +21,24 @@ ) end - describe "#at" do + describe "#at(time)" do it "returns version at specified time", :aggregate_failures do - user_old = user.at(time(350)) + user_old = user.at(time: time(350)) expect(user_old.name).to eq 'test' expect(user_old.age).to eq 0 expect(user_old.active).to eq true end it "returns nil if time is invalid (too early)" do - expect(user.at(time(99))).to be_nil + expect(user.at(time: time(99))).to be_nil end it "returns self if actual version" do - expect(user.at(time(401))).to be_equal(user) + expect(user.at(time: time(401))).to be_equal(user) end it "returns dup", :aggregate_failures do - user_old = user.at(time(100)) + user_old = user.at(time: time(100)) expect(user_old).not_to be_equal(user) user_old.age = 100 @@ -46,33 +46,33 @@ end it "retains original object's id" do - user_old = user.at(time(100)) + user_old = user.at(time: time(100)) expect(user_old.id).to be_equal(user.id) end it "handles time as string", :aggregate_failures do - user_old = user.at("2016-04-12 12:05:50") + user_old = user.at(time: "2016-04-12 12:05:50") expect(user_old.name).to eq 'test' expect(user_old.age).to eq 0 expect(user_old.active).to eq true end it "handles time as Time", :aggregate_failures do - user_old = user.at(Time.new(2016, 04, 12, 12, 05, 50)) + user_old = user.at(time: Time.new(2016, 04, 12, 12, 05, 50)) expect(user_old.name).to eq 'test' expect(user_old.age).to eq 0 expect(user_old.active).to eq true end it "handles time as Date", :aggregate_failures do - user_old = user.at(Date.new(2016, 04, 13)) + user_old = user.at(time: Date.new(2016, 04, 13)) expect(user_old).to be_equal user end end - describe "#at_version" do + describe "#at(version)" do it "returns specified version", :aggregate_failures do - user_old = user.at_version(4) + user_old = user.at(version: 4) expect(user_old.name).to eq 'test' expect(user_old.age).to eq 0 expect(user_old.active).to eq true @@ -81,7 +81,7 @@ describe "#at!" do it "update object in-place", :aggregate_failures do - user.at!(time(350)) + user.at!(time: time(350)) expect(user.name).to eq 'test' expect(user.age).to eq 0 @@ -93,7 +93,7 @@ describe "#diff_from" do it "returns diff from specified time" do - expect(user.diff_from(time(350))) + expect(user.diff_from(time: time(350))) .to eq( "id" => user.id, "changes" => @@ -212,7 +212,7 @@ before { user } it "returns reverted records", :aggregate_failures do - u = User.at(time(350)).first + u = User.at(time: time(350)).first expect(u.name).to eq 'test' expect(u.age).to eq 0 @@ -220,7 +220,7 @@ end it "returns reverted records when called on relation", :aggregate_failures do - u = User.where(active: false).order(age: :desc).at(time(350)).first + u = User.where(active: false).order(age: :desc).at(time: time(350)).first expect(u.name).to eq 'test' expect(u.age).to eq 0 @@ -238,7 +238,7 @@ } ) - expect(User.at(time(350)).size).to eq 1 + expect(User.at(time: time(350)).size).to eq 1 end end @@ -247,7 +247,7 @@ it "returns diffs for records", :aggregate_failures do expect( - User.diff_from(time(350)).first + User.diff_from(time: time(350)).first ).to eq( "id" => user.id, "changes" => @@ -260,7 +260,7 @@ it "returns diffs for records when called on relation", :aggregate_failures do expect( - User.where(active: false).order(age: :desc).diff_from(time(350)).first + User.where(active: false).order(age: :desc).diff_from(time: time(350)).first ).to eq( "id" => user.id, "changes" => @@ -278,11 +278,11 @@ end it "returns nil if no information" do - expect(user.at(time(350)).log_data.responsible_id).to be_nil + expect(user.at(time: time(350)).log_data.responsible_id).to be_nil end it "returns id for previous version" do - expect(user.at(time(250)).log_data.responsible_id).to eq 1 + expect(user.at(time: time(250)).log_data.responsible_id).to eq 1 end end @@ -348,8 +348,8 @@ article end - let(:old_article) { article.at(time(200)) } - let(:very_old_article) { article.at(time(100)) } + let(:old_article) { article.at(time: time(200)) } + let(:very_old_article) { article.at(time: time(100)) } context "when feature is disabled" do before(:all) { Logidze.associations_versioning = false } @@ -381,7 +381,7 @@ context 'when owner was not changed at the given time' do it "still returns association version" do # this returns the same article object due to implementation - not_changed_article = article.at(time(330)) + not_changed_article = article.at(time: time(330)) expect(not_changed_article.user.name).to eql('John Doe Jr.') end end From 83dd8ba7fd2b7ed9bc1f00bafb913af9db677695 Mon Sep 17 00:00:00 2001 From: Vladimir Dementyev Date: Wed, 27 Dec 2017 15:10:53 +0300 Subject: [PATCH 4/5] Support attributes types --- CHANGELOG.md | 4 ++ lib/logidze/model.rb | 26 +++++++++- .../db/migrate/20160415094739_create_users.rb | 2 + spec/dummy/db/schema.rb | 2 + spec/logidze/model_spec.rb | 49 +++++++++++++++++++ 5 files changed, 82 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3bff9da..3be8de43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ ## 0.6.0 (2017-12-27) +- [Fixes [#33](https://github.com/palkan/logidze/issues/33)] Support attributes types. ([@palkan][]) + + Added deserialization of complex types (such as `jsonb`, arrays, whatever). + - Use positional arguments in `at`/`diff_from` methods and allow passing version. ([@palkan][]) Now you can write `post.diff_from(time: ts)`, `post.diff_from(version: x)`, Post.at(time: 1.day.ago)`, etc. diff --git a/lib/logidze/model.rb b/lib/logidze/model.rb index 9701dd4a..c9f8e986 100644 --- a/lib/logidze/model.rb +++ b/lib/logidze/model.rb @@ -191,11 +191,35 @@ def association(name) protected def apply_diff(version, diff) - diff.each { |k, v| send("#{k}=", v) } + diff.each do |k, v| + apply_column_diff(k, v) + end + log_data.version = version self end + def apply_column_diff(column, value) + write_attribute column, deserialize_value(column, value) + end + + if Rails::VERSION::MAJOR < 5 + def deserialize_value(column, value) + @attributes[column].type.type_cast_from_database(value) + end + else + def deserialize_value(column, value) + @attributes[column].type.deserialize(value) + end + end + + def deserialize_changes!(diff) + diff.each do |k, v| + v["old"] = deserialize_value(k, v["old"]) + v["new"] = deserialize_value(k, v["new"]) + end + end + def logidze_past? return false unless @logidze_requested_ts diff --git a/spec/dummy/db/migrate/20160415094739_create_users.rb b/spec/dummy/db/migrate/20160415094739_create_users.rb index 074fb3b7..b216d65d 100644 --- a/spec/dummy/db/migrate/20160415094739_create_users.rb +++ b/spec/dummy/db/migrate/20160415094739_create_users.rb @@ -4,6 +4,8 @@ def change t.string :name t.integer :age t.boolean :active + t.jsonb :extra + t.string :settings, array: true t.jsonb :log_data t.timestamp :time end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 06bbf1bc..cdfc86cf 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -47,6 +47,8 @@ t.string "name" t.integer "age" t.boolean "active" + t.jsonb "extra" + t.string "settings", array: true t.jsonb "log_data" t.datetime "time" end diff --git a/spec/logidze/model_spec.rb b/spec/logidze/model_spec.rb index 1cff469c..39521504 100644 --- a/spec/logidze/model_spec.rb +++ b/spec/logidze/model_spec.rb @@ -434,4 +434,53 @@ end end end + + describe "Serialized types" do + let(:user) do + User.create!( + name: 'test', + extra: { gender: 'X', social: { fb: [1, 2], vk: false } }, + settings: [:sms, :mail], + log_data: { + 'v' => 5, + 'h' => + [ + { 'v' => 1, 'ts' => time(100), 'c' => { 'name' => nil, 'age' => nil, 'active' => nil, 'extra' => nil, 'settings' => nil } }, + { 'v' => 2, 'ts' => time(200), 'c' => { 'extra' => { 'gender' => 'M', 'social' => { 'fb' => [1] } }.to_json } }, + { 'v' => 3, 'ts' => time(200), 'r' => 1, 'c' => { 'settings' => '{mail}' } }, + { 'v' => 4, 'ts' => time(300), 'c' => { 'extra' => { 'gender' => 'F', 'social' => { 'fb' => [2] } }.to_json } }, + { 'v' => 5, 'ts' => time(400), 'r' => 2, 'c' => { 'extra' => { 'gender' => 'X', 'social' => { 'fb' => [1, 2], 'vk' => false } }.to_json, 'settings' => '{sms,mail}' } } + ] + } + ) + end + + describe "#at" do + it "returns version at specified time", :aggregate_failures do + user_old = user.at(time: time(350)) + expect(user_old.extra['gender']).to eq 'F' + expect(user_old.extra['social']).to eq('fb' => [2]) + expect(user_old.settings).to eq(['mail']) + + user_old = user.at(time: time(250)) + expect(user_old.extra['gender']).to eq 'M' + expect(user_old.extra['social']).to eq('fb' => [1]) + expect(user_old.settings).to eq(['mail']) + end + end + + describe "#diff_from" do + it "returns diff from specified time" do + expect(user.diff_from(version: 3)) + .to eq( + "id" => user.id, + "changes" => + { + "extra" => { "old" => { "gender" => "M", "social" => { "fb" => [1] } }, "new" => { "gender" => "X", "social" => { "fb" => [1, 2], "vk" => false } } }, + "settings" => { "old" => ['mail'], "new" => ['sms', 'mail'] } + } + ) + end + end + end end From d58260fec97b1d6a6a7ef3b99d8270456f1d9eba Mon Sep 17 00:00:00 2001 From: Vladimir Dementyev Date: Wed, 27 Dec 2017 20:24:03 +0300 Subject: [PATCH 5/5] Add RuboCop rake; fix styles --- .rubocop.yml | 25 ++++++++++++++++--- Gemfile | 1 + Rakefile | 3 +++ .../logidze/model/model_generator.rb | 2 +- lib/logidze/has_logidze.rb | 2 +- lib/logidze/model.rb | 7 ++++++ lib/logidze/versioned_association.rb | 7 +++--- logidze.gemspec | 11 ++++---- spec/logidze/model_spec.rb | 14 +++++------ spec/support/acceptance_helpers.rb | 3 ++- spec/support/test_helpers.rb | 2 +- 11 files changed, 54 insertions(+), 23 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 65f3e58b..78354a27 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -9,11 +9,14 @@ AllCops: - 'spec/dummy/**/*' - 'tmp/**/*' - 'bench/**/*' + - 'Rakefile' + - 'Gemfile' + - '*.gemspec' DisplayCopNames: true StyleGuideCopsOnly: false TargetRubyVersion: 2.3 -Style/AccessorMethodName: +Naming/AccessorMethodName: Enabled: false Style/TrivialAccessors: @@ -26,20 +29,30 @@ Style/Documentation: Style/StringLiterals: Enabled: false -Style/SpaceInsideStringInterpolation: +Layout/SpaceInsideStringInterpolation: EnforcedStyle: no_space Style/BlockDelimiters: Exclude: - 'spec/**/*.rb' +Style/PercentLiteralDelimiters: + Enabled: false + Lint/AmbiguousRegexpLiteral: Enabled: false +Lint/MissingCopEnableDirective: + Enabled: false + Metrics/MethodLength: Exclude: - 'spec/**/*.rb' +Metrics/BlockLength: + Exclude: + - 'spec/**/*.rb' + Metrics/LineLength: Max: 100 Exclude: @@ -59,5 +72,11 @@ Lint/HandleExceptions: Exclude: - 'spec/**/*.rb' -Style/DotPosition: +Layout/DotPosition: EnforcedStyle: leading + +Layout/IndentHeredoc: + Enabled: false + +Layout/EmptyLineAfterMagicComment: + Enabled: false \ No newline at end of file diff --git a/Gemfile b/Gemfile index 94af95aa..ae2c25f5 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,4 @@ +# frozen_string_literal: true source 'https://rubygems.org' # Specify your gem's dependencies in logidze.gemspec diff --git a/Rakefile b/Rakefile index 699e4633..7644930f 100644 --- a/Rakefile +++ b/Rakefile @@ -1,6 +1,9 @@ +# frozen_string_literal: true require "bundler/gem_tasks" require "rspec/core/rake_task" +require "rubocop/rake_task" +RuboCop::RakeTask.new RSpec::Core::RakeTask.new(:spec) namespace :dummy do diff --git a/lib/generators/logidze/model/model_generator.rb b/lib/generators/logidze/model/model_generator.rb index 1408898b..58564b3f 100644 --- a/lib/generators/logidze/model/model_generator.rb +++ b/lib/generators/logidze/model/model_generator.rb @@ -29,7 +29,7 @@ class ModelGenerator < ::ActiveRecord::Generators::Base # :nodoc: def generate_migration if options[:blacklist] && options[:whitelist] - $stderr.puts "Use only one: --whitelist or --blacklist" + warn "Use only one: --whitelist or --blacklist" exit(1) end migration_template "migration.rb.erb", "db/migrate/#{migration_file_name}" diff --git a/lib/logidze/has_logidze.rb b/lib/logidze/has_logidze.rb index e6c088f1..32258b03 100644 --- a/lib/logidze/has_logidze.rb +++ b/lib/logidze/has_logidze.rb @@ -9,7 +9,7 @@ module HasLogidze module ClassMethods # :nodoc: # Include methods to work with history. # - # rubocop:disable Style/PredicateName + # rubocop:disable Naming/PredicateName, Style/MixinUsage def has_logidze include Logidze::Model end diff --git a/lib/logidze/model.rb b/lib/logidze/model.rb index c9f8e986..0c2f29f9 100644 --- a/lib/logidze/model.rb +++ b/lib/logidze/model.rb @@ -12,6 +12,7 @@ def self.show_ts_deprecation_for(meth) end # Extends model with methods to browse history + # rubocop: disable Metrics/ModuleLength module Model require 'logidze/history/type' if Rails::VERSION::MAJOR >= 5 @@ -47,9 +48,11 @@ def without_logging(&block) Logidze.without_logging(&block) end + # rubocop: disable Naming/PredicateName def has_logidze? true end + # rubocop: enable Naming/PredicateName end # Use this to convert Ruby time to milliseconds @@ -60,6 +63,7 @@ def has_logidze? # Return a dirty copy of record at specified time # If time/version is less then the first version, then return nil. # If time/version is greater then the last version, then return self. + # rubocop: disable Metrics/AbcSize, Metrics/MethodLength def at(ts = nil, time: nil, version: nil) Deprecations.show_ts_deprecation_for("#at") if ts @@ -84,6 +88,7 @@ def at(ts = nil, time: nil, version: nil) object_at end + # rubocop: enable Metrics/AbcSize, Metrics/MethodLength # Revert record to the version at specified time (without saving to DB) def at!(ts = nil, time: nil, version: nil) @@ -165,6 +170,7 @@ def switch_to!(version, append: Logidze.append_on_undo) end end + # rubocop: disable Metrics/MethodLength def association(name) association = super @@ -187,6 +193,7 @@ def association(name) association end + # rubocop: enable Metrics/MethodLength protected diff --git a/lib/logidze/versioned_association.rb b/lib/logidze/versioned_association.rb index 4f227c87..42699941 100644 --- a/lib/logidze/versioned_association.rb +++ b/lib/logidze/versioned_association.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true -module Logidze +module Logidze # :nodoc: all module VersionedAssociation + # rubocop: disable Metrics/MethodLength, Metrics/AbcSize def load_target target = super @@ -26,9 +27,7 @@ def stale_target? def logidze_stale? return false if !loaded? || inversed - unless target.is_a?(Array) - return owner.logidze_requested_ts != target.logidze_requested_ts - end + return owner.logidze_requested_ts != target.logidze_requested_ts unless target.is_a?(Array) return false if target.empty? diff --git a/logidze.gemspec b/logidze.gemspec index 6b7633ad..09f35405 100644 --- a/logidze.gemspec +++ b/logidze.gemspec @@ -1,4 +1,4 @@ -# coding: utf-8 +# frozen_string_literal: true lib = File.expand_path('../lib', __FILE__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'logidze/version' @@ -19,13 +19,14 @@ Gem::Specification.new do |spec| spec.add_dependency "rails", ">= 4.2" - spec.add_development_dependency "pg", "~>0.18" + spec.add_development_dependency "ammeter", "~> 1.1.3" spec.add_development_dependency "bundler", "~> 1" + spec.add_development_dependency "database_cleaner", "~> 1.5" + spec.add_development_dependency "pg", "~>0.18" + spec.add_development_dependency "pry-byebug" spec.add_development_dependency "rake", ">= 10.0" spec.add_development_dependency "rspec-rails", ">= 3.4" - spec.add_development_dependency "database_cleaner", "~> 1.5" + spec.add_development_dependency "rubocop", "~> 0.52" spec.add_development_dependency "simplecov", ">= 0.3.8" - spec.add_development_dependency "ammeter", "~> 1.1.3" - spec.add_development_dependency "pry-byebug" spec.add_development_dependency "timecop", "~> 0.8" end diff --git a/spec/logidze/model_spec.rb b/spec/logidze/model_spec.rb index 39521504..21f7cd6b 100644 --- a/spec/logidze/model_spec.rb +++ b/spec/logidze/model_spec.rb @@ -301,7 +301,7 @@ { 'v' => 1, 'ts' => time(50), 'c' => { 'name' => 'John Harris', 'active' => false, 'age' => 45 } }, { 'v' => 2, 'ts' => time(150), 'c' => { 'active' => true, 'age' => 34 } }, { 'v' => 3, 'ts' => time(300), 'c' => { 'name' => 'John Doe Jr.', 'age' => 35 } }, - { 'v' => 4, 'ts' => time(350), 'c' => { 'name' => 'John Doe' } }, + { 'v' => 4, 'ts' => time(350), 'c' => { 'name' => 'John Doe' } } ] } ) @@ -319,7 +319,7 @@ [ { 'v' => 1, 'ts' => time(100), 'c' => { 'title' => 'Cool article', 'active' => false } }, { 'v' => 2, 'ts' => time(200), 'c' => { 'title' => 'Article' } }, - { 'v' => 3, 'ts' => time(300), 'c' => { 'rating' => 5, 'title' => 'Post' } }, + { 'v' => 3, 'ts' => time(300), 'c' => { 'rating' => 5, 'title' => 'Post' } } ] } ) @@ -330,7 +330,7 @@ 'h' => [ { 'v' => 1, 'ts' => time(150), 'c' => { 'content' => 'My comment' } }, - { 'v' => 2, 'ts' => time(250), 'c' => { 'content' => 'New comment' } }, + { 'v' => 2, 'ts' => time(250), 'c' => { 'content' => 'New comment' } } ] } ) @@ -340,7 +340,7 @@ 'v' => 1, 'h' => [ - { 'v' => 1, 'ts' => time(230), 'c' => { 'content' => 'New comment 2' } }, + { 'v' => 1, 'ts' => time(230), 'c' => { 'content' => 'New comment 2' } } ] } ) @@ -440,7 +440,7 @@ User.create!( name: 'test', extra: { gender: 'X', social: { fb: [1, 2], vk: false } }, - settings: [:sms, :mail], + settings: %i[sms mail], log_data: { 'v' => 5, 'h' => @@ -454,7 +454,7 @@ } ) end - + describe "#at" do it "returns version at specified time", :aggregate_failures do user_old = user.at(time: time(350)) @@ -477,7 +477,7 @@ "changes" => { "extra" => { "old" => { "gender" => "M", "social" => { "fb" => [1] } }, "new" => { "gender" => "X", "social" => { "fb" => [1, 2], "vk" => false } } }, - "settings" => { "old" => ['mail'], "new" => ['sms', 'mail'] } + "settings" => { "old" => ['mail'], "new" => %w[sms mail] } } ) end diff --git a/spec/support/acceptance_helpers.rb b/spec/support/acceptance_helpers.rb index ce35725a..5f8ca427 100644 --- a/spec/support/acceptance_helpers.rb +++ b/spec/support/acceptance_helpers.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Logidze module AcceptanceHelpers #:nodoc: def successfully(command) @@ -38,6 +39,6 @@ def suppress_output retval end - extend self + extend self # rubocop: disable all end end diff --git a/spec/support/test_helpers.rb b/spec/support/test_helpers.rb index ecbc0383..cca91235 100644 --- a/spec/support/test_helpers.rb +++ b/spec/support/test_helpers.rb @@ -13,7 +13,7 @@ def ignore_exceptions return unless block_given? begin yield - rescue + rescue StandardError end end end