From 0cc3f040963502c508bd7beba84d8971022c450c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 30 Aug 2021 18:12:50 +0200
Subject: [PATCH] Don't show preview link for budgets with results

We currently don't have any links in the public area pointing to past
budgets, so having links in the admin section to both the budget and its
results seemed a bit redundant. We're going to add more links to the
budget actions soon, so we need to make room for them; otherwise we'll
have too many.

We're also changing the "Preview" text for a published budget. Since the
budget is already public, we aren't previewing it anymore but simply
viewing it.

And, to be consistent with the "See results" link, we're opening the
"Preview" link in the current tab. Opening links in a new tab is
generally a bad idea because takes control away from users, breaks the
back button and makes navigation particularly hard on mobile browsers.
It could be argued that in this case it's useful when users are editing
the budget in one tab and previewing it in another one, so we might add
this behavior back as long as we make it clear that the link opens in a
new tab [1].

[1] https://www.nngroup.com/articles/new-browser-windows-and-tabs/
---
 .../admin/budgets/links_component.html.erb    |  4 ++--
 .../admin/budgets/links_component.rb          |  8 +++++++
 .../admin/budgets/links_component_spec.rb     | 22 +++++++++++++++++++
 spec/system/admin/budgets_spec.rb             | 13 +++++------
 4 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/app/components/admin/budgets/links_component.html.erb b/app/components/admin/budgets/links_component.html.erb
index db6b436bd..8fa1ce144 100644
--- a/app/components/admin/budgets/links_component.html.erb
+++ b/app/components/admin/budgets/links_component.html.erb
@@ -1,9 +1,9 @@
 <div class="budgets-links">
   <% if can?(:read_results, budget) %>
     <%= action(:results, text: results_text, path: budget_results_path(budget)) %>
+  <% else %>
+    <%= action(:preview, text: preview_text, path: budget_path(budget)) %>
   <% end %>
 
-  <%= action(:preview, text: t("admin.budgets.actions.preview"), path: budget_path(budget), target: "_blank") %>
-
   <%= action(:edit, text: t("admin.budgets.actions.edit")) %>
 </div>
diff --git a/app/components/admin/budgets/links_component.rb b/app/components/admin/budgets/links_component.rb
index 909a4002b..a84273420 100644
--- a/app/components/admin/budgets/links_component.rb
+++ b/app/components/admin/budgets/links_component.rb
@@ -19,4 +19,12 @@ class Admin::Budgets::LinksComponent < ApplicationComponent
         t("admin.budgets.actions.preview_results")
       end
     end
+
+    def preview_text
+      if budget.published?
+        t("admin.shared.view")
+      else
+        t("admin.budgets.actions.preview")
+      end
+    end
 end
diff --git a/spec/components/admin/budgets/links_component_spec.rb b/spec/components/admin/budgets/links_component_spec.rb
index 18968f515..6648260a3 100644
--- a/spec/components/admin/budgets/links_component_spec.rb
+++ b/spec/components/admin/budgets/links_component_spec.rb
@@ -44,6 +44,8 @@ describe Admin::Budgets::LinksComponent, controller: Admin::BaseController do
 
         expect(page).to have_link "Preview results"
         expect(page).not_to have_link "See results"
+        expect(page).not_to have_link "View"
+        expect(page).not_to have_link "Preview"
       end
 
       it "is not shown while balloting" do
@@ -56,4 +58,24 @@ describe Admin::Budgets::LinksComponent, controller: Admin::BaseController do
       end
     end
   end
+
+  describe "preview/view link" do
+    it "shows a link to preview an unpublished budget" do
+      budget = create(:budget, :drafting)
+
+      render_inline Admin::Budgets::LinksComponent.new(budget)
+
+      expect(page).to have_link "Preview"
+      expect(page).not_to have_link "View"
+    end
+
+    it "shows a link to view a published budget" do
+      budget = create(:budget, :informing)
+
+      render_inline Admin::Budgets::LinksComponent.new(budget)
+
+      expect(page).to have_link "View"
+      expect(page).not_to have_link "Preview"
+    end
+  end
 end
diff --git a/spec/system/admin/budgets_spec.rb b/spec/system/admin/budgets_spec.rb
index cb0620543..fd37d0262 100644
--- a/spec/system/admin/budgets_spec.rb
+++ b/spec/system/admin/budgets_spec.rb
@@ -115,13 +115,12 @@ describe "Admin budgets", :admin do
 
     scenario "Can preview budget before it is published" do
       visit admin_budget_path(budget)
+      click_link "Preview"
 
-      within_window(window_opened_by { click_link "Preview" }) do
-        expect(page).to have_current_path budget_path(budget)
-      end
+      expect(page).to have_current_path budget_path(budget)
     end
 
-    scenario "Can preview a budget after it is published" do
+    scenario "Can view a budget after it is published" do
       visit admin_budget_path(budget)
 
       accept_confirm { click_button "Publish budget" }
@@ -130,9 +129,9 @@ describe "Admin budgets", :admin do
       expect(page).not_to have_content "This participatory budget is in draft mode"
       expect(page).not_to have_button "Publish budget"
 
-      within_window(window_opened_by { click_link "Preview" }) do
-        expect(page).to have_current_path budget_path(budget)
-      end
+      click_link "View"
+
+      expect(page).to have_current_path budget_path(budget)
     end
   end