From 99fd838ca1b0eb8283ca569224788288b6375b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 27 May 2019 15:13:52 +0200 Subject: [PATCH 1/4] Extract partial to render finished budgets --- app/views/budgets/_finished.html.erb | 32 +++++++++++++++++++++++++++ app/views/budgets/index.html.erb | 33 +--------------------------- 2 files changed, 33 insertions(+), 32 deletions(-) create mode 100644 app/views/budgets/_finished.html.erb diff --git a/app/views/budgets/_finished.html.erb b/app/views/budgets/_finished.html.erb new file mode 100644 index 000000000..97ad2aa18 --- /dev/null +++ b/app/views/budgets/_finished.html.erb @@ -0,0 +1,32 @@ +
+
+

<%= t("budgets.index.finished_budgets") %>

+ +
+ <% budgets.each do |budget| %> +
+
+
+
+
+

<%= budget.name %>

+
+
+ +
+
+ <%= link_to t("budgets.index.see_results"), + budget_results_path(budget.id), + class: "button" %> + <%= link_to t("budgets.index.milestones"), + budget_executions_path(budget.id), + class: "button" %> +
+
+
+
+
+ <% end %> +
+
+
diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index 9e198156f..e19a13416 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -126,38 +126,7 @@ <% if @finished_budgets.present? %> -
-
-

<%= t("budgets.index.finished_budgets") %>

- -
- <% @finished_budgets.each do |budget| %> -
-
-
-
-
-

<%= budget.name %>

-
-
- -
-
- <%= link_to t("budgets.index.see_results"), - budget_results_path(budget.id), - class: "button" %> - <%= link_to t("budgets.index.milestones"), - budget_executions_path(budget.id), - class: "button" %> -
-
-
-
-
- <% end %> -
-
-
+ <%= render "finished", budgets: @finished_budgets %> <% end %> <% else %> From c8b6a1a4a1be5e5c1f5263b7c64afe059282b074 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 27 May 2019 15:24:20 +0200 Subject: [PATCH 2/4] Use the same view code for finished budgets The differences between the custom code and the code in CONSUL didn't make much sense anymore: the path can be passed a budget instead of an ID (so it uses a slug) and there's no reason to hide the link to the executions for the 2018 budget anymore. Furthermore, removing the `status: 1` parameter makes the link consistent with the rest of the application, since it was the only link to budget executions using that parameter. --- app/views/budgets/_finished.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/budgets/_finished.html.erb b/app/views/budgets/_finished.html.erb index 97ad2aa18..f7c17fc58 100644 --- a/app/views/budgets/_finished.html.erb +++ b/app/views/budgets/_finished.html.erb @@ -16,10 +16,10 @@
<%= link_to t("budgets.index.see_results"), - budget_results_path(budget.id), + budget_results_path(budget), class: "button" %> <%= link_to t("budgets.index.milestones"), - budget_executions_path(budget.id), + budget_executions_path(budget), class: "button" %>
From eb7dc3ef2d7f130af076b609c7af4bb834102213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 4 Jun 2019 11:48:14 +0200 Subject: [PATCH 3/4] Extract partial to render budget navigation links --- app/helpers/budgets_helper.rb | 14 ++++++++++++++ app/views/budgets/_subnav.html.erb | 17 +++++++++++++++++ app/views/budgets/executions/show.html.erb | 16 +--------------- app/views/budgets/results/show.html.erb | 17 +---------------- app/views/budgets/stats/show.html.erb | 17 +---------------- 5 files changed, 34 insertions(+), 47 deletions(-) create mode 100644 app/views/budgets/_subnav.html.erb diff --git a/app/helpers/budgets_helper.rb b/app/helpers/budgets_helper.rb index d8425c217..5ac9b12a1 100644 --- a/app/helpers/budgets_helper.rb +++ b/app/helpers/budgets_helper.rb @@ -105,4 +105,18 @@ module BudgetsHelper ends_at: balloting_phase.ends_at }), method: :post end + + def budget_subnav_items_for(budget) + { + results: t("budgets.results.link"), + stats: t("stats.budgets.link"), + executions: t("budgets.executions.link") + }.map do |section, text| + { + text: text, + url: send("budget_#{section}_path", budget), + active: controller_name == section.to_s + } + end + end end diff --git a/app/views/budgets/_subnav.html.erb b/app/views/budgets/_subnav.html.erb new file mode 100644 index 000000000..f5f960377 --- /dev/null +++ b/app/views/budgets/_subnav.html.erb @@ -0,0 +1,17 @@ +
+
+
    + <% budget_subnav_items_for(budget).each do |item| %> + <% if item[:active] %> +
  • + <%= t("shared.you_are_in") %> + + <%= link_to item[:text], item[:url], class: "is-active" %> +
    • + <% else %> +
  • <%= link_to item[:text], item[:url] %>
    • + <% end %> + <% end %> +
+
+
diff --git a/app/views/budgets/executions/show.html.erb b/app/views/budgets/executions/show.html.erb index a0e3324bc..acab50de4 100644 --- a/app/views/budgets/executions/show.html.erb +++ b/app/views/budgets/executions/show.html.erb @@ -25,21 +25,7 @@ -
-
-
    -
  • - <%= link_to t("budgets.results.link"), budget_results_path(@budget) %> -
    • -
  • - <%= link_to t("stats.budgets.link"), budget_stats_path(@budget) %> -
    • -
  • - <%= link_to t("budgets.executions.link"), budget_executions_path(@budget), class: "is-active" %> -
    • -
-
-
+<%= render "budgets/subnav", budget: @budget %>
diff --git a/app/views/budgets/results/show.html.erb b/app/views/budgets/results/show.html.erb index d77f50b77..6a8829437 100644 --- a/app/views/budgets/results/show.html.erb +++ b/app/views/budgets/results/show.html.erb @@ -24,22 +24,7 @@
-
-
-
    -
  • - <%= t("shared.you_are_in") %> - <%= link_to t("budgets.results.link"), budget_results_path(@budget), class: "is-active" %> -
    • -
  • - <%= link_to t("stats.budgets.link"), budget_stats_path(@budget) %> -
    • -
  • - <%= link_to t("budgets.executions.link"), budget_executions_path(@budget) %> -
    • -
-
-
+<%= render "budgets/subnav", budget: @budget %>
diff --git a/app/views/budgets/stats/show.html.erb b/app/views/budgets/stats/show.html.erb index bbbfda4bb..60697135d 100644 --- a/app/views/budgets/stats/show.html.erb +++ b/app/views/budgets/stats/show.html.erb @@ -21,22 +21,7 @@
-
-
-
    -
  • - <%= t("shared.you_are_in") %> - <%= link_to t("budgets.results.link"), budget_results_path(@budget) %> -
    • -
  • - <%= link_to t("stats.budgets.link"), budget_stats_path(@budget), class: "is-active" %> -
    • -
  • - <%= link_to t("budgets.executions.link"), budget_executions_path(@budget) %> -
    • -
-
-
+ <%= render "budgets/subnav", budget: @budget %>
From da59315530de15d4ab5c2f36b395938655a92f80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 4 Jun 2019 12:24:20 +0200 Subject: [PATCH 4/4] Don't show links to budget results if disabled --- app/helpers/budgets_helper.rb | 2 +- app/views/budgets/_finished.html.erb | 9 ++++++--- app/views/budgets/index.html.erb | 2 +- app/views/budgets/show.html.erb | 2 +- spec/features/budgets/results_spec.rb | 16 ++++++++++++++++ 5 files changed, 25 insertions(+), 6 deletions(-) diff --git a/app/helpers/budgets_helper.rb b/app/helpers/budgets_helper.rb index 5ac9b12a1..ea2a327f7 100644 --- a/app/helpers/budgets_helper.rb +++ b/app/helpers/budgets_helper.rb @@ -111,7 +111,7 @@ module BudgetsHelper results: t("budgets.results.link"), stats: t("stats.budgets.link"), executions: t("budgets.executions.link") - }.map do |section, text| + }.select { |section, _| can?(:"read_#{section}", budget) }.map do |section, text| { text: text, url: send("budget_#{section}_path", budget), diff --git a/app/views/budgets/_finished.html.erb b/app/views/budgets/_finished.html.erb index f7c17fc58..60b715917 100644 --- a/app/views/budgets/_finished.html.erb +++ b/app/views/budgets/_finished.html.erb @@ -15,9 +15,12 @@
- <%= link_to t("budgets.index.see_results"), - budget_results_path(budget), - class: "button" %> + <% if can?(:read_results, budget) %> + <%= link_to t("budgets.index.see_results"), + budget_results_path(budget), + class: "button" %> + <% end %> + <%= link_to t("budgets.index.milestones"), budget_executions_path(budget), class: "button" %> diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index e19a13416..5a7dde0ba 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -53,7 +53,7 @@ <% end %> <% end %> - <% if current_budget.finished? %> + <% if can?(:read_results, current_budget) %> <%= link_to t("budgets.show.see_results"), budget_results_path(current_budget, heading_id: current_budget.headings.first), class: "button margin-top expanded" %> diff --git a/app/views/budgets/show.html.erb b/app/views/budgets/show.html.erb index 89e4e146c..26af33856 100644 --- a/app/views/budgets/show.html.erb +++ b/app/views/budgets/show.html.erb @@ -36,7 +36,7 @@ <% end %> <% end %> - <% if @budget.finished? %> + <% if can?(:read_results, @budget) %> <%= link_to t("budgets.show.see_results"), budget_results_path(@budget), class: "button margin-top expanded" %> diff --git a/spec/features/budgets/results_spec.rb b/spec/features/budgets/results_spec.rb index e10e9e7b2..5b9277d56 100644 --- a/spec/features/budgets/results_spec.rb +++ b/spec/features/budgets/results_spec.rb @@ -15,6 +15,22 @@ describe "Results" do Budget::Result.new(budget, heading).calculate_winners end + scenario "No links to budget results with results disabled" do + budget.update(results_enabled: false) + + visit budgets_path + + expect(page).not_to have_link "See results" + + visit budget_path(budget) + + expect(page).not_to have_link "See results" + + visit budget_executions_path(budget) + + expect(page).not_to have_link "See results" + end + scenario "Diplays winner investments" do create(:budget_heading, group: group)