From ab9610bfe0b44c81dd323c84505f94e7efddf815 Mon Sep 17 00:00:00 2001 From: Ion Drimba Filho Date: Thu, 8 Feb 2018 13:29:20 -0200 Subject: [PATCH 01/19] feat: add tables name config --- lib/rating.rb | 10 ++++++++++ lib/rating/config.rb | 19 +++++++++++++++++++ lib/rating/models/rating/rating.rb | 4 ++-- spec/config/initialize_spec.rb | 10 ++++++++++ spec/config/tables_spec.rb | 12 ++++++++++++ .../db/migrate/create_review_ratings_table.rb | 17 +++++++++++++++++ .../db/migrate/create_reviews_table.rb | 15 +++++++++++++++ spec/support/migrate.rb | 2 ++ spec/support/models/review.rb | 4 ++++ spec/support/models/review_rating.rb | 4 ++++ 10 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 lib/rating/config.rb create mode 100644 spec/config/initialize_spec.rb create mode 100644 spec/config/tables_spec.rb create mode 100644 spec/support/db/migrate/create_review_ratings_table.rb create mode 100644 spec/support/db/migrate/create_reviews_table.rb create mode 100644 spec/support/models/review.rb create mode 100644 spec/support/models/review_rating.rb diff --git a/lib/rating.rb b/lib/rating.rb index 68a56da..09b8a6c 100644 --- a/lib/rating.rb +++ b/lib/rating.rb @@ -1,8 +1,18 @@ # frozen_string_literal: true module Rating + class << self + def config + @config ||= Config.new + end + + def configure + yield config + end + end end +require 'rating/config' require 'rating/models/rating/extension' require 'rating/models/rating/rate' require 'rating/models/rating/rating' diff --git a/lib/rating/config.rb b/lib/rating/config.rb new file mode 100644 index 0000000..3622683 --- /dev/null +++ b/lib/rating/config.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Rating + class Config + attr_accessor :rate_model, :rating_model + + def models(rate: nil, rating: nil) + @rate_model = rate unless rate.nil? + @rating_model = rating unless rating.nil? + + self + end + + def initialize + @rate_model = ::Rating::Rate + @rating_model = ::Rating::Rating + end + end +end diff --git a/lib/rating/models/rating/rating.rb b/lib/rating/models/rating/rating.rb index d43c169..b8d400e 100644 --- a/lib/rating/models/rating/rating.rb +++ b/lib/rating/models/rating/rating.rb @@ -2,7 +2,7 @@ module Rating class Rating < ActiveRecord::Base - self.table_name = 'rating_ratings' + self.table_name = ::Rating.config.rating_model belongs_to :resource, polymorphic: true belongs_to :scopeable, polymorphic: true @@ -107,7 +107,7 @@ def how_many_resource_received_votes_sql?(distinct:, scopeable:) end def rate_table_name - @rate_table_name ||= Rate.table_name + @rate_table_name ||= ::Rating.config.rate_model end def scope_type_query(scopeable) diff --git a/spec/config/initialize_spec.rb b/spec/config/initialize_spec.rb new file mode 100644 index 0000000..6a395cf --- /dev/null +++ b/spec/config/initialize_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Rating::Config, 'initialize' do + it 'has default models' do + expect(subject.rate_model).to eq ::Rating::Rate + expect(subject.rating_model).to eq ::Rating::Rating + end +end diff --git a/spec/config/tables_spec.rb b/spec/config/tables_spec.rb new file mode 100644 index 0000000..94b5d61 --- /dev/null +++ b/spec/config/tables_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Rating::Config, 'models' do + it 'changes the rating models' do + subject.models rate: Review, rating: ReviewRating + + expect(subject.rate_model).to eq Review + expect(subject.rating_model).to eq ReviewRating + end +end diff --git a/spec/support/db/migrate/create_review_ratings_table.rb b/spec/support/db/migrate/create_review_ratings_table.rb new file mode 100644 index 0000000..a19efdd --- /dev/null +++ b/spec/support/db/migrate/create_review_ratings_table.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CreateReviewRatingsTable < ActiveRecord::Migration[5.0] + def change + create_table :review_ratings do |t| + t.decimal :average, default: 0, mull: false, precision: 25, scale: 16 + t.decimal :estimate, default: 0, mull: false, precision: 25, scale: 16 + t.integer :sum, default: 0, mull: false + t.integer :total, default: 0, mull: false + + t.references :resource, index: true, null: false, polymorphic: true + t.references :scopeable, index: true, null: true, polymorphic: true + + t.timestamps null: false + end + end +end diff --git a/spec/support/db/migrate/create_reviews_table.rb b/spec/support/db/migrate/create_reviews_table.rb new file mode 100644 index 0000000..c03d815 --- /dev/null +++ b/spec/support/db/migrate/create_reviews_table.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateReviewsTable < ActiveRecord::Migration[5.0] + def change + create_table :reviews do |t| + t.decimal :value, default: 0, precision: 25, scale: 16 + + t.references :author, index: true, null: false, polymorphic: true + t.references :resource, index: true, null: false, polymorphic: true + t.references :scopeable, index: true, null: true, polymorphic: true + + t.timestamps null: false + end + end +end diff --git a/spec/support/migrate.rb b/spec/support/migrate.rb index e37b496..eb1f84f 100644 --- a/spec/support/migrate.rb +++ b/spec/support/migrate.rb @@ -10,4 +10,6 @@ CreateCategoriesTable.new.change CreateRateTable.new.change CreateRatingTable.new.change +CreateReviewRatingsTable.new.change +CreateReviewsTable.new.change AddCommentOnRatingRatesTable.new.change diff --git a/spec/support/models/review.rb b/spec/support/models/review.rb new file mode 100644 index 0000000..d88fa65 --- /dev/null +++ b/spec/support/models/review.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class Review < ::ActiveRecord::Base +end diff --git a/spec/support/models/review_rating.rb b/spec/support/models/review_rating.rb new file mode 100644 index 0000000..84ba430 --- /dev/null +++ b/spec/support/models/review_rating.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class ReviewRating < ::ActiveRecord::Base +end From 1c3685959b66be706d144bd415bc983ce90ecfdb Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Thu, 8 Feb 2018 14:24:05 -0200 Subject: [PATCH 02/19] fix: namespace and using string for lazy evaluate --- lib/rating/config.rb | 4 ++-- lib/rating/models/rating/rate.rb | 3 ++- lib/rating/models/rating/rating.rb | 5 +++-- spec/config/initialize_spec.rb | 4 ++-- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/rating/config.rb b/lib/rating/config.rb index 3622683..73c5d9e 100644 --- a/lib/rating/config.rb +++ b/lib/rating/config.rb @@ -12,8 +12,8 @@ def models(rate: nil, rating: nil) end def initialize - @rate_model = ::Rating::Rate - @rating_model = ::Rating::Rating + @rate_model = '::Rating::Rate' + @rating_model = '::Rating::Rating' end end end diff --git a/lib/rating/models/rating/rate.rb b/lib/rating/models/rating/rate.rb index c78792f..2329ecb 100644 --- a/lib/rating/models/rating/rate.rb +++ b/lib/rating/models/rating/rate.rb @@ -2,7 +2,8 @@ module Rating class Rate < ActiveRecord::Base - self.table_name = 'rating_rates' + self.table_name_prefix = 'rating_' + self.table_name = ::Rating.config.rate_model.constantize.table_name after_save :update_rating diff --git a/lib/rating/models/rating/rating.rb b/lib/rating/models/rating/rating.rb index b8d400e..ab234da 100644 --- a/lib/rating/models/rating/rating.rb +++ b/lib/rating/models/rating/rating.rb @@ -2,7 +2,8 @@ module Rating class Rating < ActiveRecord::Base - self.table_name = ::Rating.config.rating_model + self.table_name_prefix = 'rating_' + self.table_name = ::Rating.config.rating_model.constantize.table_name belongs_to :resource, polymorphic: true belongs_to :scopeable, polymorphic: true @@ -107,7 +108,7 @@ def how_many_resource_received_votes_sql?(distinct:, scopeable:) end def rate_table_name - @rate_table_name ||= ::Rating.config.rate_model + @rate_table_name ||= ::Rating.config.rate_model.constantize.table_name end def scope_type_query(scopeable) diff --git a/spec/config/initialize_spec.rb b/spec/config/initialize_spec.rb index 6a395cf..520a932 100644 --- a/spec/config/initialize_spec.rb +++ b/spec/config/initialize_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Rating::Config, 'initialize' do it 'has default models' do - expect(subject.rate_model).to eq ::Rating::Rate - expect(subject.rating_model).to eq ::Rating::Rating + expect(subject.rate_model).to eq '::Rating::Rate' + expect(subject.rating_model).to eq '::Rating::Rating' end end From 3bab0f5899ef6be316c83c35ff2c15959817fac4 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Thu, 8 Feb 2018 17:15:48 -0200 Subject: [PATCH 03/19] spec: fix spec file name --- spec/config/{tables_spec.rb => models_spec.rb} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename spec/config/{tables_spec.rb => models_spec.rb} (85%) diff --git a/spec/config/tables_spec.rb b/spec/config/models_spec.rb similarity index 85% rename from spec/config/tables_spec.rb rename to spec/config/models_spec.rb index 94b5d61..e0fbcc6 100644 --- a/spec/config/tables_spec.rb +++ b/spec/config/models_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe Rating::Config, 'models' do +RSpec.describe Rating::Config, '.models' do it 'changes the rating models' do subject.models rate: Review, rating: ReviewRating From 008909edf39007fd8bed4d360137bdcf6f9b86ab Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Thu, 8 Feb 2018 17:15:52 -0200 Subject: [PATCH 04/19] lint --- spec/support/db/migrate/create_reviews_table.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/db/migrate/create_reviews_table.rb b/spec/support/db/migrate/create_reviews_table.rb index c03d815..d0c5d42 100644 --- a/spec/support/db/migrate/create_reviews_table.rb +++ b/spec/support/db/migrate/create_reviews_table.rb @@ -7,7 +7,7 @@ def change t.references :author, index: true, null: false, polymorphic: true t.references :resource, index: true, null: false, polymorphic: true - t.references :scopeable, index: true, null: true, polymorphic: true + t.references :scopeable, index: true, null: true, polymorphic: true t.timestamps null: false end From a5474cfd517b47a9f80e009241cb5d58e737a8f5 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Thu, 8 Feb 2018 17:16:03 -0200 Subject: [PATCH 05/19] up: table_name is already configured --- lib/rating/models/rating/rating.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rating/models/rating/rating.rb b/lib/rating/models/rating/rating.rb index ab234da..1a441f9 100644 --- a/lib/rating/models/rating/rating.rb +++ b/lib/rating/models/rating/rating.rb @@ -108,7 +108,7 @@ def how_many_resource_received_votes_sql?(distinct:, scopeable:) end def rate_table_name - @rate_table_name ||= ::Rating.config.rate_model.constantize.table_name + @rate_table_name ||= Rate.table_name end def scope_type_query(scopeable) From 1525a7b95f9eb396bed21cf5d496b82d5065e335 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Thu, 8 Feb 2018 17:16:23 -0200 Subject: [PATCH 06/19] spec: xit spec where we need to know how to do --- spec/config/configure_spec.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 spec/config/configure_spec.rb diff --git a/spec/config/configure_spec.rb b/spec/config/configure_spec.rb new file mode 100644 index 0000000..cafdd42 --- /dev/null +++ b/spec/config/configure_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Rating::Config, '.configure' do + before do + Rating.configure do |config| + config.models rate: 'Review', rating: 'ReviewRating' + end + end + + let!(:author) { create :author } + let!(:resource) { create :article } + + xit 'changes the rating models' do + author.rate resource, 5 + + expect(Review.count).to eq 1 + expect(ReviewRating.count).to eq 1 + end +end From e29e0c049e6899f153a24ba9e191447d8011645b Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Thu, 8 Feb 2018 18:37:13 -0200 Subject: [PATCH 07/19] fix: avoid error when has no rate record --- lib/rating/models/rating/rating.rb | 2 +- spec/models/rating/update_rating_spec.rb | 22 ++++++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/rating/models/rating/rating.rb b/lib/rating/models/rating/rating.rb index 1a441f9..a5c4dfa 100644 --- a/lib/rating/models/rating/rating.rb +++ b/lib/rating/models/rating/rating.rb @@ -101,7 +101,7 @@ def how_many_resource_received_votes_sql?(distinct:, scopeable:) count = distinct ? 'COUNT(DISTINCT resource_id)' : 'COUNT(1)' %(( - SELECT #{count} + SELECT GREATEST(#{count}, 1) FROM #{rate_table_name} WHERE resource_type = :resource_type AND #{scope_type_query(scopeable)} )) diff --git a/spec/models/rating/update_rating_spec.rb b/spec/models/rating/update_rating_spec.rb index d339580..1dc8992 100644 --- a/spec/models/rating/update_rating_spec.rb +++ b/spec/models/rating/update_rating_spec.rb @@ -4,9 +4,9 @@ require 'support/shared_context/with_database_records' RSpec.describe Rating::Rating, ':update_rating' do - include_context 'with_database_records' - context 'with no scopeable' do + include_context 'with_database_records' + it 'updates the rating data of the given resource' do record = described_class.find_by(resource: article_1) @@ -18,6 +18,8 @@ end context 'with scopeable' do + include_context 'with_database_records' + it 'updates the rating data of the given resource respecting the scope' do record = described_class.find_by(resource: article_1, scopeable: category) @@ -27,4 +29,20 @@ expect(record.total).to eq 2 end end + + context 'when rate table has no record' do + let!(:resource) { create :article } + let!(:scope) { nil } + + it 'calculates with counts values as zero' do + described_class.update_rating resource, scope + + record = described_class.last + + expect(record.average).to eq 0 + expect(record.estimate).to eq 0 + expect(record.sum).to eq 0 + expect(record.total).to eq 0 + end + end end From 726c6ae5ac9126833e8c31b332708e8e68a6f2de Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Thu, 8 Feb 2018 19:50:11 -0200 Subject: [PATCH 08/19] lint --- lib/generators/rating/install_generator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/generators/rating/install_generator.rb b/lib/generators/rating/install_generator.rb index e5b3fa3..c810816 100644 --- a/lib/generators/rating/install_generator.rb +++ b/lib/generators/rating/install_generator.rb @@ -4,7 +4,7 @@ module Rating class InstallGenerator < Rails::Generators::Base source_root File.expand_path('../templates', __FILE__) - desc 'creates Rating migration' + desc 'Creates Rating migration' def create_migration template 'db/migrate/create_rating_table.rb', "db/migrate/#{timestamp(0)}_create_rating_table.rb" From 296669ea6f785b8daa79282b071832be03627582 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Thu, 8 Feb 2018 19:50:48 -0200 Subject: [PATCH 09/19] up: adds initializer template --- lib/generators/rating/install_generator.rb | 6 ++++++ .../rating/templates/config/initializers/rating.rb | 3 +++ 2 files changed, 9 insertions(+) create mode 100644 lib/generators/rating/templates/config/initializers/rating.rb diff --git a/lib/generators/rating/install_generator.rb b/lib/generators/rating/install_generator.rb index c810816..0dd8853 100644 --- a/lib/generators/rating/install_generator.rb +++ b/lib/generators/rating/install_generator.rb @@ -4,6 +4,12 @@ module Rating class InstallGenerator < Rails::Generators::Base source_root File.expand_path('../templates', __FILE__) + desc 'Creates Rating initializer' + + def copy_initializer + copy_file 'config/initializers/rating.rb', 'config/initializers/rating.rb' + end + desc 'Creates Rating migration' def create_migration diff --git a/lib/generators/rating/templates/config/initializers/rating.rb b/lib/generators/rating/templates/config/initializers/rating.rb new file mode 100644 index 0000000..fd789c5 --- /dev/null +++ b/lib/generators/rating/templates/config/initializers/rating.rb @@ -0,0 +1,3 @@ +# Rating.configure do |config| +# config.models rate: '::Rating::Rate', rating: '::Rating::Rating' +# end From 4c7d957c228b1c7ecd066f12ec3b0ff4d5b3fee7 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Thu, 8 Feb 2018 20:44:39 -0200 Subject: [PATCH 10/19] feat: scoping support --- lib/rating/models/rating/extension.rb | 16 ++- spec/factories/comment.rb | 5 + spec/models/extension/after_create_spec.rb | 28 ----- spec/models/extension/after_save_spec.rb | 37 ++++++ spec/models/extension/rating_warm_up_spec.rb | 115 ++++++++++++++++++ .../db/migrate/create_categories_table.rb | 2 + .../db/migrate/create_comments_table.rb | 7 ++ spec/support/migrate.rb | 1 + spec/support/models/article.rb | 4 +- spec/support/models/category.rb | 1 + spec/support/models/comment.rb | 5 + 11 files changed, 190 insertions(+), 31 deletions(-) create mode 100644 spec/factories/comment.rb delete mode 100644 spec/models/extension/after_create_spec.rb create mode 100644 spec/models/extension/after_save_spec.rb create mode 100644 spec/models/extension/rating_warm_up_spec.rb create mode 100644 spec/support/db/migrate/create_comments_table.rb create mode 100644 spec/support/models/comment.rb diff --git a/lib/rating/models/rating/extension.rb b/lib/rating/models/rating/extension.rb index a8acca7..c25d6aa 100644 --- a/lib/rating/models/rating/extension.rb +++ b/lib/rating/models/rating/extension.rb @@ -28,11 +28,23 @@ def rated(scope: nil) def rating(scope: nil) rating_records.find_by scopeable: scope end + + def rating_warm_up(scoping: nil) + return Rating.find_or_create_by(resource: self) if scoping.blank? + + [scoping].flatten.compact.map do |attribute| + next unless respond_to?(attribute) + + [public_send(attribute)].flatten.compact.map do |object| + Rating.find_or_create_by! resource: self, scopeable: object + end + end.flatten.compact + end end module ClassMethods - def rating(as: nil) - after_create -> { Rating.find_or_create_by resource: self }, unless: -> { as == :author } + def rating(as: nil, scoping: nil) + after_save -> { rating_warm_up scoping: scoping }, unless: -> { as == :author } has_many :rating_records, as: :resource, diff --git a/spec/factories/comment.rb b/spec/factories/comment.rb new file mode 100644 index 0000000..257d3f6 --- /dev/null +++ b/spec/factories/comment.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :comment +end diff --git a/spec/models/extension/after_create_spec.rb b/spec/models/extension/after_create_spec.rb deleted file mode 100644 index cb68339..0000000 --- a/spec/models/extension/after_create_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Rating::Extension, ':after_create' do - context 'when :as is nil' do - let!(:article) { create :article } - - it 'creates a rating record with zero values just to be easy to make the count' do - rating = Rating::Rating.find_by(resource: article) - - expect(rating.average).to eq 0 - expect(rating.estimate).to eq 0 - expect(rating.resource).to eq article - expect(rating.scopeable).to eq nil - expect(rating.sum).to eq 0 - expect(rating.total).to eq 0 - end - end - - context 'when :as is :author' do - let!(:author) { create :author } - - it 'does not creates a rating record' do - expect(Rating::Rating.exists?(resource: author)).to eq false - end - end -end diff --git a/spec/models/extension/after_save_spec.rb b/spec/models/extension/after_save_spec.rb new file mode 100644 index 0000000..daa5a89 --- /dev/null +++ b/spec/models/extension/after_save_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Rating::Extension, 'after_save' do + context 'when record is author' do + let!(:record) { build :author } + + it 'does warm up the cache' do + expect(record).not_to receive(:rating_warm_up) + + record.save + end + end + + context 'when record is not author' do + context 'when record has scoping' do + let!(:record) { build :article } + + it 'warms up the cache' do + expect(record).to receive(:rating_warm_up).with(scoping: :categories) + + record.save + end + end + + context 'when record has no scoping' do + let!(:record) { build :comment } + + it 'warms up the cache' do + expect(record).to receive(:rating_warm_up).with(scoping: nil) + + record.save + end + end + end +end diff --git a/spec/models/extension/rating_warm_up_spec.rb b/spec/models/extension/rating_warm_up_spec.rb new file mode 100644 index 0000000..3040000 --- /dev/null +++ b/spec/models/extension/rating_warm_up_spec.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Rating::Extension, '.rating_warm_up' do + context 'when scoping is nil' do + context 'when update is made' do + let!(:record) { create :comment } + let!(:rating) { ::Rating::Rating.find_by resource: record } + + it 'creates the cache' do + record.rating_warm_up scoping: nil + + expect(::Rating::Rating.all).to eq [rating] + end + + it 'returns the cached object' do + expect(record.rating_warm_up).to eq rating + end + end + + context 'when record does not exist' do + let!(:record) { create :comment } + + before { ::Rating::Rating.destroy_all } + + it 'creates the cache' do + record.rating_warm_up scoping: nil + + expect(::Rating::Rating.all.map(&:resource)).to eq [record] + end + + it 'returns the cached object' do + expect(record.rating_warm_up).to eq ::Rating::Rating.last + end + end + end + + context 'when scoping is not nil' do + context 'when update is made' do + let!(:category_1) { create :category } + let!(:category_2) { create :category } + let!(:record) { create :article, categories: [category_1, category_2] } + + it 'creates the cache' do + record.rating_warm_up scoping: :categories + + ratings = ::Rating::Rating.all + + expect(ratings.map(&:scopeable)).to match_array [category_1, category_2] + expect(ratings.map(&:resource)).to match_array [record, record] + end + + it 'returns the cached objects' do + expect(record.rating_warm_up(scoping: :categories)).to eq ::Rating::Rating.all + end + end + + context 'when record does not exist' do + let!(:category_1) { create :category } + let!(:category_2) { create :category } + let!(:record) { create :article, categories: [category_1, category_2] } + + before { ::Rating::Rating.destroy_all } + + it 'creates the cache' do + record.rating_warm_up scoping: :categories + + ratings = ::Rating::Rating.all + + expect(ratings.map(&:scopeable)).to match_array [category_1, category_2] + expect(ratings.map(&:resource)).to match_array [record, record] + end + + it 'returns the cached objects' do + expect(record.rating_warm_up(scoping: :categories)).to eq ::Rating::Rating.all + end + end + + context 'when a non existent scoping is given' do + let!(:record) { create :article } + + it 'returns an empty array' do + expect(record.rating_warm_up(scoping: :missing)).to eq [] + end + end + + context 'when scoping is given inside array' do + let!(:category) { create :category } + let!(:record) { create :article, categories: [category] } + + it 'returns the cache' do + expect(record.rating_warm_up(scoping: [:categories])).to eq ::Rating::Rating.all + end + end + + context 'when scoping is given inside multiple arrays' do + let!(:category) { create :category } + let!(:record) { create :article, categories: [category] } + + it 'returns the cache' do + expect(record.rating_warm_up(scoping: [[:categories]])).to eq ::Rating::Rating.all + end + end + + context 'when scoping is given with nil value together' do + let!(:category) { create :category } + let!(:record) { create :article, categories: [category] } + + it 'returns the cache' do + expect(record.rating_warm_up(scoping: [[:categories, nil], nil])).to eq ::Rating::Rating.all + end + end + end +end diff --git a/spec/support/db/migrate/create_categories_table.rb b/spec/support/db/migrate/create_categories_table.rb index f76519a..5b6760c 100644 --- a/spec/support/db/migrate/create_categories_table.rb +++ b/spec/support/db/migrate/create_categories_table.rb @@ -4,6 +4,8 @@ class CreateCategoriesTable < ActiveRecord::Migration[5.0] def change create_table :categories do |t| t.string :name, null: false + + t.references :article, foreign_key: true, index: true end end end diff --git a/spec/support/db/migrate/create_comments_table.rb b/spec/support/db/migrate/create_comments_table.rb new file mode 100644 index 0000000..c333099 --- /dev/null +++ b/spec/support/db/migrate/create_comments_table.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class CreateCommentsTable < ActiveRecord::Migration[5.0] + def change + create_table :comments + end +end diff --git a/spec/support/migrate.rb b/spec/support/migrate.rb index eb1f84f..1a24fbd 100644 --- a/spec/support/migrate.rb +++ b/spec/support/migrate.rb @@ -8,6 +8,7 @@ CreateArticlesTable.new.change CreateAuthorsTable.new.change CreateCategoriesTable.new.change +CreateCommentsTable.new.change CreateRateTable.new.change CreateRatingTable.new.change CreateReviewRatingsTable.new.change diff --git a/spec/support/models/article.rb b/spec/support/models/article.rb index 85e8d8c..6485453 100644 --- a/spec/support/models/article.rb +++ b/spec/support/models/article.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true class Article < ::ActiveRecord::Base - rating + rating scoping: :categories + + has_many :categories end diff --git a/spec/support/models/category.rb b/spec/support/models/category.rb index cff22d3..8230753 100644 --- a/spec/support/models/category.rb +++ b/spec/support/models/category.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true class Category < ::ActiveRecord::Base + belongs_to :article end diff --git a/spec/support/models/comment.rb b/spec/support/models/comment.rb new file mode 100644 index 0000000..2b2e650 --- /dev/null +++ b/spec/support/models/comment.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class Comment < ::ActiveRecord::Base + rating +end From 6bfea476ed2599f325c5a74611302fbc8f3f87ff Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Fri, 9 Feb 2018 16:48:41 -0200 Subject: [PATCH 11/19] gem: update --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 8e85f3f..941d724 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -34,7 +34,7 @@ GEM arel (8.0.0) ast (2.4.0) builder (3.2.3) - byebug (9.1.0) + byebug (10.0.0) coderay (1.1.2) concurrent-ruby (1.0.5) crass (1.0.3) @@ -46,7 +46,7 @@ GEM factory_bot_rails (4.8.2) factory_bot (~> 4.8.2) railties (>= 3.0.0) - i18n (0.9.3) + i18n (0.9.4) concurrent-ruby (~> 1.0) loofah (2.1.1) crass (~> 1.0.2) @@ -64,8 +64,8 @@ GEM pry (0.11.3) coderay (~> 1.1.0) method_source (~> 0.9.0) - pry-byebug (3.5.1) - byebug (~> 9.1) + pry-byebug (3.6.0) + byebug (~> 10.0) pry (~> 0.10) rack (2.0.4) rack-test (0.8.2) From 148f12f8f9ce68796b25d14d2382f21b7a420328 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Fri, 9 Feb 2018 16:49:25 -0200 Subject: [PATCH 12/19] fix: avoid warm up on update like before it will avoid a bunch of unecessary updates --- lib/rating/models/rating/extension.rb | 2 +- .../{after_save_spec.rb => after_create_spec.rb} | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) rename spec/models/extension/{after_save_spec.rb => after_create_spec.rb} (70%) diff --git a/lib/rating/models/rating/extension.rb b/lib/rating/models/rating/extension.rb index c25d6aa..e4365ca 100644 --- a/lib/rating/models/rating/extension.rb +++ b/lib/rating/models/rating/extension.rb @@ -44,7 +44,7 @@ def rating_warm_up(scoping: nil) module ClassMethods def rating(as: nil, scoping: nil) - after_save -> { rating_warm_up scoping: scoping }, unless: -> { as == :author } + after_create -> { rating_warm_up scoping: scoping }, unless: -> { as == :author } has_many :rating_records, as: :resource, diff --git a/spec/models/extension/after_save_spec.rb b/spec/models/extension/after_create_spec.rb similarity index 70% rename from spec/models/extension/after_save_spec.rb rename to spec/models/extension/after_create_spec.rb index daa5a89..cf14e90 100644 --- a/spec/models/extension/after_save_spec.rb +++ b/spec/models/extension/after_create_spec.rb @@ -2,11 +2,11 @@ require 'rails_helper' -RSpec.describe Rating::Extension, 'after_save' do +RSpec.describe Rating::Extension, 'after_create' do context 'when record is author' do let!(:record) { build :author } - it 'does warm up the cache' do + it 'does not warm up the cache' do expect(record).not_to receive(:rating_warm_up) record.save @@ -33,5 +33,15 @@ record.save end end + + context 'when update is made' do + let!(:record) { create :comment } + + it 'does not warm up the cache' do + expect(record).not_to receive(:rating_warm_up) + + record.save + end + end end end From e9e08564e7a95824ea2b883861a9055a409130cb Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Fri, 9 Feb 2018 16:53:00 -0200 Subject: [PATCH 13/19] up: using config to build ar relations --- lib/rating/models/rating/extension.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rating/models/rating/extension.rb b/lib/rating/models/rating/extension.rb index e4365ca..e44fb82 100644 --- a/lib/rating/models/rating/extension.rb +++ b/lib/rating/models/rating/extension.rb @@ -48,17 +48,17 @@ def rating(as: nil, scoping: nil) has_many :rating_records, as: :resource, - class_name: '::Rating::Rating', + class_name: ::Rating.config.rating_model, dependent: :destroy has_many :rates_records, as: :resource, - class_name: '::Rating::Rate', + class_name: ::Rating.config.rate_model, dependent: :destroy has_many :rated_records, as: :author, - class_name: '::Rating::Rate', + class_name: ::Rating.config.rate_model, dependent: :destroy scope :order_by_rating, ->(column = :estimate, direction = :desc, scope: nil) { From b7ffd3dfc2fa5498252cc0e78717adc1a1a9c6c3 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Wed, 14 Feb 2018 10:44:04 -0200 Subject: [PATCH 14/19] gem: update --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 941d724..70c6e67 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -46,9 +46,9 @@ GEM factory_bot_rails (4.8.2) factory_bot (~> 4.8.2) railties (>= 3.0.0) - i18n (0.9.4) + i18n (0.9.5) concurrent-ruby (~> 1.0) - loofah (2.1.1) + loofah (2.2.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) method_source (0.9.0) From e2aaa8359801efdd229e079886835953585e93a7 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Wed, 14 Feb 2018 14:43:13 -0200 Subject: [PATCH 15/19] feat: makes table name configurable via yml --- .gitignore | 1 + lib/rating/config.rb | 20 +++++++++++------- lib/rating/models/rating/extension.rb | 6 +++--- lib/rating/models/rating/rate.rb | 2 +- lib/rating/models/rating/rating.rb | 2 +- spec/config/configure_spec.rb | 21 ------------------- spec/config/initialize_spec.rb | 10 --------- spec/config/models_spec.rb | 12 ----------- spec/config/rate_table_spec.rb | 13 ++++++++++++ spec/config/rating_table_spec.rb | 13 ++++++++++++ .../add_comment_on_rating_rates_table.rb | 1 + spec/support/models/review.rb | 1 + 12 files changed, 46 insertions(+), 56 deletions(-) delete mode 100644 spec/config/configure_spec.rb delete mode 100644 spec/config/initialize_spec.rb delete mode 100644 spec/config/models_spec.rb create mode 100644 spec/config/rate_table_spec.rb create mode 100644 spec/config/rating_table_spec.rb diff --git a/.gitignore b/.gitignore index 7e1052c..148fc15 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ .byebug_history *.gem +/config diff --git a/lib/rating/config.rb b/lib/rating/config.rb index 73c5d9e..172b793 100644 --- a/lib/rating/config.rb +++ b/lib/rating/config.rb @@ -2,18 +2,22 @@ module Rating class Config - attr_accessor :rate_model, :rating_model + def config + @config ||= begin + file_path = File.expand_path('config/rating.yml') - def models(rate: nil, rating: nil) - @rate_model = rate unless rate.nil? - @rating_model = rating unless rating.nil? + return {} unless File.exist?(file_path) - self + YAML.safe_load(File.read(file_path))['rating'] + end end - def initialize - @rate_model = '::Rating::Rate' - @rating_model = '::Rating::Rating' + def rate_table + @rate_table ||= config[__method__.to_s] || 'rating_rates' + end + + def rating_table + @rating_table ||= config[__method__.to_s] || 'rating_ratings' end end end diff --git a/lib/rating/models/rating/extension.rb b/lib/rating/models/rating/extension.rb index e44fb82..e4365ca 100644 --- a/lib/rating/models/rating/extension.rb +++ b/lib/rating/models/rating/extension.rb @@ -48,17 +48,17 @@ def rating(as: nil, scoping: nil) has_many :rating_records, as: :resource, - class_name: ::Rating.config.rating_model, + class_name: '::Rating::Rating', dependent: :destroy has_many :rates_records, as: :resource, - class_name: ::Rating.config.rate_model, + class_name: '::Rating::Rate', dependent: :destroy has_many :rated_records, as: :author, - class_name: ::Rating.config.rate_model, + class_name: '::Rating::Rate', dependent: :destroy scope :order_by_rating, ->(column = :estimate, direction = :desc, scope: nil) { diff --git a/lib/rating/models/rating/rate.rb b/lib/rating/models/rating/rate.rb index 2329ecb..9cc98f1 100644 --- a/lib/rating/models/rating/rate.rb +++ b/lib/rating/models/rating/rate.rb @@ -3,7 +3,7 @@ module Rating class Rate < ActiveRecord::Base self.table_name_prefix = 'rating_' - self.table_name = ::Rating.config.rate_model.constantize.table_name + self.table_name = ::Rating.config.rate_table after_save :update_rating diff --git a/lib/rating/models/rating/rating.rb b/lib/rating/models/rating/rating.rb index a5c4dfa..23e55d4 100644 --- a/lib/rating/models/rating/rating.rb +++ b/lib/rating/models/rating/rating.rb @@ -3,7 +3,7 @@ module Rating class Rating < ActiveRecord::Base self.table_name_prefix = 'rating_' - self.table_name = ::Rating.config.rating_model.constantize.table_name + self.table_name = ::Rating.config.rating_table belongs_to :resource, polymorphic: true belongs_to :scopeable, polymorphic: true diff --git a/spec/config/configure_spec.rb b/spec/config/configure_spec.rb deleted file mode 100644 index cafdd42..0000000 --- a/spec/config/configure_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Rating::Config, '.configure' do - before do - Rating.configure do |config| - config.models rate: 'Review', rating: 'ReviewRating' - end - end - - let!(:author) { create :author } - let!(:resource) { create :article } - - xit 'changes the rating models' do - author.rate resource, 5 - - expect(Review.count).to eq 1 - expect(ReviewRating.count).to eq 1 - end -end diff --git a/spec/config/initialize_spec.rb b/spec/config/initialize_spec.rb deleted file mode 100644 index 520a932..0000000 --- a/spec/config/initialize_spec.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Rating::Config, 'initialize' do - it 'has default models' do - expect(subject.rate_model).to eq '::Rating::Rate' - expect(subject.rating_model).to eq '::Rating::Rating' - end -end diff --git a/spec/config/models_spec.rb b/spec/config/models_spec.rb deleted file mode 100644 index e0fbcc6..0000000 --- a/spec/config/models_spec.rb +++ /dev/null @@ -1,12 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Rating::Config, '.models' do - it 'changes the rating models' do - subject.models rate: Review, rating: ReviewRating - - expect(subject.rate_model).to eq Review - expect(subject.rating_model).to eq ReviewRating - end -end diff --git a/spec/config/rate_table_spec.rb b/spec/config/rate_table_spec.rb new file mode 100644 index 0000000..72968fa --- /dev/null +++ b/spec/config/rate_table_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Rating::Config, '.rate_table' do + context 'when rating.yml does not exist' do + it { expect(subject.rate_table).to eq 'rating_rates' } + end if ENV['CONFIG_ENABLED'] != 'true' + + context 'when rating.yml exists' do + it { expect(subject.rate_table).to eq 'reviews' } + end if ENV['CONFIG_ENABLED'] == 'true' +end diff --git a/spec/config/rating_table_spec.rb b/spec/config/rating_table_spec.rb new file mode 100644 index 0000000..9d9bdc9 --- /dev/null +++ b/spec/config/rating_table_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Rating::Config, '.rating_table' do + context 'when rating.yml does not exist' do + it { expect(subject.rating_table).to eq 'rating_ratings' } + end if ENV['CONFIG_ENABLED'] != 'true' + + context 'when rating.yml exists' do + it { expect(subject.rating_table).to eq 'review_ratings' } + end if ENV['CONFIG_ENABLED'] == 'true' +end diff --git a/spec/support/db/migrate/add_comment_on_rating_rates_table.rb b/spec/support/db/migrate/add_comment_on_rating_rates_table.rb index 690c213..e53c402 100644 --- a/spec/support/db/migrate/add_comment_on_rating_rates_table.rb +++ b/spec/support/db/migrate/add_comment_on_rating_rates_table.rb @@ -3,5 +3,6 @@ class AddCommentOnRatingRatesTable < ActiveRecord::Migration[5.0] def change add_column :rating_rates, :comment, :text + add_column :reviews, :comment, :text end end diff --git a/spec/support/models/review.rb b/spec/support/models/review.rb index d88fa65..f1a3dc3 100644 --- a/spec/support/models/review.rb +++ b/spec/support/models/review.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true class Review < ::ActiveRecord::Base + belongs_to :scopeable, polymorphic: true end From 1b05516762304a4296ac1d13e37751ed3d6d9442 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Wed, 14 Feb 2018 14:43:26 -0200 Subject: [PATCH 16/19] up: add rake to run spec with config enabled --- Rakefile | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Rakefile b/Rakefile index dc8c23d..c3c8075 100644 --- a/Rakefile +++ b/Rakefile @@ -5,3 +5,21 @@ require 'rspec/core/rake_task' RSpec::Core::RakeTask.new task default: :spec + +desc 'Runs tests with config enabled' +task :spec_config do + directory_config = File.expand_path('config') + unsafe_path = ['', '/', '.', './'].include?(directory_config) + + `mkdir -p #{directory_config}` + + File.open(File.expand_path('config/rating.yml'), 'w+') do |file| + file.write "rating:\n rate_table: 'reviews'\n rating_table: 'review_ratings'" + end + + ENV['CONFIG_ENABLED'] = 'true' + + Rake::Task['spec'].invoke + + FileUtils.rm_rf(directory_config) unless unsafe_path +end From ce7802072eaa75288bda4ae448b5af6031560fda Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Wed, 14 Feb 2018 14:44:14 -0200 Subject: [PATCH 17/19] rm: config via initializer was dropped table name cannot be change after load --- lib/generators/rating/install_generator.rb | 6 ------ .../rating/templates/config/initializers/rating.rb | 3 --- 2 files changed, 9 deletions(-) delete mode 100644 lib/generators/rating/templates/config/initializers/rating.rb diff --git a/lib/generators/rating/install_generator.rb b/lib/generators/rating/install_generator.rb index 0dd8853..c810816 100644 --- a/lib/generators/rating/install_generator.rb +++ b/lib/generators/rating/install_generator.rb @@ -4,12 +4,6 @@ module Rating class InstallGenerator < Rails::Generators::Base source_root File.expand_path('../templates', __FILE__) - desc 'Creates Rating initializer' - - def copy_initializer - copy_file 'config/initializers/rating.rb', 'config/initializers/rating.rb' - end - desc 'Creates Rating migration' def create_migration diff --git a/lib/generators/rating/templates/config/initializers/rating.rb b/lib/generators/rating/templates/config/initializers/rating.rb deleted file mode 100644 index fd789c5..0000000 --- a/lib/generators/rating/templates/config/initializers/rating.rb +++ /dev/null @@ -1,3 +0,0 @@ -# Rating.configure do |config| -# config.models rate: '::Rating::Rate', rating: '::Rating::Rating' -# end From 6d71dccb2d51208b78e916f1c9628fa775d3632d Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Wed, 14 Feb 2018 14:46:51 -0200 Subject: [PATCH 18/19] ref: no more need block config --- lib/rating.rb | 9 --------- lib/rating/config.rb | 4 +++- lib/rating/models/rating/rate.rb | 2 +- lib/rating/models/rating/rating.rb | 2 +- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/rating.rb b/lib/rating.rb index 09b8a6c..cbd22fa 100644 --- a/lib/rating.rb +++ b/lib/rating.rb @@ -1,15 +1,6 @@ # frozen_string_literal: true module Rating - class << self - def config - @config ||= Config.new - end - - def configure - yield config - end - end end require 'rating/config' diff --git a/lib/rating/config.rb b/lib/rating/config.rb index 172b793..facdefa 100644 --- a/lib/rating/config.rb +++ b/lib/rating/config.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true module Rating - class Config + module Config + module_function + def config @config ||= begin file_path = File.expand_path('config/rating.yml') diff --git a/lib/rating/models/rating/rate.rb b/lib/rating/models/rating/rate.rb index 9cc98f1..d7bbf75 100644 --- a/lib/rating/models/rating/rate.rb +++ b/lib/rating/models/rating/rate.rb @@ -3,7 +3,7 @@ module Rating class Rate < ActiveRecord::Base self.table_name_prefix = 'rating_' - self.table_name = ::Rating.config.rate_table + self.table_name = ::Rating::Config.rate_table after_save :update_rating diff --git a/lib/rating/models/rating/rating.rb b/lib/rating/models/rating/rating.rb index 23e55d4..35c3c55 100644 --- a/lib/rating/models/rating/rating.rb +++ b/lib/rating/models/rating/rating.rb @@ -3,7 +3,7 @@ module Rating class Rating < ActiveRecord::Base self.table_name_prefix = 'rating_' - self.table_name = ::Rating.config.rating_table + self.table_name = ::Rating::Config.rating_table belongs_to :resource, polymorphic: true belongs_to :scopeable, polymorphic: true From 7247a25a96cc923424062189342dcef7b53665c7 Mon Sep 17 00:00:00 2001 From: Washington Botelho Date: Wed, 14 Feb 2018 15:02:44 -0200 Subject: [PATCH 19/19] up: makes spec for with and without config --- Rakefile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Rakefile b/Rakefile index c3c8075..d180f88 100644 --- a/Rakefile +++ b/Rakefile @@ -23,3 +23,11 @@ task :spec_config do FileUtils.rm_rf(directory_config) unless unsafe_path end + +desc 'Runs tests with and without config enabled' +task :spec do + Rake::Task['spec'].invoke + Rake::Task['spec_config'].invoke + + puts "Spec with and without config enabled executed." +end