From 5b3f9e564218ed99002c50e0dbb043a1f4578441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Sun, 6 Sep 2015 11:40:56 +0200 Subject: [PATCH 1/4] uses preloaded info --- app/views/comments/_comment.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 8bf07a8b3..239ce50f8 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, comment, @commentable, Flag.flagged?(current_user, comment)] do %> +<% cache [locale_and_user_status, comment, @commentable, (@comment_flags[comment.id] if @comment_flags)] do %>
From 60089b898631304333769e7f76e790b4d176fd4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Sun, 6 Sep 2015 12:13:39 +0200 Subject: [PATCH 2/4] uses author info in comment/debate cache author is already preloaded and we avoid expensive touching methods --- app/models/user.rb | 11 ----------- app/views/comments/_comment.html.erb | 2 +- app/views/debates/_comments.html.erb | 2 +- app/views/debates/show.html.erb | 2 +- spec/models/comment_spec.rb | 6 +++--- spec/models/debate_spec.rb | 4 ++-- 6 files changed, 8 insertions(+), 19 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 0ae4d3290..71db4ee2e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -36,17 +36,6 @@ class User < ActiveRecord::Base scope :officials, -> { where("official_level > 0") } scope :for_render, -> { includes(:organization) } - after_update :touch_debates, :touch_comments - after_touch :touch_debates, :touch_comments - - def touch_debates - debates.map(&:touch) - end - - def touch_comments - comments.map(&:touch) - end - def self.find_for_oauth(auth, signed_in_resource = nil) # Get the identity and user if they exist identity = Identity.find_for_oauth(auth) diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 239ce50f8..a84aebb63 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, comment, @commentable, (@comment_flags[comment.id] if @comment_flags)] do %> +<% cache [locale_and_user_status, comment, @commentable, comment.author, (@comment_flags[comment.id] if @comment_flags)] do %>
diff --git a/app/views/debates/_comments.html.erb b/app/views/debates/_comments.html.erb index 39ed635ba..9813ea4f8 100644 --- a/app/views/debates/_comments.html.erb +++ b/app/views/debates/_comments.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, @all_visible_comments.map(&:cache_key), @debate.comments_count, @comment_flags.to_a] do %> +<% cache [locale_and_user_status, @all_visible_comments, @all_visible_comments.map(&:author), @debate.comments_count, @comment_flags.to_a] do %>
diff --git a/app/views/debates/show.html.erb b/app/views/debates/show.html.erb index 1767266db..07f7504cf 100644 --- a/app/views/debates/show.html.erb +++ b/app/views/debates/show.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, @debate, Flag.flagged?(current_user, @debate)] do %> +<% cache [locale_and_user_status, @debate, @debate.author, Flag.flagged?(current_user, @debate)] do %>
diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 8798f4935..0349992cf 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -52,18 +52,18 @@ describe Comment do it "should expire cache when the author is hidden" do expect { comment.user.hide } - .to change { comment.reload.updated_at } + .to change { [comment.reload.updated_at, comment.author.updated_at] } end it "should expire cache when the author changes" do expect { comment.user.update(username: "Isabel") } - .to change { comment.reload.updated_at } + .to change { [comment.reload.updated_at, comment.author.updated_at] } end it "should expire cache when the author's organization get verified" do create(:organization, user: comment.user) expect { comment.user.organization.verify } - .to change { comment.reload.updated_at} + .to change { [comment.reload.updated_at, comment.author.updated_at] } end end end diff --git a/spec/models/debate_spec.rb b/spec/models/debate_spec.rb index 951837956..39ba93e51 100644 --- a/spec/models/debate_spec.rb +++ b/spec/models/debate_spec.rb @@ -288,13 +288,13 @@ describe Debate do it "should expire cache when its author changes" do expect { debate.author.update(username: "Eva") } - .to change { debate.reload.updated_at } + .to change { [debate.reload.updated_at, debate.author.updated_at] } end it "should expire cache when the author's organization get verified" do create(:organization, user: debate.author) expect { debate.author.organization.verify } - .to change { debate.reload.updated_at} + .to change { [debate.reload.updated_at, debate.author.updated_at] } end end From 31b00c32f4c8e034e58c263f5f433c2242ad38dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Sun, 6 Sep 2015 12:25:47 +0200 Subject: [PATCH 3/4] removes unneeded to_a --- app/views/debates/_comments.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/debates/_comments.html.erb b/app/views/debates/_comments.html.erb index 9813ea4f8..fe7fc3cb3 100644 --- a/app/views/debates/_comments.html.erb +++ b/app/views/debates/_comments.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, @all_visible_comments, @all_visible_comments.map(&:author), @debate.comments_count, @comment_flags.to_a] do %> +<% cache [locale_and_user_status, @all_visible_comments, @all_visible_comments.map(&:author), @debate.comments_count, @comment_flags] do %>
From e706aaff2c90a77b41fc8b3bd3d3d5f2c7aaeae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Sun, 6 Sep 2015 12:40:13 +0200 Subject: [PATCH 4/4] adds extra specs for debate's cache --- spec/models/debate_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/models/debate_spec.rb b/spec/models/debate_spec.rb index 39ba93e51..c36031089 100644 --- a/spec/models/debate_spec.rb +++ b/spec/models/debate_spec.rb @@ -286,6 +286,16 @@ describe Debate do .to change { debate.updated_at } end + it "should expire cache when hidden" do + expect { debate.hide } + .to change { debate.updated_at } + end + + it "should expire cache when the author is hidden" do + expect { debate.author.hide } + .to change { [debate.reload.updated_at, debate.author.updated_at] } + end + it "should expire cache when its author changes" do expect { debate.author.update(username: "Eva") } .to change { [debate.reload.updated_at, debate.author.updated_at] }