Refactor dashboard_choices

cherry-picked

2 files changed, 26 insertions, 21 deletions

diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb
index 4349f12ec5a..f58e2c6635b 100644
--- a/app/helpers/preferences_helper.rb
+++ b/app/helpers/preferences_helper.rb
@@ -1,21 +1,24 @@
 # Helper methods for per-User preferences
 module PreferencesHelper
-  # Populates the dashboard preference select field with more user-friendly
-  # values.
-  def dashboard_choices
-    orig = User.dashboards.keys
+  # Maps `dashboard` values to more user-friendly option text
+  DASHBOARD_CHOICES = {
+    projects: 'Your Projects (default)',
+    stars:    'Starred Projects'
+  }.with_indifferent_access.freeze
 
-    choices = [
-      ['Your Projects (default)', orig[0]],
-      ['Starred Projects',        orig[1]]
-    ]
+  # Returns an Array usable by a select field for more user-friendly option text
+  def dashboard_choices
+    defined = User.dashboards
 
-    if orig.size != choices.size
-      # Assure that anyone adding new options updates this method too
-      raise RuntimeError, "`User` defines #{orig.size} dashboard choices," +
-        " but #{__method__} defined #{choices.size}"
+    if defined.size != DASHBOARD_CHOICES.size
+      # Ensure that anyone adding new options updates this method too
+      raise RuntimeError, "`User` defines #{defined.size} dashboard choices," +
+        " but `DASHBOARD_CHOICES` defined #{DASHBOARD_CHOICES.size}."
     else
-      choices
+      defined.map do |key, _|
+        # Use `fetch` so `KeyError` gets raised when a key is missing
+        [DASHBOARD_CHOICES.fetch(key), key]
+      end
     end
   end
 end
diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb
index 46d104a8e80..0fba3ba5631 100644
--- a/spec/helpers/preferences_helper_spec.rb
+++ b/spec/helpers/preferences_helper_spec.rb
@@ -3,18 +3,20 @@ require 'spec_helper'
 describe PreferencesHelper do
   describe 'dashboard_choices' do
     it 'raises an exception when defined choices may be missing' do
-      dashboards = User.dashboards
-      expect(User).to receive(:dashboards).
-        and_return(dashboards.merge(foo: 'foo'))
+      expect(User).to receive(:dashboards).and_return(foo: 'foo')
+      expect { dashboard_choices }.to raise_error(RuntimeError)
+    end
 
-      expect { dashboard_choices }.to raise_error
+    it 'raises an exception when defined choices may be using the wrong key' do
+      expect(User).to receive(:dashboards).and_return(foo: 'foo', bar: 'bar')
+      expect { dashboard_choices }.to raise_error(KeyError)
     end
 
     it 'provides better option descriptions' do
-      choices = dashboard_choices
-
-      expect(choices[0]).to eq ['Your Projects (default)', 'projects']
-      expect(choices[1]).to eq ['Starred Projects',        'stars']
+      expect(dashboard_choices).to match_array [
+        ['Your Projects (default)', 'projects'],
+        ['Starred Projects',        'stars']
+      ]
     end
   end
 end