Decouple the liquidvoting state object

Gemfile.lock

@@ -522,7 +522,9 @@ GEM
     mime-types (3.3.1)
       mime-types-data (~> 3.2015)
     mime-types-data (3.2021.0225)
-    mimemagic (0.3.5)
+    mimemagic (0.3.10)
+      nokogiri (~> 1)
+      rake
     mini_magick (4.11.0)
     mini_mime (1.0.2)
     mini_portile2 (2.5.0)

app/controllers/decidim/liquidvoting/proposal_vote_delegations_controller.rb

@@ -10,7 +10,7 @@ class ProposalVoteDelegationsController < Decidim::Proposals::ApplicationControl
       def create
         enforce_permission_to :vote, :proposal, proposal: proposal
 
-        Decidim::Liquidvoting::Client.create_delegation(
+        Decidim::Liquidvoting::ApiClient.create_delegation(
           proposal_url: proposal_locator.url,
           delegator_email: delegator_email,
           delegate_email: params[:delegate_email]
@@ -19,7 +19,7 @@ def create
         @from_proposals_list = params[:from_proposals_list] == "true"
         @proposals = [] + [proposal]
 
-        @lv_state = Decidim::Liquidvoting::Client.current_proposal_state(delegator_email, proposal_locator.url)
+        @lv_state = Liquidvoting.user_proposal_state(delegator_email, proposal_locator.url)
         render "decidim/proposals/proposal_votes/update_buttons_and_counters"
       end
 
@@ -32,7 +32,7 @@ def index
       def destroy
         enforce_permission_to :unvote, :proposal, proposal: proposal
 
-        Decidim::Liquidvoting::Client.delete_delegation(
+        Decidim::Liquidvoting::ApiClient.delete_delegation(
           proposal_url: proposal_locator.url,
           delegator_email: delegator_email,
           delegate_email: params[:delegate_email]
@@ -41,7 +41,7 @@ def destroy
         @from_proposals_list = params[:from_proposals_list] == "true"
         @proposals = [] + [proposal]
 
-        @lv_state = Decidim::Liquidvoting::Client.current_proposal_state(delegator_email, proposal_locator.url)
+        @lv_state = Liquidvoting.user_proposal_state(delegator_email, proposal_locator.url)
         render "decidim/proposals/proposal_votes/update_buttons_and_counters"
       end
 

app/overrides/commands/decidim/proposals/unvote_proposal.rb

@@ -21,13 +21,13 @@ def initialize(proposal, current_user)
       #
       # Returns nothing.
       def call
-        response = Decidim::Liquidvoting::Client.delete_vote(
+        response = Decidim::Liquidvoting::ApiClient.delete_vote(
           proposal_url: ResourceLocatorPresenter.new(@proposal).url,
           participant_email: current_user.email
         )
 
         new_vote_count = response.voting_result&.in_favor
-        @proposal.update_with_lv_vote_count(new_vote_count)
+        Liquidvoting.update_votes_count(@proposal, new_vote_count)
 
         broadcast(:ok, @proposal)
       end

app/overrides/commands/decidim/proposals/vote_proposal.rb

@@ -28,14 +28,14 @@ def call
         build_proposal_vote
         return broadcast(:invalid) unless vote.valid?
 
-        response = Decidim::Liquidvoting::Client.create_vote(
+        response = Decidim::Liquidvoting::ApiClient.create_vote(
           proposal_url: ResourceLocatorPresenter.new(@proposal).url,
           participant_email: current_user.email,
           yes: true
         )
 
         new_vote_count = response.voting_result&.in_favor
-        @proposal.update_with_lv_vote_count(new_vote_count)
+        Liquidvoting.update_votes_count(@proposal, new_vote_count)
 
         broadcast(:ok, vote)
       end

app/overrides/controllers/decidim/proposals/proposal_votes_controller.rb

@@ -62,8 +62,7 @@ def proposal
       end
 
       def lv_state
-        # don't memoize, always get a fresh one
-        @lv_state = Decidim::Liquidvoting::Client.current_proposal_state(
+        @lv_state = Liquidvoting.user_proposal_state(
           current_user&.email,
           ResourceLocatorPresenter.new(proposal).url
         )

app/overrides/controllers/decidim/proposals/proposals_controller.rb

@@ -48,7 +48,7 @@ def index
       def show
         raise ActionController::RoutingError, "Not Found" if @proposal.blank? || !can_show_proposal?
 
-        @lv_state = Decidim::Liquidvoting::Client.current_proposal_state(current_user&.email, proposal_url)
+        @lv_state = Liquidvoting.user_proposal_state(current_user&.email, proposal_url)
         @report_form = form(Decidim::ReportForm).from_params(reason: "spam")
       end
 

app/overrides/models/decidim/proposals/proposal.rb

@@ -1,26 +1,22 @@
 # frozen_string_literal: true
 
-# Liquidvoting repurposes the :proposal_votes_count attribute to carry the current vote
-# count from the Liquidvoting external service.
-#
-# To repurpose, we override the existing :update_votes_count method to no longer refresh the count
-# from the :votes association, and to log the (unexpected) event. This can happen when other Decidim
-# code, for example seeding the database, calls the method. The :votes association and ProposalVote
-# model are considered abandoned.
+# Liquidvoting repurposes the :proposal_votes_count attribute in this model to carry instead
+# the current vote count from the Liquidvoting external service, and not a cached
+# count from the Proposal#votes association.
+
+# Because we are bypassing the Proposal#votes association and ProposalVote model, we don't
+# really expect this :update_votes_count method to be called. However, it can happen, as in
+# the rake db:seeds. So we've overridden the method to log the occurrence.
 #
-# We also add an explicit :update_with_lv_vote_count method that will set the attribute with the current
-# Liquidvoting count, and also log the event. This is the expected way to manage the attribute.
+# See the Decidim::Liquidvoting.update_votes_count for the canonical way to update vote counts.
 #
 Decidim::Proposals::Proposal.class_eval do
+  # rubocop:disable Rails/SkipsModelValidations
   def update_votes_count
-    msg = "TRACE: Surprise :update_votes_count call; Liquidvoting uses :update_with_lv_vote_count"
+    msg = "TRACE: Surprise :update_votes_count call; see Decidim::Liquidvoting.update_votes_count"
     Decidim::Liquidvoting::Logger.info msg
-  end
 
-  # rubocop:disable Rails/SkipsModelValidations
-  def update_with_lv_vote_count(lv_count)
-    update_columns(proposal_votes_count: lv_count)
-    Decidim::Liquidvoting::Logger.info "TRACE: Liquidvoting set the proposal_votes_count to #{lv_count.inspect}"
+    update_columns(proposal_votes_count: votes.count) # IDK why `super` doesn't work here
   end
   # rubocop:enable Rails/SkipsModelValidations
 end

app/views/decidim/liquidvoting/_delegation.html.erb

@@ -1,6 +1,6 @@
 <div id="proposal-<%= proposal.id %>-delegate-ui" class="button--vote-button">
 <% if current_user %>
-  <% if @lv_state.user_has_voted        # VOTED so disable delegation UI %>
+  <% if @lv_state.user_has_supported    # VOTED so disable delegation UI %>
     <%= content_tag :button, t("decidim.proposals.proposals.delegate_button.delegate"), class: "button #{vote_button_classes(from_proposals_list)} disabled", disabled: true %>
   <% elsif @lv_state.delegate_email     # NOT VOTED BUT DELEGATED so present undelegation UI -%>
     <%= t("decidim.proposals.proposals.delegation_ui.to") %>: <br>

app/views/decidim/proposals/proposals/_vote_button.html.erb

@@ -17,7 +17,7 @@
     <% elsif @lv_state.delegate_email %>
       <%= content_tag :button, t("decidim.proposals.proposals.vote_button.vote_delegated"), class: "button #{vote_button_classes(from_proposals_list)} disabled", id: "vote_button-#{proposal.id}", disabled: true %>
     <% else %>
-      <% if @lv_state.user_has_voted %>
+      <% if @lv_state.user_has_supported %>
         <%= action_authorized_button_to(
           :vote,
           proposal_proposal_vote_path(proposal_id: proposal, from_proposals_list: from_proposals_list),

lib/decidim/liquidvoting.rb

@@ -3,12 +3,29 @@
 require "decidim/liquidvoting/admin"
 require "decidim/liquidvoting/admin_engine"
 require "decidim/liquidvoting/engine"
-require "decidim/liquidvoting/client"
+require "decidim/liquidvoting/api_client"
 require "decidim/liquidvoting/logger"
 
 module Decidim
   # This namespace holds the logic of the `Liquidvoting` module
   module Liquidvoting
+    # rubocop:disable Rails/SkipsModelValidations
+    def self.update_votes_count(proposal, new_count)
+      proposal.update_columns(proposal_votes_count: new_count)
+
+      msg = "TRACE: Liquidvoting.update_votes_count set #{new_count.inspect} for proposal id=#{proposal.id}"
+      Decidim::Liquidvoting::Logger.info msg
+    end
+    # rubocop:enable Rails/SkipsModelValidations
+
+    UserProposalState = Struct.new(:user_has_supported, :delegate_email)
+
+    def self.user_proposal_state(user_email, proposal_url)
+      user_has_supported = Decidim::Liquidvoting::ApiClient.fetch_user_supported?(user_email, proposal_url)
+      delegate_email = Decidim::Liquidvoting::ApiClient.fetch_delegate_email(user_email, proposal_url)
+
+      UserProposalState.new(user_has_supported, delegate_email)
+    end
   end
 end
 

lib/decidim/liquidvoting/client.rb → lib/decidim/liquidvoting/api_client.rb

@@ -5,15 +5,13 @@
 
 module Decidim
   module Liquidvoting
-    ProposalState = Struct.new(:user_has_voted, :delegate_email)
-
     # Copied over from https://github.com/liquidvotingio/ruby-client/blob/master/liquid_voting_api.rb.
     # Changes here will be applied there as well. Doing this for development speed, until
     # basics are ironed out and we can we publish the client as a gem.
 
     # This client integrates with the liquidvoting.io api, allowing for delegative voting
     # in a participatory space proposal.
-    module Client
+    module ApiClient
       URL = ENV.fetch("LIQUID_VOTING_API_URL", "http://localhost:4000")
       # URL = ENV.fetch('LIQUID_VOTING_API_URL', 'https://api.liquidvoting.io')
       AUTH_KEY = ENV.fetch("LIQUID_VOTING_API_AUTH_KEY", "62309201-d2f0-407f-875b-9f836f94f2ca")
@@ -31,19 +29,12 @@ def headers(_context)
       SCHEMA = ::GraphQL::Client.load_schema(HTTP)
       CLIENT = ::GraphQL::Client.new(schema: SCHEMA, execute: HTTP)
 
-      ## Return a snapshot of current Liquidvoting state for the given user and proposal.
-      ##
-      ## The intent is to encapsulate all of the LV state relevant to a user and a specific proposal
-      ## in a single LV state object, to give controllers a simple way to acquire that object, and
-      ## to make that state available throughout the duration of the web request.
-      ##
-      ## As a ruby Struct, the object is immutable; the best way to refresh the state is to
-      ## reacquire this state object.
-      def self.current_proposal_state(participant_email, proposal_url)
-        user_has_voted = user_voted?(participant_email, proposal_url)
-        delegate_email = delegate_email_for(participant_email, proposal_url)
+      def self.fetch_user_supported?(user_email, proposal_url)
+        user_voted?(user_email, proposal_url)
+      end
 
-        ProposalState.new(user_has_voted, delegate_email)
+      def self.fetch_delegate_email(user_email, proposal_url)
+        delegate_email_for(user_email, proposal_url)
       end
 
       ## Example:
@@ -133,38 +124,30 @@ def self.delete_delegation(proposal_url:, delegator_email:, delegate_email:)
         CLIENT.query(query, variables: variables)
       end
 
-      private_class_method def self.votes
-        response = send_query(VotesQuery)
-
-        raise response.data.errors.messages["votes"].join(", ") if response.data.errors.any?
-
-        response.data.votes
-      end
-
+      # This method is a hack until we can properly query a subset of votes;
+      # it currently retrieves ALL votes in LV and then filters!
       private_class_method def self.user_voted?(participant_email, proposal_url)
         return false unless participant_email.present? && proposal_url.present?
 
-        # this is a hack until we can properly query a subset of delegations
-        vote = votes.find do |v|
+        api_response = send_query(VotesQuery)
+        raise api_response.data.errors.messages["votes"].join(", ") if api_response.data.errors.any?
+
+        vote = api_response.data.votes.find do |v|
           v.participant.email == participant_email && v.proposal_url == proposal_url
         end
 
         !!vote
       end
 
-      private_class_method def self.delegations
-        response = send_query(DelegationsQuery)
-
-        raise response.data.errors.messages["delegations"].join(", ") if response.data.errors.any?
-
-        response.data.delegations
-      end
-
+      # This method is a hack until we can properly query a subset of delegations;
+      # it currently retrieves ALL delegations in LV and then filters!
       private_class_method def self.delegate_email_for(delegator_email, proposal_url)
         return unless delegator_email.present? && proposal_url.present?
 
-        # this is a hack until we can properly query a subset of delegations
-        delegation = delegations.find do |d|
+        api_response = send_query(DelegationsQuery)
+        raise api_response.data.errors.messages["delegations"].join(", ") if api_response.data.errors.any?
+
+        delegation = api_response.data.delegations.find do |d|
           d.delegator.email == delegator_email && d.proposal_url == proposal_url
         end
 

spec/lib/decidim/liquidvoting_spec.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+describe Decidim::Liquidvoting do
+  subject { Decidim::Liquidvoting }
+
+  describe "#update_votes_count" do
+    let(:proposal) { create(:proposal) }
+    let(:new_vote_count) { 35 }
+    let(:expected_msg) { "TRACE: Liquidvoting.update_votes_count set #{new_vote_count} for proposal id=#{proposal.id}" }
+
+    it "logs the unexpected call" do
+      expect(Decidim::Liquidvoting::Logger).to receive(:info).with(expected_msg)
+
+      subject.update_votes_count(proposal, new_vote_count)
+    end
+  end
+
+  describe "#user_proposal_state" do
+    let(:user) { create(:user) }
+    let(:delegate) { create(:user) }
+
+    before do
+      # stub API
+      allow(Decidim::Liquidvoting::ApiClient).to receive(:fetch_user_supported?).and_return(true)
+      allow(Decidim::Liquidvoting::ApiClient).to receive(:fetch_delegate_email).and_return(delegate.email)
+    end
+
+    it "includes :user_has_supported" do
+      lv_state = Decidim::Liquidvoting.user_proposal_state(user.email, "https://url_1")
+
+      expect(lv_state.user_has_supported).to be(true)
+    end
+
+    it "includes :delegate_email" do
+      lv_state = Decidim::Liquidvoting.user_proposal_state(user.email, "https://url_1")
+
+      expect(lv_state.delegate_email).to eq(delegate.email)
+    end
+  end
+end

spec/models/decidim/proposals/proposal_spec.rb

@@ -7,35 +7,23 @@
 
   it { is_expected.to be_valid }
 
-  describe "#update_votes_count (no-op stubbed Decidim method)" do
-    it "logs the unexpected call" do
-      expect(Decidim::Liquidvoting::Logger).to receive(:info).with(/TRACE: Surprise/)
+  describe "#update_votes_count (LV extending Decidim method)" do
+    context "when something updates the vote count attribute" do
+      let(:votes_in_proposal_votes) { 3 }
 
-      subject.update_votes_count
-    end
-  end
-
-  describe "#update_with_lv_vote_count" do
-    let(:new_vote_count) { 35 }
-
-    context "when we update the vote count" do
       before do
-        subject.update_with_lv_vote_count(new_vote_count)
-        subject.reload
+        create_list(:proposal_vote, votes_in_proposal_votes, proposal: subject)
       end
 
-      it "is updated" do
-        expect(subject.proposal_votes_count).to eq(new_vote_count)
-      end
+      it "logs the unexpected call" do
+        expect(Decidim::Liquidvoting::Logger).to receive(:info).with(/TRACE: Surprise/)
 
-      it "is independent of the :votes association" do
-        expect(subject.votes.count).to eq(0)
+        subject.update_votes_count
       end
 
-      it "logs the call" do
-        expect(Decidim::Liquidvoting::Logger).to receive(:info).with(/TRACE: Liquidvoting set/)
-
-        subject.update_with_lv_vote_count(new_vote_count)
+      it "is updated" do
+        # subject.update_votes_count unneeded, because it's done in ProposalVote :after_save
+        expect(subject.proposal_votes_count).to eq(votes_in_proposal_votes)
       end
     end
   end