diff --git a/app/controllers/buffered_articles_controller.rb b/app/controllers/buffered_articles_controller.rb index 046cdd114..0831d9a3a 100644 --- a/app/controllers/buffered_articles_controller.rb +++ b/app/controllers/buffered_articles_controller.rb @@ -1,20 +1,21 @@ class BufferedArticlesController < ApplicationController # No authorization required for entirely public controller + def index - @article_urls = buffered_article_urls - render json: { - urls: @article_urls - }.to_json + render json: { urls: buffered_articles_urls }.to_json end - def buffered_article_urls - if Rails.env.production? - Article. - where("last_buffered > ? OR published_at > ?", 24.hours.ago, 20.minutes.ago). - select(:path). - map { |a| "https://#{ApplicationConfig['APP_DOMAIN']}#{a.path}" } - else - Article.all.map { |a| "https://#{ApplicationConfig['APP_DOMAIN']}#{a.path}" } - end + private + + def buffered_articles_urls + relation = if Rails.env.production? + Article.where("last_buffered > ?", 24.hours.ago). + or(Article.where("published_at > ?", 20.minutes.ago)) + else + Article.all + end + + paths = relation.pluck(:path) + paths.map { |path| "https://#{ApplicationConfig['APP_DOMAIN']}#{path}" } end end diff --git a/app/controllers/reactions_controller.rb b/app/controllers/reactions_controller.rb index 774fdb1c6..47309cfbc 100644 --- a/app/controllers/reactions_controller.rb +++ b/app/controllers/reactions_controller.rb @@ -4,37 +4,46 @@ class ReactionsController < ApplicationController def index skip_authorization + if params[:article_id] id = params[:article_id] - reactions = if session_current_user_id.present? - Reaction.where(reactable_id: id, - reactable_type: "Article", - user_id: session_current_user_id). - where("points > ?", 0) + + reactions = if session_current_user_id + Reaction.positive. + where( + reactable_id: id, + reactable_type: "Article", + user_id: session_current_user_id, + ) else - [] + Reaction.none end - render json: - { - current_user: { id: session_current_user_id }, - article_reaction_counts: Reaction.count_for_article(id), - reactions: reactions - }.to_json + + result = { article_reaction_counts: Reaction.count_for_article(id) } else - comments = Comment.where( - commentable_id: params[:commentable_id], - commentable_type: params[:commentable_type], - ).select(%i[id positive_reactions_count]) - comment_ids = comments.map(&:id) - reaction_counts = comments.map { |c| { id: c.id, count: c.positive_reactions_count } } - reactions = session_current_user_id ? cached_user_positive_reactions(current_user).where(reactable_id: comment_ids) : [] - render json: - { - current_user: { id: session_current_user_id }, - positive_reaction_counts: reaction_counts, - reactions: reactions - }.to_json + comments = Comment. + where(commentable_id: params[:commentable_id], commentable_type: params[:commentable_type]). + select(%i[id positive_reactions_count]) + + reaction_counts = comments.map do |comment| + { id: comment.id, count: comment.positive_reactions_count } + end + + reactions = if session_current_user_id + comment_ids = reaction_counts.map { |rc| rc[:id] } + cached_user_positive_reactions(current_user).where(reactable_id: comment_ids) + else + Reaction.none + end + + result = { positive_reaction_counts: reaction_counts } end + + render json: { + current_user: { id: session_current_user_id }, + reactions: reactions + }.merge(result).to_json + set_surrogate_key_header params.to_s unless session_current_user_id end @@ -79,8 +88,7 @@ class ReactionsController < ApplicationController def cached_user_positive_reactions(user) Rails.cache.fetch("cached_user_reactions-#{user.id}-#{user.updated_at}", expires_in: 24.hours) do - Reaction.where(user_id: user.id). - where("points > ?", 0) + user.reactions.positive end end diff --git a/app/labor/mailchimp_bot.rb b/app/labor/mailchimp_bot.rb index b85c0507b..65ef09868 100644 --- a/app/labor/mailchimp_bot.rb +++ b/app/labor/mailchimp_bot.rb @@ -76,8 +76,12 @@ class MailchimpBot return false unless user.tag_moderator? success = false - tags = user.roles.where(name: "tag_moderator").map { |tag| Tag.find(tag.resource_id).name } + + tag_ids = user.roles.where(name: "tag_moderator").pluck(:resource_id) + tag_names = Tag.where(id: tag_ids).pluck(:name) + status = user.email_tag_mod_newsletter ? "subscribed" : "unsubscribed" + begin gibbon.lists(SiteConfig.mailchimp_tag_moderators_id).members(target_md5_email).upsert( body: { @@ -89,7 +93,7 @@ class MailchimpBot TWITTER: user.twitter_username.to_s, GITHUB: user.github_username.to_s, IMAGE_URL: user.profile_image_url.to_s, - TAGS: tags.join(", ") + TAGS: tag_names.join(", ") } }, ) diff --git a/app/models/chat_channel.rb b/app/models/chat_channel.rb index e8c4066ec..76ac46a17 100644 --- a/app/models/chat_channel.rb +++ b/app/models/chat_channel.rb @@ -55,7 +55,7 @@ class ChatChannel < ApplicationRecord raise "Invalid direct channel" if users.size != 2 && channel_type == "direct" if channel_type == "direct" - usernames = users.map(&:username).sort + usernames = users.map(&:username).sort # .map as `users` is an array contrived_name = "Direct chat between " + usernames.join(" and ") slug = usernames.join("/") else diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index 96e2fa87a..7aefbdfaa 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -6,6 +6,6 @@ module Reactable end def sync_reactions_count - update_column(:positive_reactions_count, reactions.where("points > ?", 0).size) + update_column(:positive_reactions_count, reactions.positive.size) end end diff --git a/app/models/reaction.rb b/app/models/reaction.rb index 3f32630e0..7f350a21a 100644 --- a/app/models/reaction.rb +++ b/app/models/reaction.rb @@ -14,6 +14,8 @@ class Reaction < ApplicationRecord } counter_culture :user + scope :positive, -> { where("points > ?", 0) } + validates :category, inclusion: { in: CATEGORIES } validates :reactable_type, inclusion: { in: REACTABLE_TYPES } validates :status, inclusion: { in: STATUSES } @@ -48,8 +50,10 @@ class Reaction < ApplicationRecord def count_for_article(id) Rails.cache.fetch("count_for_reactable-Article-#{id}", expires_in: 1.hour) do reactions = Reaction.where(reactable_id: id, reactable_type: "Article") + counts = reactions.group(:category).count + %w[like readinglist unicorn].map do |type| - { category: type, count: reactions.where(category: type).size } + { category: type, count: counts.fetch(type, 0) } end end end diff --git a/app/services/analytics_service.rb b/app/services/analytics_service.rb index d8b6e4994..aca9a0ce7 100644 --- a/app/services/analytics_service.rb +++ b/app/services/analytics_service.rb @@ -85,9 +85,8 @@ class AnalyticsService where("score > 0") @follow_data = Follow. where(followable_type: user_or_org.class.name, followable_id: user_or_org.id) - @reaction_data = Reaction. - where(reactable_id: article_ids, reactable_type: "Article"). - where("points > 0") + @reaction_data = Reaction.positive. + where(reactable_id: article_ids, reactable_type: "Article") @page_view_data = PageView.where(article_id: article_ids) # filter data by date if needed diff --git a/db/seeds.rb b/db/seeds.rb index c37a3578f..ca1f2a413 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -92,7 +92,7 @@ Article.clear_index! num_articles.times do |i| tags = [] tags << "discuss" if (i % 3).zero? - tags.concat Tag.order(Arel.sql("RANDOM()")).select("name").first(3).map(&:name) + tags.concat Tag.order(Arel.sql("RANDOM()")).limit(3).pluck(:name) markdown = <<~MARKDOWN --- diff --git a/spec/models/reaction_spec.rb b/spec/models/reaction_spec.rb index 8c1fa45af..0ec318c6b 100644 --- a/spec/models/reaction_spec.rb +++ b/spec/models/reaction_spec.rb @@ -111,6 +111,20 @@ RSpec.describe Reaction, type: :model do end end + describe ".count_for_article" do + it "counts the reactions an article has grouped by category" do + create(:reaction, reactable: article, user: user, category: "like") + create(:reaction, reactable: article, user: user, category: "unicorn") + + expected_result = [ + { category: "like", count: 1 }, + { category: "readinglist", count: 0 }, + { category: "unicorn", count: 1 }, + ] + expect(described_class.count_for_article(article.id)).to eq(expected_result) + end + end + context "when callbacks are called after save" do let!(:reaction) { build(:reaction, category: "like", reactable: article, user: user) } diff --git a/spec/models/shared_examples/sync_reactions_count.rb b/spec/models/shared_examples/sync_reactions_count.rb index 1a0470b6c..4616d80d7 100644 --- a/spec/models/shared_examples/sync_reactions_count.rb +++ b/spec/models/shared_examples/sync_reactions_count.rb @@ -11,7 +11,7 @@ RSpec.shared_examples "#sync_reactions_count" do |reactable_type| expect(reactable.positive_reactions_count).to eq(0) reactable.sync_reactions_count reactable.reload - expected_count = reactable.reactions.where("points > ?", 0).size + expected_count = reactable.reactions.positive.size expect(reactable.positive_reactions_count).to eq(expected_count) end end diff --git a/spec/requests/buffered_articles_spec.rb b/spec/requests/buffered_articles_spec.rb index 3e9be4649..cc44cd465 100644 --- a/spec/requests/buffered_articles_spec.rb +++ b/spec/requests/buffered_articles_spec.rb @@ -2,20 +2,23 @@ require "rails_helper" RSpec.describe "BufferedArticles", type: :request do describe "GET /buffered_articles" do - it "works! (now write some real specs)" do - get "/buffered_articles" + it "works successfully" do + get buffered_articles_path + expect(response).to have_http_status(:ok) end it "responds with json" do - get "/buffered_articles" + get buffered_articles_path + expect(response.content_type).to eq("application/json") end it "responds with at least one url" do - create(:article) - get "/buffered_articles" - expect(response.body).to include(ApplicationConfig["APP_DOMAIN"]) + article = create(:article) + + get buffered_articles_path + expect(response.parsed_body["urls"].first).to eq(article.decorate.url) end end end diff --git a/spec/requests/reactions_spec.rb b/spec/requests/reactions_spec.rb index 11b8e4013..5ea51cb78 100644 --- a/spec/requests/reactions_spec.rb +++ b/spec/requests/reactions_spec.rb @@ -2,20 +2,35 @@ require "rails_helper" RSpec.describe "Reactions", type: :request do let(:user) { create(:user) } - let(:article) { create(:article, user_id: user.id) } + let(:article) { create(:article, user: user) } let(:comment) { create(:comment, commentable: article) } - let(:max_age) { FastlyRails.configuration.max_age } - let(:stale_if_error) { FastlyRails.configuration.stale_if_error } + + let_it_be(:max_age) { FastlyRails.configuration.max_age } + let_it_be(:stale_if_error) { FastlyRails.configuration.stale_if_error } describe "GET /reactions?article_id=:article_id" do + before do + create(:reaction, reactable: article, user: user, points: 1) + end + context "when signed in" do before do sign_in user - get "/reactions?article_id=#{article.id}" + + get reactions_path(article_id: article.id) end - it "returns reactions count for article" do - expect(response.body).to include("article_reaction_counts") + it "returns the correct json response" do + result = response.parsed_body + + expect(result["current_user"]).to eq("id" => user.id) + expected_reactions_counts = [ + { "category" => "like", "count" => 1 }, + { "category" => "readinglist", "count" => 0 }, + { "category" => "unicorn", "count" => 0 }, + ] + expect(result["article_reaction_counts"]).to eq(expected_reactions_counts) + expect(result["reactions"].to_json).to eq(user.reactions.where(reactable: article).to_json) end it "does not set cache control headers" do @@ -31,10 +46,19 @@ RSpec.describe "Reactions", type: :request do end context "when signed out" do - before { get "/reactions?article_id=#{article.id}" } + before { get reactions_path(article_id: article.id) } - it "returns reactions count for article" do - expect(response.body).to include("article_reaction_counts") + it "returns the correct json response" do + result = response.parsed_body + + expect(result["current_user"]).to eq("id" => nil) + expected_reactions = [ + { "category" => "like", "count" => 1 }, + { "category" => "readinglist", "count" => 0 }, + { "category" => "unicorn", "count" => 0 }, + ] + expect(result["article_reaction_counts"]).to eq(expected_reactions) + expect(result["reactions"]).to be_empty end it "sets the surrogate key header equal to params for article" do @@ -50,19 +74,28 @@ RSpec.describe "Reactions", type: :request do end end - describe "GET /reactions?commentable_id=:article.id&commentable_type=Comment" do + describe "GET /reactions?commentable_id=:article.id&commentable_type=Article" do + before do + create(:reaction, reactable: comment, user: user, points: 1) + end + context "when signed in" do before do sign_in user - get "/reactions?commentable_id=#{article.id}&commentable_type=Comment" + + get reactions_path(commentable_id: article.id, commentable_type: "Article") end - it "returns positive reaction counts" do - expect(response.body).to include("positive_reaction_counts") + it "returns the correct json response" do + result = response.parsed_body + + expect(result["current_user"]).to eq("id" => user.id) + expect(result["positive_reaction_counts"]).to eq([{ "id" => article.comments.last.id, "count" => 1 }]) + expect(result["reactions"].to_json).to eq(user.reactions.where(reactable: comment).to_json) end it "does not set surrogate key headers" do - expect(response.headers["Surrogate-Key"]).to eq(nil) + expect(response.headers["surrogate-key"]).to be_nil end it "does not set Fastly cache control and surrogate control headers" do @@ -74,10 +107,14 @@ RSpec.describe "Reactions", type: :request do end context "when signed out" do - before { get "/reactions?commentable_id=#{article.id}&commentable_type=Comment" } + before { get reactions_path(commentable_id: article.id, commentable_type: "Article") } - it "returns positive reaction counts" do - expect(response.body).to include("positive_reaction_counts") + it "returns the correct json response" do + result = response.parsed_body + + expect(result["current_user"]).to eq("id" => nil) + expect(result["positive_reaction_counts"]).to eq([{ "id" => article.comments.last.id, "count" => 1 }]) + expect(result["reactions"]).to be_empty end it "sets the surrogate key header equal to params" do