Merge pull request #3581 from consul/duplicate_translations

Avoid duplicate records using translations

app/controllers/polls_controller.rb

@@ -12,8 +12,9 @@ class PollsController < ApplicationController
   ::Poll::Answer # trigger autoload
 
   def index
-    @polls = @polls.not_budget.public_polls.send(@current_filter).includes(:geozones)
+    @polls = Kaminari.paginate_array(
-                              .sort_for_list.page(params[:page])
+      @polls.public_polls.not_budget.send(@current_filter).includes(:geozones).sort_for_list
+    ).page(params[:page])
   end
 
   def show

app/controllers/valuation/budget_investments_controller.rb

@@ -75,9 +75,7 @@ class Valuation::BudgetInvestmentsController < Valuation::BaseController
     def heading_filters
       investments = @budget.investments.by_valuator(current_user.valuator.try(:id))
                                        .visible_to_valuators.distinct
-      investment_headings = Budget::Heading.joins(:translations)
+      investment_headings = Budget::Heading.where(id: investments.pluck(:heading_id)).sort_by(&:name)
-                                           .where(id: investments.pluck(:heading_id).uniq)
-                                           .order(name: :asc)
 
       all_headings_filter = [
                               {

app/models/budget/group.rb

@@ -28,7 +28,9 @@ class Budget
     validates :budget_id, presence: true
     validates :slug, presence: true, format: /\A[a-z0-9\-_]+\z/
 
-    scope :sort_by_name, -> { joins(:translations).order(:name) }
+    def self.sort_by_name
+      all.sort_by(&:name)
+    end
 
     def single_heading_group?
       headings.count == 1

app/models/budget/heading.rb

@@ -40,8 +40,7 @@ class Budget
 
     delegate :budget, :budget_id, to: :group, allow_nil: true
 
-    scope :i18n,                  -> { joins(:translations) }
+    scope :allow_custom_content,  -> { where(allow_custom_content: true).sort_by(&:name) }
-    scope :allow_custom_content,  -> { i18n.where(allow_custom_content: true).order("budget_heading_translations.name") }
 
     def self.sort_by_name
       all.sort do |heading, other_heading|

app/models/poll.rb

@@ -49,7 +49,19 @@ class Poll < ApplicationRecord
   scope :not_budget,    -> { where(budget_id: nil) }
   scope :created_by_admin, -> { where(related_type: nil) }
 
-  scope :sort_for_list, -> { joins(:translations).order(:geozone_restricted, :starts_at, "poll_translations.name") }
+  def self.sort_for_list
+    all.sort do |poll, another_poll|
+      if poll.geozone_restricted? == another_poll.geozone_restricted?
+        [poll.starts_at, poll.name] <=> [another_poll.starts_at, another_poll.name]
+      else
+        if poll.geozone_restricted?
+          1
+        else
+          -1
+        end
+      end
+    end
+  end
 
   def self.overlaping_with(poll)
     where("? < ends_at and ? >= starts_at", poll.starts_at.beginning_of_day,

app/models/poll/booth.rb

@@ -12,7 +12,7 @@ class Poll
     end
 
     def self.available
-      where(polls: { id: Poll.current_or_recounting }).joins(polls: :translations)
+      where(polls: { id: Poll.current_or_recounting }).joins(:polls)
     end
 
     def assignment_on_poll(poll)

app/models/user.rb

@@ -131,9 +131,7 @@ class User < ApplicationRecord
   end
 
   def headings_voted_within_group(group)
-    Budget::Heading.joins(:translations)
+    Budget::Heading.where(id: voted_investments.by_group(group).pluck(:heading_id))
-                   .order("name")
-                   .where(id: voted_investments.by_group(group).pluck(:heading_id))
   end
 
   def voted_investments

app/views/budgets/investments/_votes.html.erb

@@ -36,7 +36,7 @@
                 verify_account: link_to(t("votes.verify_account"), verification_path),
                 signin: link_to(t("votes.signin"), new_user_session_path),
                 signup: link_to(t("votes.signup"), new_user_registration_path),
-                supported_headings: (current_user && current_user.headings_voted_within_group(investment.group).map(&:name).to_sentence)
+                supported_headings: (current_user && current_user.headings_voted_within_group(investment.group).map(&:name).sort.to_sentence)
                ).html_safe %>
         </small>
       </p>

spec/models/budget/group_spec.rb

@@ -32,4 +32,42 @@ describe Budget::Group do
     end
 
   end
+
+  describe "#sort_by_name" do
+    it "returns groups sorted by name ASC" do
+      thinkers = create(:budget_group, name: "Mmmm...")
+      sleepers = create(:budget_group, name: "Zzz...")
+      startled = create(:budget_group, name: "Aaaaah!")
+
+      expect(Budget::Group.sort_by_name).to eq [startled, thinkers, sleepers]
+    end
+
+    it "returns groups with multiple translations only once" do
+      create(:budget_group, name_en: "English", name_es: "Spanish")
+
+      expect(Budget::Group.sort_by_name.count).to eq 1
+    end
+
+    context "fallback locales" do
+      before do
+        allow(I18n.fallbacks).to receive(:[]).and_return([:es])
+        Globalize.set_fallbacks_to_all_available_locales
+      end
+
+      it "orders by name considering fallback locale," do
+        budget = create(:budget, name: "Teams")
+        charlie = create(:budget_group, budget: budget, name: "Charlie")
+        delta = create(:budget_group, budget: budget, name: "Delta")
+        zulu = Globalize.with_locale(:es) do
+          create(:budget_group, budget: budget, name: "Zulu", name_fr: "Alpha")
+        end
+        bravo = Globalize.with_locale(:es) do
+          create(:budget_group, budget: budget, name: "Bravo")
+        end
+
+        expect(Budget::Group.sort_by_name.count).to eq 4
+        expect(Budget::Group.sort_by_name).to eq [bravo, charlie, delta, zulu]
+      end
+    end
+  end
 end

spec/models/budget/heading_spec.rb

@@ -319,6 +319,12 @@ describe Budget::Heading do
       expect(Budget::Heading.allow_custom_content.first).to eq first_heading
       expect(Budget::Heading.allow_custom_content.last).to eq last_heading
     end
+
+    it "returns headings with multiple translations only once" do
+      create(:budget_heading, allow_custom_content: true, name_en: "English", name_es: "Spanish")
+
+      expect(Budget::Heading.allow_custom_content.count).to eq 1
+    end
   end
 
 end

spec/models/poll/booth_spec.rb

@@ -24,7 +24,7 @@ describe Poll::Booth do
     end
   end
 
-  describe "#available" do
+  describe ".available" do
 
     it "returns booths associated to current polls" do
       booth_for_current_poll  = create(:poll_booth)
@@ -40,5 +40,10 @@ describe Poll::Booth do
       expect(described_class.available).not_to include(booth_for_expired_poll)
     end
 
+    it "returns polls with multiple translations only once" do
+      create(:poll_booth, polls: [create(:poll, :current, name: "English", name_es: "Spanish")])
+
+      expect(Poll::Booth.available.count).to eq 1
+    end
   end
 end

spec/models/poll/poll_spec.rb

@@ -359,4 +359,58 @@ describe Poll do
 
   end
 
+  describe "#sort_for_list" do
+    it "returns polls sorted by name ASC" do
+      starts_at = Time.current + 1.day
+      poll1 = create(:poll, geozone_restricted: true, starts_at: starts_at, name: "Zzz...")
+      poll2 = create(:poll, geozone_restricted: true, starts_at: starts_at, name: "Mmmm...")
+      poll3 = create(:poll, geozone_restricted: true, starts_at: starts_at, name: "Aaaaah!")
+
+      expect(Poll.sort_for_list).to eq [poll3, poll2, poll1]
+    end
+
+    it "returns not geozone restricted polls first" do
+      starts_at = Time.current + 1.day
+      poll1 = create(:poll, geozone_restricted: false, starts_at: starts_at, name: "Zzz...")
+      poll2 = create(:poll, geozone_restricted: true, starts_at: starts_at, name: "Aaaaaah!")
+
+      expect(Poll.sort_for_list).to eq [poll1, poll2]
+    end
+
+    it "returns polls earlier to start first" do
+      starts_at = Time.current + 1.day
+      poll1 = create(:poll, geozone_restricted: false, starts_at: starts_at - 1.hour, name: "Zzz...")
+      poll2 = create(:poll, geozone_restricted: false, starts_at: starts_at, name: "Aaaaah!")
+
+      expect(Poll.sort_for_list).to eq [poll1, poll2]
+    end
+
+    it "returns polls with multiple translations only once" do
+      create(:poll, name_en: "English", name_es: "Spanish")
+
+      expect(Poll.sort_for_list.count).to eq 1
+    end
+
+    context "fallback locales" do
+      before do
+        allow(I18n.fallbacks).to receive(:[]).and_return([:es])
+        Globalize.set_fallbacks_to_all_available_locales
+      end
+
+      it "orders by name considering fallback locales" do
+        starts_at = Time.current + 1.day
+        poll1 = create(:poll, starts_at: starts_at, name: "Charlie")
+        poll2 = create(:poll, starts_at: starts_at, name: "Delta")
+        poll3 = Globalize.with_locale(:es) do
+          create(:poll, starts_at: starts_at, name: "Zzz...", name_fr: "Aaaah!")
+        end
+        poll4 = Globalize.with_locale(:es) do
+          create(:poll, starts_at: starts_at, name: "Bravo")
+        end
+
+        expect(Poll.sort_for_list.count).to eq 4
+        expect(Poll.sort_for_list).to eq [poll4, poll1, poll2, poll3]
+      end
+    end
+  end
 end

spec/models/user_spec.rb

@@ -3,7 +3,7 @@ require "rails_helper"
 describe User do
 
   describe "#headings_voted_within_group" do
-    it "returns the headings voted by a user ordered by name" do
+    it "returns the headings voted by a user" do
       user1 = create(:user)
       user2 = create(:user)
 
@@ -35,6 +35,15 @@ describe User do
       expect(user2.headings_voted_within_group(group)).not_to include(san_franciso)
       expect(user2.headings_voted_within_group(group)).not_to include(another_heading)
     end
+
+    it "returns headings with multiple translations only once" do
+      user = create(:user)
+      group = create(:budget_group)
+      heading = create(:budget_heading, group: group, name_en: "English", name_es: "Spanish")
+      create(:vote, votable: create(:budget_investment, heading: heading), voter: user)
+
+      expect(user.headings_voted_within_group(group).count).to eq 1
+    end
   end
 
   describe "#debate_votes" do