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/Gemfile.lock b/Gemfile.lock index 8e85f3f..70c6e67 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,9 +46,9 @@ GEM factory_bot_rails (4.8.2) factory_bot (~> 4.8.2) railties (>= 3.0.0) - i18n (0.9.3) + 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) @@ -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) diff --git a/Rakefile b/Rakefile index dc8c23d..d180f88 100644 --- a/Rakefile +++ b/Rakefile @@ -5,3 +5,29 @@ 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 + +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 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" diff --git a/lib/rating.rb b/lib/rating.rb index 68a56da..cbd22fa 100644 --- a/lib/rating.rb +++ b/lib/rating.rb @@ -3,6 +3,7 @@ module Rating 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..facdefa --- /dev/null +++ b/lib/rating/config.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Rating + module Config + module_function + + def config + @config ||= begin + file_path = File.expand_path('config/rating.yml') + + return {} unless File.exist?(file_path) + + YAML.safe_load(File.read(file_path))['rating'] + end + end + + 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 a8acca7..e4365ca 100644 --- a/lib/rating/models/rating/extension.rb +++ b/lib/rating/models/rating/extension.rb @@ -28,11 +28,23 @@ module Rating 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_create -> { rating_warm_up scoping: scoping }, unless: -> { as == :author } has_many :rating_records, as: :resource, diff --git a/lib/rating/models/rating/rate.rb b/lib/rating/models/rating/rate.rb index c78792f..d7bbf75 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_table after_save :update_rating diff --git a/lib/rating/models/rating/rating.rb b/lib/rating/models/rating/rating.rb index d43c169..35c3c55 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_ratings' + self.table_name_prefix = 'rating_' + self.table_name = ::Rating::Config.rating_table belongs_to :resource, polymorphic: true belongs_to :scopeable, polymorphic: true @@ -100,7 +101,7 @@ module Rating 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/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/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 index cb68339..cf14e90 100644 --- a/spec/models/extension/after_create_spec.rb +++ b/spec/models/extension/after_create_spec.rb @@ -2,27 +2,46 @@ require 'rails_helper' -RSpec.describe Rating::Extension, ':after_create' do - context 'when :as is nil' do - let!(:article) { create :article } +RSpec.describe Rating::Extension, 'after_create' do + context 'when record is author' do + let!(:record) { build :author } - it 'creates a rating record with zero values just to be easy to make the count' do - rating = Rating::Rating.find_by(resource: article) + it 'does not warm up the cache' do + expect(record).not_to receive(:rating_warm_up) - 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 + record.save end end - context 'when :as is :author' do - let!(:author) { create :author } + context 'when record is not author' do + context 'when record has scoping' do + let!(:record) { build :article } - it 'does not creates a rating record' do - expect(Rating::Rating.exists?(resource: author)).to eq false + 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 + + 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 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/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 'rails_helper' 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 @@ RSpec.describe Rating::Rating, ':update_rating' do 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 @@ RSpec.describe Rating::Rating, ':update_rating' do 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 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/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/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..d0c5d42 --- /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..1a24fbd 100644 --- a/spec/support/migrate.rb +++ b/spec/support/migrate.rb @@ -8,6 +8,9 @@ Dir[File.expand_path('db/migrate/*.rb', __dir__)].each { |file| require file } CreateArticlesTable.new.change CreateAuthorsTable.new.change CreateCategoriesTable.new.change +CreateCommentsTable.new.change CreateRateTable.new.change CreateRatingTable.new.change +CreateReviewRatingsTable.new.change +CreateReviewsTable.new.change AddCommentOnRatingRatesTable.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 diff --git a/spec/support/models/review.rb b/spec/support/models/review.rb new file mode 100644 index 0000000..f1a3dc3 --- /dev/null +++ b/spec/support/models/review.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class Review < ::ActiveRecord::Base + belongs_to :scopeable, polymorphic: true +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