From c156621a4cf66dbae08b50a22d80d323d40a7080 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 5 Dec 2020 17:10:03 +0100
Subject: [PATCH] Add method to generate a list of links

A list of links is a very common pattern in the web, and we use it in
many places. Here we're applying it to one of the most simple ones; the
help page.

Generally speaking, I'm not a big fan of helpers, but there are methods
which IMHO qualify as helpers when:

* They do not deal with application objects but mainly strings and
  arrays
* They return text or an HTML tag
* Their logic is simple and splitting it into several methods is not
  necessary

Many Rails helpers, like `tag`, follow these principles.
---
 app/helpers/link_list_helper.rb       | 13 +++++
 app/views/pages/help/_menu.html.erb   | 73 ++++++++++++++++-----------
 spec/helpers/link_list_helper_spec.rb | 46 +++++++++++++++++
 3 files changed, 102 insertions(+), 30 deletions(-)
 create mode 100644 app/helpers/link_list_helper.rb
 create mode 100644 spec/helpers/link_list_helper_spec.rb

diff --git a/app/helpers/link_list_helper.rb b/app/helpers/link_list_helper.rb
new file mode 100644
index 000000000..5c38bf042
--- /dev/null
+++ b/app/helpers/link_list_helper.rb
@@ -0,0 +1,13 @@
+module LinkListHelper
+  def link_list(*links, **options)
+    return "" if links.compact.empty?
+
+    tag.ul(options) do
+      safe_join(links.compact.map do |text, url, **link_options|
+        tag.li do
+          link_to text, url, link_options
+        end
+      end)
+    end
+  end
+end
diff --git a/app/views/pages/help/_menu.html.erb b/app/views/pages/help/_menu.html.erb
index b9e5c3074..c56a2eb42 100644
--- a/app/views/pages/help/_menu.html.erb
+++ b/app/views/pages/help/_menu.html.erb
@@ -1,34 +1,47 @@
 <div class="row">
   <div class="small-12 column">
-    <ul class="menu-pages" data-magellan>
-      <% if feature?(:debates) %>
-      <li>
-        <%= link_to t("pages.help.menu.debates"), "#debates", class: "button hollow expanded" %>
-      </li>
-      <% end %>
-      <% if feature?(:proposals) %>
-        <li>
-          <%= link_to t("pages.help.menu.proposals"), "#proposals", class: "button hollow expanded" %>
-        </li>
-      <% end %>
-      <% if feature?(:budgets) %>
-      <li>
-        <%= link_to t("pages.help.menu.budgets"), "#budgets", class: "button hollow expanded" %>
-      </li>
-      <% end %>
-      <% if feature?(:polls) %>
-      <li>
-        <%= link_to t("pages.help.menu.polls"), "#polls", class: "button hollow expanded" %>
-      </li>
-      <% end %>
-      <% if feature?(:legislation) %>
-      <li>
-        <%= link_to t("pages.help.menu.processes"), "#processes", class: "button hollow expanded" %>
-      </li>
-      <% end %>
-      <li>
-        <%= link_to t("pages.help.menu.other"), "#other", class: "button hollow expanded" %>
-      </li>
-    </ul>
+    <%= link_list(
+      (
+        [
+          t("pages.help.menu.debates"),
+          "#debates",
+          class: "button hollow expanded"
+        ] if feature?(:debates)
+      ),
+      (
+        [
+          t("pages.help.menu.proposals"),
+          "#proposals",
+          class: "button hollow expanded"
+        ] if feature?(:proposals)
+      ),
+      (
+        [
+          t("pages.help.menu.budgets"),
+          "#budgets",
+          class: "button hollow expanded"
+        ] if feature?(:budgets)
+      ),
+      (
+        [
+          t("pages.help.menu.polls"),
+          "#polls",
+          class: "button hollow expanded"
+        ] if feature?(:polls)
+      ),
+      (
+        [
+          t("pages.help.menu.processes"),
+          "#processes",
+          class: "button hollow expanded"
+        ]
+      ),
+      [
+        t("pages.help.menu.other"),
+        "#other",
+        class: "button hollow expanded"
+      ],
+      class: "menu-pages", "data-magellan": true
+    ) %>
   </div>
 </div>
diff --git a/spec/helpers/link_list_helper_spec.rb b/spec/helpers/link_list_helper_spec.rb
new file mode 100644
index 000000000..a6c4af0b3
--- /dev/null
+++ b/spec/helpers/link_list_helper_spec.rb
@@ -0,0 +1,46 @@
+require "rails_helper"
+
+describe LinkListHelper do
+  describe "#link_list" do
+    it "returns an empty string with an empty list" do
+      expect(helper.link_list).to eq ""
+    end
+
+    it "returns nothing with a list of nil elements" do
+      expect(helper.link_list(nil, nil)).to eq ""
+    end
+
+    it "generates a list of links" do
+      list = helper.link_list(["Home", "/"], ["Info", "/info"], class: "menu")
+
+      expect(list).to eq '<ul class="menu"><li><a href="/">Home</a></li><li><a href="/info">Info</a></li></ul>'
+      expect(list).to be_html_safe
+    end
+
+    it "accepts options for links" do
+      render helper.link_list(["Home", "/", class: "root"], ["Info", "/info", id: "info"])
+
+      expect(page).to have_css "a", count: 2
+      expect(page).to have_css "a.root", count: 1, exact_text: "Home"
+      expect(page).to have_css "a#info", count: 1, exact_text: "Info"
+    end
+
+    it "ignores nil entries" do
+      render helper.link_list(["Home", "/", class: "root"], nil, ["Info", "/info", id: "info"])
+
+      expect(page).to have_css "a", count: 2
+      expect(page).to have_css "a.root", count: 1, exact_text: "Home"
+      expect(page).to have_css "a#info", count: 1, exact_text: "Info"
+    end
+  end
+
+  attr_reader :content
+
+  def render(content)
+    @content = content
+  end
+
+  def page
+    Capybara::Node::Simple.new(content)
+  end
+end