From 0817e2ac9ee834c68677243252827ccc3cba3f51 Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Tue, 10 Mar 2020 10:59:49 -0400 Subject: [PATCH] Create implicit experience level rating vote upon creating of readinglist reaction (#6522) [deploy] * Do not render or link to empty rss feeds * s * Move calculation to worker * remove xit test * Also allow implicit rating votes in comments * Fix alignment issue * Remove comment logic --- app/controllers/comments_controller.rb | 1 - app/controllers/rating_votes_controller.rb | 1 - app/controllers/reactions_controller.rb | 7 ++++ app/models/rating_vote.rb | 16 +++---- .../rating_votes/assign_rating_worker.rb | 21 ++++++++++ ...00303222558_add_context_to_rating_votes.rb | 5 +++ ...200308144606_add_intdex_to_rating_votes.rb | 7 ++++ db/schema.rb | 4 +- spec/models/rating_vote_spec.rb | 42 +++++++++++-------- spec/requests/reactions_spec.rb | 21 ++++++++++ .../rating_votes/assign_rating_worker_spec.rb | 42 +++++++++++++++++++ 11 files changed, 136 insertions(+), 31 deletions(-) create mode 100644 app/workers/rating_votes/assign_rating_worker.rb create mode 100644 db/migrate/20200303222558_add_context_to_rating_votes.rb create mode 100644 db/migrate/20200308144606_add_intdex_to_rating_votes.rb create mode 100644 spec/workers/rating_votes/assign_rating_worker_spec.rb diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 3fca99186..cfc0437a6 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -75,7 +75,6 @@ class CommentsController < ApplicationController render json: { status: "comment already exists" } return end - render json: { status: "created", css: @comment.custom_css, diff --git a/app/controllers/rating_votes_controller.rb b/app/controllers/rating_votes_controller.rb index 29581fdfd..259de37e9 100644 --- a/app/controllers/rating_votes_controller.rb +++ b/app/controllers/rating_votes_controller.rb @@ -9,7 +9,6 @@ class RatingVotesController < ApplicationController rating_vote.rating = rating_vote_params[:rating].to_f rating_vote.group = rating_vote_params[:group] if rating_vote.save - rating_vote.assign_article_rating redirect_back(fallback_location: "/mod") else render json: { result: "Not Upserted Successfully" } diff --git a/app/controllers/reactions_controller.rb b/app/controllers/reactions_controller.rb index 47309cfbc..03df88f3e 100644 --- a/app/controllers/reactions_controller.rb +++ b/app/controllers/reactions_controller.rb @@ -82,6 +82,13 @@ class ReactionsController < ApplicationController Moderator::SinkArticles.call(reaction.reactable_id) if vomit_reaction_on_user?(reaction) Notification.send_reaction_notification(reaction, reaction_user(reaction)) Notification.send_reaction_notification(reaction, reaction.reactable.organization) if organization_article?(reaction) + if category == "readinglist" && current_user.experience_level + RatingVote.create(article_id: reaction.reactable_id, + group: "experience_level", + user_id: current_user.id, + context: "readinglist_reaction", + rating: current_user.experience_level) + end end render json: { result: result, category: category } end diff --git a/app/models/rating_vote.rb b/app/models/rating_vote.rb index 7469719cd..70c162cb6 100644 --- a/app/models/rating_vote.rb +++ b/app/models/rating_vote.rb @@ -2,28 +2,24 @@ class RatingVote < ApplicationRecord belongs_to :article belongs_to :user - validates :user_id, uniqueness: { scope: :article_id } + validates :user_id, uniqueness: { scope: %i[article_id context] } validates :group, inclusion: { in: %w[experience_level] } + validates :context, inclusion: { in: %w[explicit readinglist_reaction comment] } validates :rating, numericality: { greater_than: 0.0, less_than_or_equal_to: 10.0 } validate :permissions + after_create_commit :assign_article_rating + counter_culture :article counter_culture :user def assign_article_rating - ratings = article.rating_votes.where(group: group).pluck(:rating) - average = ratings.sum / ratings.size - - article.update_columns( - experience_level_rating: average, - experience_level_rating_distribution: ratings.max - ratings.min, - last_experience_level_rating_at: Time.current, - ) + RatingVotes::AssignRatingWorker.perform_async(article_id) end private def permissions - errors.add(:user_id, "is not permitted to take this action.") if !user&.trusted && user_id != article&.user_id + errors.add(:user_id, "is not permitted to take this action.") if context == "explicit" && !user&.trusted && user_id != article&.user_id end end diff --git a/app/workers/rating_votes/assign_rating_worker.rb b/app/workers/rating_votes/assign_rating_worker.rb new file mode 100644 index 000000000..4292cac13 --- /dev/null +++ b/app/workers/rating_votes/assign_rating_worker.rb @@ -0,0 +1,21 @@ +module RatingVotes + class AssignRatingWorker + include Sidekiq::Worker + + sidekiq_options queue: :low_priority, retry: 10 + + def perform(article_id, group = "experience_level") + article = Article.find_by(id: article_id) + return unless article + + ratings = article.rating_votes.where(group: group).pluck(:rating) + average = ratings.sum / ratings.size + + article.update_columns( + experience_level_rating: average, + experience_level_rating_distribution: ratings.max - ratings.min, + last_experience_level_rating_at: Time.current, + ) + end + end +end diff --git a/db/migrate/20200303222558_add_context_to_rating_votes.rb b/db/migrate/20200303222558_add_context_to_rating_votes.rb new file mode 100644 index 000000000..22a960662 --- /dev/null +++ b/db/migrate/20200303222558_add_context_to_rating_votes.rb @@ -0,0 +1,5 @@ +class AddContextToRatingVotes < ActiveRecord::Migration[5.2] + def change + add_column :rating_votes, :context, :string, default: "explicit" + end +end diff --git a/db/migrate/20200308144606_add_intdex_to_rating_votes.rb b/db/migrate/20200308144606_add_intdex_to_rating_votes.rb new file mode 100644 index 000000000..8b3bb772e --- /dev/null +++ b/db/migrate/20200308144606_add_intdex_to_rating_votes.rb @@ -0,0 +1,7 @@ +class AddIntdexToRatingVotes < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_index :rating_votes, %i[user_id article_id context], unique: true, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index c4ac47d4c..7d12e24a3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_03_04_220534) do +ActiveRecord::Schema.define(version: 2020_03_08_144606) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -931,12 +931,14 @@ ActiveRecord::Schema.define(version: 2020_03_04_220534) do create_table "rating_votes", force: :cascade do |t| t.bigint "article_id" + t.string "context", default: "explicit" t.datetime "created_at", null: false t.string "group" t.float "rating" t.datetime "updated_at", null: false t.bigint "user_id" t.index ["article_id"], name: "index_rating_votes_on_article_id" + t.index ["user_id", "article_id", "context"], name: "index_rating_votes_on_user_id_and_article_id_and_context", unique: true t.index ["user_id"], name: "index_rating_votes_on_user_id" end diff --git a/spec/models/rating_vote_spec.rb b/spec/models/rating_vote_spec.rb index b671ac075..cf863526e 100644 --- a/spec/models/rating_vote_spec.rb +++ b/spec/models/rating_vote_spec.rb @@ -21,36 +21,42 @@ RSpec.describe RatingVote, type: :model do rating = build(:rating_vote, article_id: article.id, user_id: user.id) expect(rating).not_to be_valid end + + it "does allows more than one reaction if different contexts" do + create(:rating_vote, article_id: article.id, user_id: user.id) + rating = build(:rating_vote, article_id: article.id, user_id: user.id, context: "readinglist_reaction") + expect(rating).to be_valid + end + + it "does allows more than one two reactions if all different contexts" do + create(:rating_vote, article_id: article.id, user_id: user.id) + create(:rating_vote, article_id: article.id, user_id: user.id, context: "readinglist_reaction") + rating = build(:rating_vote, article_id: article.id, user_id: user.id, context: "comment") + expect(rating).to be_valid + end end describe "modifies article rating score" do - it "assigns article rating" do - rating = create(:rating_vote, article_id: article.id, user_id: user.id, rating: 2.0) - create(:rating_vote, article_id: article.id, user_id: user2.id, rating: 3.0) - - rating.assign_article_rating - article.reload - - expect(article.experience_level_rating).to eq(2.5) - expect(article.experience_level_rating_distribution).to eq(1.0) + before do + allow(RatingVotes::AssignRatingWorker).to receive(:perform_async) end - it "assigns article rating with larger distribution" do - rating = create(:rating_vote, article_id: article.id, user_id: user.id, rating: 1.0) - create(:rating_vote, article_id: article.id, user_id: user2.id, rating: 7.0) + it "assigns article rating" do + create(:rating_vote, article_id: article.id, user_id: user2.id, rating: 3.0) - rating.assign_article_rating - article.reload - - expect(article.experience_level_rating).to eq(4.0) - expect(article.experience_level_rating_distribution).to eq(6.0) + expect(RatingVotes::AssignRatingWorker).to have_received(:perform_async).with(article.id) end end describe "permissions" do let_it_be(:untrusted_user) { create(:user) } - it "allows trusted users to make rating" do + it "allows untrusted user to leave readinglist_reaction context rating" do + rating = build(:rating_vote, article_id: article.id, user_id: untrusted_user.id, context: "readinglist_reaction") + expect(rating).to be_valid + end + + it "allows trusted users to make explicit rating" do rating = build(:rating_vote, article_id: article.id, user_id: user.id) expect(rating).to be_valid end diff --git a/spec/requests/reactions_spec.rb b/spec/requests/reactions_spec.rb index af02f6312..4ab7e4ea4 100644 --- a/spec/requests/reactions_spec.rb +++ b/spec/requests/reactions_spec.rb @@ -165,6 +165,27 @@ RSpec.describe "Reactions", type: :request do end end + context "when creating readinglist" do + before do + user.update_column(:experience_level, 8) + sign_in user + post "/reactions", params: { + reactable_id: article.id, + reactable_type: "Article", + category: "readinglist" + } + end + + it "creates reaction" do + expect(Reaction.last.reactable_id).to eq(article.id) + end + + it "creates rating vote" do + expect(RatingVote.last.context).to eq("readinglist_reaction") + expect(RatingVote.last.rating).to be(8.0) + end + end + context "when vomiting on a user" do before do sign_in trusted_user diff --git a/spec/workers/rating_votes/assign_rating_worker_spec.rb b/spec/workers/rating_votes/assign_rating_worker_spec.rb new file mode 100644 index 000000000..26da2fb07 --- /dev/null +++ b/spec/workers/rating_votes/assign_rating_worker_spec.rb @@ -0,0 +1,42 @@ +require "rails_helper" + +RSpec.describe RatingVotes::AssignRatingWorker, type: :worker do + let(:article) { create(:article) } + let(:user) { create(:user) } + let(:second_user) { create(:user) } + let(:worker) { subject } + + before do + allow(CacheBuster).to receive(:bust_podcast) + end + + describe "#perform" do + it "assigns explicit score" do + user.add_role(:trusted) + second_user.add_role(:trusted) + create(:rating_vote, article_id: article.id, user_id: user.id, rating: 3.0) + create(:rating_vote, article_id: article.id, user_id: second_user.id, rating: 2.0) + worker.perform(article.id) + expect(article.reload.experience_level_rating).to eq(2.5) + expect(article.reload.experience_level_rating_distribution).to eq(1.0) + end + + it "assigns implicit readinglist_reaction score" do + user.add_role(:trusted) + create(:rating_vote, article_id: article.id, user_id: user.id, rating: 4.0) + create(:rating_vote, article_id: article.id, user_id: second_user.id, rating: 2.0, context: "readinglist_reaction") + worker.perform(article.id) + expect(article.reload.experience_level_rating).to eq(3.0) + expect(article.reload.experience_level_rating_distribution).to eq(2.0) + end + + it "assigns implicit comment score" do + user.add_role(:trusted) + create(:rating_vote, article_id: article.id, user_id: user.id, rating: 4.0) + create(:rating_vote, article_id: article.id, user_id: second_user.id, rating: 1.0, context: "comment") + worker.perform(article.id) + expect(article.reload.experience_level_rating).to eq(2.5) + expect(article.reload.experience_level_rating_distribution).to eq(3.0) + end + end +end