Order translations using ruby

Joining the translations table caused duplicate records to appear.
Ordering with SQL is simply too hard because we need to consider
fallback locales.

Thanks Senén for providing most of the tests in the poll spec.

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)
-                              .sort_for_list.page(params[:page])
+    @polls = Kaminari.paginate_array(
+      @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)
-                                           .where(id: investments.pluck(:heading_id).uniq)
-                                           .order(name: :asc)
+      investment_headings = Budget::Heading.where(id: investments.pluck(:heading_id)).sort_by(&:name)
 
       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,  -> { i18n.where(allow_custom_content: true).order("budget_heading_translations.name") }
+    scope :allow_custom_content,  -> { where(allow_custom_content: true).sort_by(&: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/user.rb

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