Remove votes query optimizations

Just like we did in commit 0214184b2d for investments, we're removing
some possible optimizations (we don't have any benchmarks proving they
affect performance at all) in order to simplify the code.

The investement votes component `delegate` code was accidentally left
but isn't used since commit 0214184b2, so we're removing it now that
we're removing the `voted_for?` helper method.
This commit is contained in:
Javi Martín
2021-09-28 19:31:57 +02:00
parent 78f372fd0b
commit b98244afd9
29 changed files with 39 additions and 123 deletions

View File

@@ -1,6 +1,6 @@
class Budgets::Investments::VotesComponent < ApplicationComponent
attr_reader :investment
delegate :namespace, :current_user, :voted_for?, :image_absolute_url,
delegate :namespace, :current_user, :image_absolute_url,
:link_to_verify_account, :link_to_signin, :link_to_signup, to: :helpers
def initialize(investment)

View File

@@ -1,4 +1,4 @@
<% voted_classes = css_classes_for_vote(debate_votes, debate) %>
<% voted_classes = css_classes_for_vote(debate) %>
<div class="votes">
<div class="in-favor inline-block">
<% if user_signed_in? %>

View File

@@ -1,9 +1,8 @@
class Debates::VotesComponent < ApplicationComponent
attr_reader :debate, :debate_votes
attr_reader :debate
delegate :css_classes_for_vote, :current_user, :link_to_verify_account, :user_signed_in?, :votes_percentage, to: :helpers
def initialize(debate, debate_votes:)
def initialize(debate)
@debate = debate
@debate_votes = debate_votes
end
end

View File

@@ -1,4 +1,4 @@
<% voted_classes = css_classes_for_vote(legislation_proposal_votes, proposal) %>
<% voted_classes = css_classes_for_vote(proposal) %>
<div class="votes">
<% if proposal.process.proposals_phase.open? %>
<div class="in-favor inline-block">
@@ -64,7 +64,7 @@
</div>
<% end %>
<% if voted_for?(legislation_proposal_votes, proposal) && setting["twitter_handle"] %>
<% if current_user&.voted_as_when_voted_for(proposal) && setting["twitter_handle"] %>
<div class="share-supported">
<%= render "shared/social_share",
title: proposal.title,

View File

@@ -1,9 +1,8 @@
class Legislation::Proposals::VotesComponent < ApplicationComponent
attr_reader :proposal, :legislation_proposal_votes
delegate :css_classes_for_vote, :current_user, :link_to_verify_account, :user_signed_in?, :voted_for?, :votes_percentage, to: :helpers
attr_reader :proposal
delegate :css_classes_for_vote, :current_user, :link_to_verify_account, :user_signed_in?, :votes_percentage, to: :helpers
def initialize(proposal, legislation_proposal_votes:)
def initialize(proposal)
@proposal = proposal
@legislation_proposal_votes = legislation_proposal_votes
end
end

View File

@@ -2,7 +2,7 @@
<%= render "proposals/supports", proposal: proposal %>
<div class="in-favor">
<% if voted_for?(proposal_votes, proposal) %>
<% if current_user&.voted_for?(proposal) %>
<div class="supported callout success">
<%= t("proposals.proposal.already_supported") %>
</div>
@@ -39,7 +39,7 @@
</div>
<% end %>
<% if voted_for?(proposal_votes, proposal) && setting["twitter_handle"] %>
<% if current_user&.voted_for?(proposal) && setting["twitter_handle"] %>
<div class="share-supported">
<%= render "proposals/social_share", proposal: proposal, share_title: false %>
</div>

View File

@@ -1,10 +1,9 @@
class Proposals::VotesComponent < ApplicationComponent
attr_reader :proposal, :vote_url, :proposal_votes
delegate :current_user, :link_to_verify_account, :user_signed_in?, :voted_for?, to: :helpers
attr_reader :proposal, :vote_url
delegate :current_user, :link_to_verify_account, :user_signed_in?, to: :helpers
def initialize(proposal, vote_url:, proposal_votes:)
def initialize(proposal, vote_url:)
@proposal = proposal
@vote_url = vote_url
@proposal_votes = proposal_votes
end
end

View File

@@ -68,14 +68,6 @@ class ApplicationController < ActionController::Base
end
end
def set_debate_votes(debates)
@debate_votes = current_user ? current_user.debate_votes(debates) : {}
end
def set_proposal_votes(proposals)
@proposal_votes = current_user ? current_user.proposal_votes(proposals) : {}
end
def set_comment_flags(comments)
@comment_flags = current_user ? current_user.comment_flags(comments) : {}
end

View File

@@ -17,14 +17,11 @@ module CommentableActions
@tag_cloud = tag_cloud
set_resource_votes(@resources)
set_resources_instance
@remote_translations = detect_remote_translations(@resources, featured_proposals)
end
def show
set_resource_votes(resource)
@commentable = resource
@comment_tree = CommentTree.new(@commentable, params[:page], @current_order)
set_comment_flags(@comment_tree.comments)
@@ -99,10 +96,6 @@ module CommentableActions
@categories = Tag.category.order(:name)
end
def set_resource_votes(instance)
send("set_#{resource_name}_votes", instance)
end
def index_customization
nil
end

View File

@@ -30,7 +30,6 @@ class DebatesController < ApplicationController
def vote
@debate.register_vote(current_user, params[:value])
set_debate_votes(@debate)
end
def unmark_featured

View File

@@ -2,8 +2,4 @@ class Legislation::BaseController < ApplicationController
include FeatureFlags
feature_flag :legislation
def legislation_proposal_votes(proposals)
@legislation_proposal_votes = current_user ? current_user.legislation_proposal_votes(proposals) : {}
end
end

View File

@@ -124,7 +124,6 @@ class Legislation::ProcessesController < Legislation::BaseController
end
if @process.proposals_phase.started? || current_user&.administrator?
legislation_proposal_votes(@proposals)
render :proposals
else
render :phase_not_open

View File

@@ -19,7 +19,6 @@ class Legislation::ProposalsController < Legislation::BaseController
def show
super
legislation_proposal_votes(@process.proposals)
@document = Document.new(documentable: @proposal)
if request.path != legislation_process_proposal_path(params[:process_id], @proposal)
redirect_to legislation_process_proposal_path(params[:process_id], @proposal),
@@ -39,7 +38,6 @@ class Legislation::ProposalsController < Legislation::BaseController
def vote
@proposal.register_vote(current_user, params[:value])
legislation_proposal_votes(@proposal)
end
private

View File

@@ -37,12 +37,10 @@ class Management::ProposalsController < Management::BaseController
def vote
@follow = Follow.find_or_create_by!(user: current_user, followable: @proposal)
@proposal.register_vote(managed_user, "yes")
set_proposal_votes(@proposal)
end
def print
@proposals = Proposal.send("sort_by_#{@current_order}").for_render.limit(5)
set_proposal_votes(@proposal)
end
private
@@ -66,10 +64,6 @@ class Management::ProposalsController < Management::BaseController
check_verified_user t("management.proposals.alert.unverified_user")
end
def set_proposal_votes(proposals)
@proposal_votes = managed_user ? managed_user.proposal_votes(proposals) : {}
end
def set_comment_flags(comments)
@comment_flags = managed_user ? managed_user.comment_flags(comments) : {}
end

View File

@@ -58,7 +58,6 @@ class ProposalsController < ApplicationController
def vote
@follow = Follow.find_or_create_by!(user: current_user, followable: @proposal)
@proposal.register_vote(current_user, "yes")
set_proposal_votes(@proposal)
end
def retire
@@ -75,7 +74,6 @@ class ProposalsController < ApplicationController
def vote_featured
@follow = Follow.find_or_create_by!(user: current_user, followable: @proposal)
@proposal.register_vote(current_user, "yes")
set_featured_proposal_votes(@proposal)
end
def summary
@@ -118,10 +116,6 @@ class ProposalsController < ApplicationController
Proposal
end
def set_featured_proposal_votes(proposals)
@featured_proposals_votes = current_user ? current_user.proposal_votes(proposals) : {}
end
def discard_draft
@resources = @resources.published
end
@@ -156,7 +150,6 @@ class ProposalsController < ApplicationController
@featured_proposals = Proposal.not_archived.unsuccessful
.sort_by_confidence_score.limit(Setting["featured_proposals_number"])
if @featured_proposals.present?
set_featured_proposal_votes(@featured_proposals)
@resources = @resources.where.not(id: @featured_proposals)
end
end

View File

@@ -13,8 +13,8 @@ module VotesHelper
end
end
def css_classes_for_vote(votes, votable)
case votes[votable.id]
def css_classes_for_vote(votable)
case current_user&.voted_as_when_voted_for(votable)
when true
{ in_favor: "voted", against: "no-voted" }
when false
@@ -23,8 +23,4 @@ module VotesHelper
{ in_favor: "", against: "" }
end
end
def voted_for?(votes, votable)
votes[votable.id]
end
end

View File

@@ -147,21 +147,6 @@ class User < ApplicationRecord
organization? ? organization.name : username
end
def debate_votes(debates)
voted = votes.for_debates(Array(debates).map(&:id))
voted.each_with_object({}) { |v, h| h[v.votable_id] = v.value }
end
def proposal_votes(proposals)
voted = votes.for_proposals(Array(proposals).map(&:id))
voted.each_with_object({}) { |v, h| h[v.votable_id] = v.value }
end
def legislation_proposal_votes(proposals)
voted = votes.for_legislation_proposals(proposals)
voted.each_with_object({}) { |v, h| h[v.votable_id] = v.value }
end
def comment_flags(comments)
comment_flags = flags.for_comments(comments)
comment_flags.each_with_object({}) { |f, h| h[f.flaggable_id] = true }

View File

@@ -1,4 +1,4 @@
<% cache [locale_and_user_status, debate, @debate_votes[debate.id]] do %>
<% cache [locale_and_user_status, debate, current_user&.voted_as_when_voted_for(debate)] do %>
<div id="<%= dom_id(debate) %>" class="debate clear" data-type="debate">
<div class="panel">
<div class="row">

View File

@@ -4,7 +4,7 @@
<div class="small-12 column">
<div class="debate-content">
<% cache [locale_and_user_status,
"index_minimal", debate, @debate_votes[debate.id]] do %>
"index_minimal", debate, current_user&.voted_as_when_voted_for(debate)] do %>
<h3><%= link_to debate.title, debate %></h3>
<% end %>
</div>

View File

@@ -1 +1 @@
<%= render Debates::VotesComponent.new(debate, debate_votes: @debate_votes) %>
<%= render Debates::VotesComponent.new(debate) %>

View File

@@ -3,7 +3,11 @@
<%= render "shared/canonical", href: debate_url(@debate) %>
<% end %>
<% cache [locale_and_user_status(@debate), @debate, @debate.author, Flag.flagged?(current_user, @debate), @debate_votes] do %>
<% cache [locale_and_user_status(@debate),
@debate,
@debate.author,
Flag.flagged?(current_user, @debate),
current_user&.voted_as_when_voted_for(@debate)] do %>
<div class="debate-show">
<div id="<%= dom_id(@debate) %>" class="row">
<div class="small-12 medium-9 column">

View File

@@ -1 +1 @@
<%= render Legislation::Proposals::VotesComponent.new(proposal, legislation_proposal_votes: @legislation_proposal_votes) %>
<%= render Legislation::Proposals::VotesComponent.new(proposal) %>

View File

@@ -21,7 +21,11 @@
</div>
</div>
<% cache [locale_and_user_status(@proposal), @proposal, @proposal.author, Flag.flagged?(current_user, @proposal), @legislation_proposal_votes] do %>
<% cache [locale_and_user_status(@proposal),
@proposal,
@proposal.author,
Flag.flagged?(current_user, @proposal),
current_user&.voted_as_when_voted_for(@proposal)] do %>
<div class="proposal-show legislation-proposal-show">
<div id="<%= dom_id(@proposal) %>" class="row">
<div class="small-12 medium-9 column">

View File

@@ -1,6 +1,6 @@
<div class="supports text-center">
<div class="in-favor">
<% if voted_for?(@featured_proposals_votes, proposal) %>
<% if current_user&.voted_for?(proposal) %>
<div class="supported">
<%= t("proposals.proposal.already_supported") %>
</div>
@@ -29,7 +29,7 @@
<%= render "shared/login_to_vote" %>
<% end %>
<% if voted_for?(@featured_proposals_votes, proposal) %>
<% if current_user&.voted_for?(proposal) %>
<% if setting["twitter_handle"] %>
<div class="share-supported">
<%= render "shared/social_share",

View File

@@ -1 +1 @@
<%= render Proposals::VotesComponent.new(proposal, vote_url: vote_url, proposal_votes: @proposal_votes) %>
<%= render Proposals::VotesComponent.new(proposal, vote_url: vote_url) %>

View File

@@ -9,7 +9,11 @@
<%= render "shared/canonical", href: proposal_url(@proposal) %>
<% end %>
<% cache [locale_and_user_status(@proposal), @proposal, @proposal.author, Flag.flagged?(current_user, @proposal), @proposal_votes] do %>
<% cache [locale_and_user_status(@proposal),
@proposal,
@proposal.author,
Flag.flagged?(current_user, @proposal),
current_user&.voted_for?(@proposal)] do %>
<div id="<%= dom_id(@proposal) %>" class="row">
<div class="small-12 medium-8 column">

View File

@@ -18,7 +18,7 @@
@proposal.author,
Flag.flagged?(current_user, @proposal),
@proposal.followed_by?(current_user),
@proposal_votes] do %>
current_user&.voted_for?(@proposal)] do %>
<div class="proposal-show">
<div id="<%= dom_id(@proposal) %>" class="row">
<div class="small-12 medium-9 column">

View File

@@ -1,22 +1,6 @@
require "rails_helper"
describe VotesHelper do
describe "#voted_for?" do
it "returns true if voted for a proposal" do
proposal = create(:proposal)
votes = { proposal.id => true }
expect(voted_for?(votes, proposal)).to eq(true)
end
it "returns false if not voted for a proposals" do
proposal = create(:proposal)
votes = { proposal.id => nil }
expect(voted_for?(votes, proposal)).to eq(nil)
end
end
describe "#votes_percentage" do
it "alwayses sum 100%" do
debate = create(:debate)

View File

@@ -33,28 +33,6 @@ describe User do
end
end
describe "#debate_votes" do
let(:user) { create(:user) }
it "returns {} if no debate" do
expect(user.debate_votes([])).to eq({})
end
it "returns a hash of debates ids and votes" do
debate1 = create(:debate)
debate2 = create(:debate)
debate3 = create(:debate)
create(:vote, voter: user, votable: debate1, vote_flag: true)
create(:vote, voter: user, votable: debate3, vote_flag: false)
voted = user.debate_votes([debate1, debate2, debate3])
expect(voted[debate1.id]).to eq(true)
expect(voted[debate2.id]).to eq(nil)
expect(voted[debate3.id]).to eq(false)
end
end
describe "#comment_flags" do
let(:user) { create(:user) }