Merge 366b940 into 7b30ab7

app/components/valuation/budgets/index_component.html.erb

@@ -0,0 +1,21 @@
+<h2 class="inline-block"><%= t("valuation.budgets.index.title") %></h2>
+
+<% if budgets.any? %>
+  <table>
+    <thead>
+      <tr>
+        <th><%= t("valuation.budgets.index.table_name") %></th>
+        <th><%= t("valuation.budgets.index.table_phase") %></th>
+        <th><%= t("valuation.budgets.index.table_assigned_investments_valuation_open") %></th>
+        <th><%= t("valuation.budgets.index.table_actions") %></th>
+      </tr>
+    </thead>
+    <tbody>
+      <%= render Valuation::Budgets::RowComponent.with_collection(budgets) %>
+    </tbody>
+  </table>
+<% else %>
+  <div class="callout primary clear">
+    <%= t("valuation.budgets.index.no_budgets") %>
+  </div>
+<% end %>

app/components/valuation/budgets/index_component.rb

@@ -0,0 +1,7 @@
+class Valuation::Budgets::IndexComponent < ApplicationComponent
+  attr_reader :budgets
+
+  def initialize(budgets)
+    @budgets = budgets
+  end
+end

app/components/valuation/budgets/row_component.html.erb

@@ -0,0 +1,18 @@
+<tr id="<%= dom_id(budget) %>" class="budget">
+  <td>
+    <%= budget.name %>
+  </td>
+  <td>
+    <%= budget.current_phase.name %>
+  </td>
+  <td class="investments-count">
+    <%= investments.count %>
+  </td>
+  <td>
+    <% if investments.any? && budget.valuating? %>
+      <%= link_to t("valuation.budgets.index.evaluate"),
+                  valuation_budget_budget_investments_path(budget_id: budget.id),
+                  class: "button hollow expanded" %>
+    <% end %>
+  </td>
+</tr>

app/components/valuation/budgets/row_component.rb

@@ -0,0 +1,16 @@
+class Valuation::Budgets::RowComponent < ApplicationComponent
+  attr_reader :budget
+  with_collection_parameter :budget
+
+  delegate :current_user, to: :helpers
+
+  def initialize(budget:)
+    @budget = budget
+  end
+
+  def investments
+    return Budget::Investment.none unless budget.valuating?
+
+    budget.investments.visible_to_valuators.by_valuator(current_user.valuator).valuation_open
+  end
+end

app/controllers/valuation/budget_investments_controller.rb

@@ -4,9 +4,9 @@ class Valuation::BudgetInvestmentsController < Valuation::BaseController
 
   feature_flag :budgets
 
+  before_action :load_budget
   before_action :restrict_access_to_assigned_items, only: [:show, :edit, :valuate]
   before_action :restrict_access, only: [:edit, :valuate]
-  before_action :load_budget
   before_action :load_investment, only: [:show, :edit, :valuate]
 
   has_orders %w[oldest], only: [:show, :edit]
@@ -110,7 +110,7 @@ def allowed_params
     end
 
     def restrict_access
-      unless current_user.administrator? || current_budget.valuating?
+      unless current_user.administrator? || @budget.valuating?
         raise CanCan::AccessDenied, I18n.t("valuation.budget_investments.not_in_valuating_phase")
       end
     end

app/controllers/valuation/budgets_controller.rb

@@ -5,9 +5,6 @@ class Valuation::BudgetsController < Valuation::BaseController
   load_and_authorize_resource
 
   def index
-    @budget = current_budget
-    if @budget.present?
-      @investments = @budget.investments.by_valuator(current_user.valuator).valuation_open
-    end
+    @budgets = @budgets.published.order(created_at: :desc)
   end
 end

app/views/valuation/budgets/index.html.erb

@@ -1,36 +1 @@
-<h2 class="inline-block"><%= t("valuation.budgets.index.title") %></h2>
-
-<% if @budget.present? %>
-  <table>
-    <thead>
-      <tr>
-        <th><%= t("valuation.budgets.index.table_name") %></th>
-        <th><%= t("valuation.budgets.index.table_phase") %></th>
-        <th><%= t("valuation.budgets.index.table_assigned_investments_valuation_open") %></th>
-        <th><%= t("valuation.budgets.index.table_actions") %></th>
-      </tr>
-    </thead>
-    <tbody>
-      <tr id="<%= dom_id(@budget) %>" class="budget">
-        <td>
-          <%= @budget.name %>
-        </td>
-        <td>
-          <%= @budget.current_phase.name %>
-        </td>
-        <td>
-          <%= @investments.count %>
-        </td>
-        <td>
-          <%= link_to t("valuation.budgets.index.evaluate"),
-                      valuation_budget_budget_investments_path(budget_id: @budget.id),
-                      class: "button hollow expanded" %>
-        </td>
-      </tr>
-    </tbody>
-  </table>
-<% else %>
-  <div class="callout primary clear">
-    <%= t("valuation.budgets.index.no_budgets") %>
-  </div>
-<% end %>
+<%= render Valuation::Budgets::IndexComponent.new(@budgets) %>

spec/system/admin/budget_investments_spec.rb

@@ -1567,6 +1567,7 @@
     end
 
     scenario "Shows the correct investments to valuators" do
+      budget.update!(phase: :valuating)
       investment1.update!(visible_to_valuators: true)
       investment2.update!(visible_to_valuators: false)
 

spec/system/valuation/budget_investments_spec.rb

@@ -501,6 +501,17 @@
       expect(page).to have_content("Investments can only be valuated when Budget is in valuating phase")
     end
 
+    scenario "restric access to the budget given by params when is not in valuating phase" do
+      budget.update!(phase: "publishing_prices")
+      create(:budget, :valuating)
+      investment = create(:budget_investment, budget: budget, valuators: [valuator])
+
+      login_as(valuator.user)
+      visit edit_valuation_budget_budget_investment_path(budget, investment)
+
+      expect(page).to have_content("Investments can only be valuated when Budget is in valuating phase")
+    end
+
     scenario "visible to admins regardless of not being in valuating phase" do
       budget.update!(phase: "publishing_prices")
 

spec/system/valuation/budgets_spec.rb

@@ -1,29 +1,75 @@
 require "rails_helper"
 
 describe "Valuation budgets" do
-  before do
-    valuator = create(:valuator, user: create(:user, username: "Rachel", email: "rachel@valuators.org"))
-    login_as(valuator.user)
-  end
+  let(:valuator) { create(:valuator, user: create(:user)) }
+
+  before { login_as(valuator.user) }
 
   context "Index" do
-    scenario "Displaying budgets" do
-      budget = create(:budget)
+    scenario "Displays published budgets" do
+      create(:budget, name: "Sports")
+      create(:budget, name: "Draft", published: false)
+
+      visit valuation_budgets_path
+
+      expect(page).to have_content("Sports")
+      expect(page).not_to have_content("Draft")
+    end
+
+    scenario "Displays visible and assigned investments count when budget is in valuating phase" do
+      budget = create(:budget, :valuating, name: "Sports")
+      create(:budget_investment, :visible_to_valuators, budget: budget, valuators: [valuator])
+      create(:budget_investment, :invisible_to_valuators, budget: budget, valuators: [valuator])
+      create(:budget_investment, :visible_to_valuators, budget: budget)
+
+      visit valuation_budgets_path
+
+      within "#budget_#{budget.id}" do
+        expect(page).to have_selector(".investments-count", text: "1")
+      end
+    end
+
+    scenario "Displays zero as investments count when budget is not in valuating phase" do
+      budget = create(:budget, %i[accepting finished].sample, name: "Sports")
+      create(:budget_investment, :visible_to_valuators, budget: budget, valuators: [valuator])
+
       visit valuation_budgets_path
 
-      expect(page).to have_content(budget.name)
+      within "#budget_#{budget.id}" do
+        expect(page).to have_selector(".investments-count", text: "0")
+      end
     end
 
-    scenario "Filters by phase" do
-      budget1 = create(:budget, :finished)
-      budget2 = create(:budget, :finished)
-      budget3 = create(:budget, :accepting)
+    scenario "Displays the link to evaluate investments when valuator has visible investments assigned
+              and budget is in valuating phase" do
+      valuating = create(:budget, :valuating)
+      create(:budget_investment, :visible_to_valuators, budget: valuating, valuators: [valuator])
+      valuating_invisible = create(:budget, :valuating)
+      create(:budget_investment, :invisible_to_valuators, budget: valuating_invisible, valuators: [valuator])
+      valuating_unassigned = create(:budget, :valuating)
+      create(:budget_investment, :visible_to_valuators, budget: valuating_unassigned)
+      accepting = create(:budget, :accepting)
+      create(:budget_investment, :visible_to_valuators, budget: accepting, valuators: [valuator])
+      finished = create(:budget, :finished)
+      create(:budget_investment, :visible_to_valuators, budget: finished, valuators: [valuator])
 
       visit valuation_budgets_path
 
-      expect(page).not_to have_content(budget1.name)
-      expect(page).not_to have_content(budget2.name)
-      expect(page).to have_content(budget3.name)
+      within "#budget_#{valuating.id}" do
+        expect(page).to have_link("Evaluate")
+      end
+      within "#budget_#{valuating_invisible.id}" do
+        expect(page).not_to have_link("Evaluate")
+      end
+      within "#budget_#{valuating_unassigned.id}" do
+        expect(page).not_to have_link("Evaluate")
+      end
+      within "#budget_#{accepting.id}" do
+        expect(page).not_to have_link("Evaluate")
+      end
+      within "#budget_#{finished.id}" do
+        expect(page).not_to have_link("Evaluate")
+      end
     end
 
     scenario "With no budgets" do