From 958d3732476e762dca9696984dc9ebb45142f529 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 8 Apr 2020 14:36:04 +0200
Subject: [PATCH] Fix duplicate records in investments by tag

When an investment had been assigned a user tag and a valuation tag with
the same name, it appeared twice when filtering by tag.

This is because by design, in order to provide compatibility with scopes
using "select" or "distinct", the method `tagged_with` doesn't select
unique records.

Forcing the query to return unique records solves the issue.
---
 app/helpers/budget_executions_helper.rb       |  2 +-
 app/models/budget/investment.rb               |  2 +-
 spec/helpers/budget_executions_helper_spec.rb | 17 +++++++++++++++++
 spec/models/budget/investment_spec.rb         |  8 ++++++++
 4 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 spec/helpers/budget_executions_helper_spec.rb

diff --git a/app/helpers/budget_executions_helper.rb b/app/helpers/budget_executions_helper.rb
index 182fd3fe4..3cfab0557 100644
--- a/app/helpers/budget_executions_helper.rb
+++ b/app/helpers/budget_executions_helper.rb
@@ -5,7 +5,7 @@ module BudgetExecutionsHelper
 
   def options_for_milestone_tags
     @budget.investments_milestone_tags.map do |tag|
-      ["#{tag} (#{@budget.investments.winners.tagged_with(tag).count})", tag]
+      ["#{tag} (#{@budget.investments.winners.by_tag(tag).count})", tag]
     end
   end
 
diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb
index a3d1bd23e..d685b8507 100644
--- a/app/models/budget/investment.rb
+++ b/app/models/budget/investment.rb
@@ -98,7 +98,7 @@ class Budget
     scope :by_group,          ->(group_id)    { where(group_id: group_id) }
     scope :by_heading,        ->(heading_id)  { where(heading_id: heading_id) }
     scope :by_admin,          ->(admin_id)    { where(administrator_id: admin_id) }
-    scope :by_tag,            ->(tag_name)    { tagged_with(tag_name) }
+    scope :by_tag,            ->(tag_name)    { tagged_with(tag_name).distinct }
 
     scope :for_render, -> { includes(:heading) }
 
diff --git a/spec/helpers/budget_executions_helper_spec.rb b/spec/helpers/budget_executions_helper_spec.rb
new file mode 100644
index 000000000..80a01a423
--- /dev/null
+++ b/spec/helpers/budget_executions_helper_spec.rb
@@ -0,0 +1,17 @@
+require "rails_helper"
+
+describe BudgetExecutionsHelper do
+  describe "#options_for_milestone_tags" do
+    let(:budget) { create(:budget) }
+
+    it "does not return duplicate records for tags in different contexts" do
+      create(:budget_investment, :winner, budget: budget, milestone_tag_list: ["Multiple"])
+      create(:budget_investment, :winner, budget: budget, milestone_tag_list: ["Multiple"])
+      create(:budget_investment, :winner, budget: budget, milestone_tag_list: ["Dup"], tag_list: ["Dup"])
+
+      @budget = budget
+
+      expect(options_for_milestone_tags).to eq [["Dup (1)", "Dup"], ["Multiple (2)", "Multiple"]]
+    end
+  end
+end
diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb
index d2ccb9415..f358584c3 100644
--- a/spec/models/budget/investment_spec.rb
+++ b/spec/models/budget/investment_spec.rb
@@ -796,6 +796,14 @@ describe Budget::Investment do
 
         expect(investment.valuation_tag_list).to match_array(%w[Code Test Refactor])
       end
+
+      describe ".by_tag" do
+        it "does not return duplicate records for tags in different contexts" do
+          investment = create(:budget_investment, tag_list: ["Same"], valuation_tag_list: ["Same"])
+
+          expect(Budget::Investment.by_tag("Same")).to eq [investment]
+        end
+      end
     end
   end