From 784900ccfa3ed093f806dfbe8eb54abb2d7be400 Mon Sep 17 00:00:00 2001
From: JackBlacklight <jd2148@columbia.edu>
Date: Wed, 4 Sep 2024 11:47:41 -0400
Subject: [PATCH] refactor deposits enable site option code

---
 .../admin/site_options_controller.rb          | 12 ++-
 app/models/site_option.rb                     | 16 ++--
 .../deposits/_enable_deposits_form.html.erb   | 12 +++
 app/views/layouts/admin.html.erb              |  2 +
 spec/controllers/uploads_controller_spec.rb   | 29 ++++---
 spec/factories/site_option.rb                 |  2 +
 spec/features/agreement_spec.rb               |  2 +-
 spec/features/homepage_spec.rb                | 76 +++++++++++--------
 spec/features/myworks_spec.rb                 | 12 +--
 spec/features/upload_spec.rb                  |  4 +-
 10 files changed, 100 insertions(+), 67 deletions(-)
 create mode 100644 app/views/admin/deposits/_enable_deposits_form.html.erb

diff --git a/app/controllers/admin/site_options_controller.rb b/app/controllers/admin/site_options_controller.rb
index 4a4a449f..ea71cf51 100644
--- a/app/controllers/admin/site_options_controller.rb
+++ b/app/controllers/admin/site_options_controller.rb
@@ -1,12 +1,16 @@
+# frozen_string_literal: true
+
 module Admin
   class SiteOptionsController < AdminController
     load_and_authorize_resource class: SiteOption
 
     def update
-      option = SiteOption.find_by(name: "deposits_enabled")
-      option.update(value: params["enable_deposits"])
-       redirect_back fallback_location: { controller: 'admin/deposits', action: 'index' }
+      option_keys = params.keys.select { |key| SiteOption::OPTIONS.include?(key) }
+      option_keys.each do |option_key|
+        option = SiteOption.find_by(name: option_key)
+        option.update(value: params[option_key])
+        redirect_back fallback_location: { controller: 'admin', action: 'index' }
+      end
     end
-
   end
 end
diff --git a/app/models/site_option.rb b/app/models/site_option.rb
index 4238fde6..60db2883 100644
--- a/app/models/site_option.rb
+++ b/app/models/site_option.rb
@@ -1,11 +1,13 @@
-class SiteOption < ApplicationRecord
+# frozen_string_literal: true
 
-    validates_presence_of :name
-    validates :value, inclusion: { in: [ true, false ] }
+class SiteOption < ApplicationRecord
+  DEPOSITS_ENABLED = 'deposits_enabled'
+  OPTIONS = [DEPOSITS_ENABLED].freeze
 
-    def self.deposits_enabled
-        find_by(name: 'deposits_enabled')&.value
-    end
+  validates :name, presence: { inclusion: { in: OPTIONS } }
+  validates :value, inclusion: { in: [true, false] }
 
+  def self.deposits_enabled
+    find_by(name: DEPOSITS_ENABLED)&.value
+  end
 end
-  
\ No newline at end of file
diff --git a/app/views/admin/deposits/_enable_deposits_form.html.erb b/app/views/admin/deposits/_enable_deposits_form.html.erb
new file mode 100644
index 00000000..7451bf90
--- /dev/null
+++ b/app/views/admin/deposits/_enable_deposits_form.html.erb
@@ -0,0 +1,12 @@
+<%= form_with url: admin_site_options_path, method: :patch do |form| %>
+    <% if deposits_enabled?  %>
+      <% action_label = "Disable Deposits" %>
+      <% enable = false %>
+    <% else %>
+      <% action_label = "Enable Deposits" %>
+      <% enable = true %>
+    <% end %>
+    <%= form.hidden_field SiteOption::DEPOSITS_ENABLED, value: enable %>
+    <%= form.submit action_label %>
+  <% end %>
+  
\ No newline at end of file
diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb
index 0f17537f..0dccd47e 100644
--- a/app/views/layouts/admin.html.erb
+++ b/app/views/layouts/admin.html.erb
@@ -12,11 +12,13 @@
           <i class="fas fa-list-ul" aria-hidden></i> <%= t('blacklight.dashboard_links.my_works') %>
         <% end %>
         </li>
+        <% if deposits_enabled? %>
         <li>
           <%= link_to uploads_path do %>
           <i class="far fa-plus-square" aria-hidden></i> <%= t('blacklight.dashboard_links.add_new_work') %>
         <% end %>
         </li>
+        <% end %>
         <li class="active">
           <%= link_to admin_path do %>
           <i class="fas fa-cog" aria-hidden></i> <%= t('blacklight.dashboard_links.admin') %>
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 5d7db5a4..5bbc86f7 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -1,11 +1,10 @@
 require 'rails_helper'
 
-
 RSpec.describe UploadsController, type: :controller do
-    context 'when user logged in ' do
-      include_context 'non-admin user for controller'
+  context 'when user logged in ' do
+    include_context 'non-admin user for controller'
 
-      describe "deposits are enabled" do
+    describe 'deposits are enabled' do
       let(:deposit) { Deposit.first }
 
       let(:http_request) do
@@ -25,11 +24,10 @@
              }
       end
 
-      before do 
-        SiteOption.create!(name: 'deposits_enabled', value: true)
+      before do
+        SiteOption.create!(name: SiteOption::DEPOSITS_ENABLED, value: true)
       end
 
-
       it 'response is successful' do
         http_request
         expect(response).to have_http_status :success
@@ -55,14 +53,15 @@
       end
     end
 
-      describe "when deposits are disabled" do
-        before do 
-          SiteOption.create!(name: 'deposits_enabled', value: false)
-          get :new 
-        end
-        it 'redirects to home page' do
-          expect(response).to redirect_to(root_path)
-        end
+    describe 'when deposits are disabled' do
+      before do
+        SiteOption.create!(name: SiteOption::DEPOSITS_ENABLED, value: false)
+        get :new
+      end
+
+      it 'redirects to home page' do
+        expect(response).to redirect_to(root_path)
       end
     end
   end
+end
diff --git a/spec/factories/site_option.rb b/spec/factories/site_option.rb
index a07b89e5..77e6c7d7 100644
--- a/spec/factories/site_option.rb
+++ b/spec/factories/site_option.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 FactoryBot.define do
   factory :site_option do
     name { '' }
diff --git a/spec/features/agreement_spec.rb b/spec/features/agreement_spec.rb
index 93312010..16cc9cb4 100644
--- a/spec/features/agreement_spec.rb
+++ b/spec/features/agreement_spec.rb
@@ -4,7 +4,7 @@
   include_context 'non-admin user for feature'
 
   before do
-    SiteOption.create!(name: 'deposits_enabled', value: true)
+    SiteOption.create!(name: SiteOption::DEPOSITS_ENABLED, value: true)
     visit agreement_path
   end
 
diff --git a/spec/features/homepage_spec.rb b/spec/features/homepage_spec.rb
index a1fce884..7f5edbff 100644
--- a/spec/features/homepage_spec.rb
+++ b/spec/features/homepage_spec.rb
@@ -1,51 +1,63 @@
 require 'rails_helper'
 
 describe 'Homepage', type: :feature do
-  before { visit root_path }
-
-  it 'has the "Stats at a glance" content panel' do
-    expect(page).to have_content('Stats at a glance')
+  shared_context 'when visiting the home page' do
+    before do
+      visit root_path
+    end
   end
 
-  it 'renders the CUL header' do
-    expect(page).to have_css('div.cul-banner', text: 'Columbia University Libraries')
-  end
+  describe 'when visiting home page' do
+    include_context 'when visiting the home page'
 
-  it 'links to the about page' do
-    within('.about-links') do
-      expect(page).to have_link('About', href: '/about')
-      click_link 'About'
+    it 'has the "Stats at a glance" content panel' do
+      expect(page).to have_content('Stats at a glance')
     end
-    expect(page).to have_css 'h3', text: 'Enhanced discoverability'
-  end
 
-  it 'links to the explore page' do
-    within('.about-links') do
-      expect(page).to have_link('Explore', href: '/explore')
+    it 'renders the CUL header' do
+      expect(page).to have_css('div.cul-banner', text: 'Columbia University Libraries')
     end
-  end
 
-  it 'displays correct total number of items in repository' do
-    expect(page).to have_content('1 total works')
-  end
+    it 'links to the about page' do
+      within('.about-links') do
+        expect(page).to have_link('About', href: '/about')
+        click_link 'About'
+      end
+      expect(page).to have_css 'h3', text: 'Enhanced discoverability'
+    end
 
-  context 'when deposits are enabled' do
-    before do 
-      SiteOption.create!(name: 'deposits_enabled', value: true)
-      visit root_path 
+    it 'links to the explore page' do
+      within('.about-links') do
+        expect(page).to have_link('Explore', href: '/explore')
+      end
     end
-    it 'links to the upload page' do
-      expect(page).to have_css('a[href="/upload"]')
+
+    it 'displays correct total number of items in repository' do
+      expect(page).to have_content('1 total works')
     end
   end
 
-  context 'when deposits are disabled' do
-    before do 
-      SiteOption.create!(name: 'deposits_enabled', value: false)
-      visit root_path 
+  describe 'when rendering the home page with site options' do
+    context 'when deposits are enabled' do
+      before do
+        SiteOption.create!(name: SiteOption::DEPOSITS_ENABLED, value: true)
+      end
+
+      include_context 'when visiting the home page'
+      it 'links to the upload page' do
+        expect(page).to have_css('a[href="/upload"]')
+      end
     end
-    it 'does not link to the upload page' do
-      expect(page).not_to have_css('a[href="/upload"]')
+
+    context 'when deposits are disabled' do
+      before do
+        SiteOption.create!(name: SiteOption::DEPOSITS_ENABLED, value: false)
+      end
+
+      include_context 'when visiting the home page'
+      it 'does not link to the upload page' do
+        expect(page).not_to have_css('a[href="/upload"]')
+      end
     end
   end
 end
diff --git a/spec/features/myworks_spec.rb b/spec/features/myworks_spec.rb
index d0926982..9ef7e1d5 100644
--- a/spec/features/myworks_spec.rb
+++ b/spec/features/myworks_spec.rb
@@ -63,12 +63,12 @@
     end
   end
 
-  
   context 'when deposits are enabled' do
-    before do 
-      SiteOption.create!(name: 'deposits_enabled', value: true)
+    before do
+      SiteOption.create!(name: SiteOption::DEPOSITS_ENABLED, value: true)
       visit myworks_path
     end
+
     it 'links to the upload page' do
       # this needs to look for a link by href
       expect(page).to have_css('a[href="/upload"]')
@@ -76,13 +76,13 @@
   end
 
   context 'when deposits are disabled' do
-    before do 
-      SiteOption.create!(name: 'deposits_enabled', value: false)
+    before do
+      SiteOption.create!(name: SiteOption::DEPOSITS_ENABLED, value: false)
       visit myworks_path
     end
+
     it 'does not link to the upload page' do
       expect(page).not_to have_css('a[href="/upload"]')
     end
   end
-
 end
diff --git a/spec/features/upload_spec.rb b/spec/features/upload_spec.rb
index 77952d49..cec43986 100644
--- a/spec/features/upload_spec.rb
+++ b/spec/features/upload_spec.rb
@@ -23,7 +23,7 @@
     include_context 'non-admin user for feature'
 
     before do
-      SiteOption.create!(name: 'deposits_enabled', value: true)
+      SiteOption.create!(name: SiteOption::DEPOSITS_ENABLED, value: true)
       visit uploads_path
     end
 
@@ -40,7 +40,7 @@
     include_context 'non-admin user for feature'
 
     before do
-      SiteOption.create!(name: 'deposits_enabled', value: true)
+      SiteOption.create!(name: SiteOption::DEPOSITS_ENABLED, value: true)
       # Added signed agreement for logged in user, tried mocking :signed_latest_agreement? but it did not work
       Agreement.create(user: User.first, name: 'Test User', email: 'tu123@columbia.edu', agreement_version: Agreement::LATEST_AGREEMENT_VERSION)
       visit uploads_path