Merge pull request 'Refactor admin settings routes' (#156) from feature/content_settings into master

Reviewed-on: #156

app/controllers/admin/settings/registrations_controller.rb

@@ -1,8 +1,8 @@
 class Admin::Settings::RegistrationsController < Admin::SettingsController
-  def index
+  def show
   end
 
-  def create
+  def update
     update_settings
 
     redirect_to admin_settings_registrations_path, flash: {

app/controllers/admin/settings/services_controller.rb

@@ -1,19 +1,32 @@
 class Admin::Settings::ServicesController < Admin::SettingsController
-  def index
-    @service = params[:s]
+  before_action :set_service, only: [:show, :update]
 
-    if @service.blank?
-      redirect_to admin_settings_services_path(params: { s: "btcpay" })
-    end
+  def index
+    redirect_to admin_settings_service_path("btcpay")
   end
 
-  def create
-    service = params.require(:service)
+  def show
+  end
 
+  def update
     update_settings
 
-    redirect_to admin_settings_services_path(params: { s: service }), flash: {
+    redirect_to admin_settings_service_path(@service), flash: {
       success: "Settings saved"
     }
   end
+
+  private
+
+    def set_subsection
+      @subsection = "services"
+    end
+
+    def set_service
+      @service = params[:service]
+
+      if @service.blank?
+        redirect_to admin_settings_services_path and return
+      end
+    end
 end

app/controllers/admin/settings_controller.rb

@@ -20,7 +20,7 @@ class Admin::SettingsController < Admin::BaseController
     end
 
     if @errors.any?
-      render :index and return
+      render :show and return
     end
 
     changed_keys.each do |key|

app/views/admin/settings/registrations/show.html.erb

@@ -1,7 +1,7 @@
 <%= render HeaderComponent.new(title: "Settings") %>
 
 <%= render MainWithSidenavComponent.new(sidenav_partial: 'shared/admin_sidenav_settings') do %>
-  <%= form_for(Setting.new, url: admin_settings_registrations_path) do |f| %>
+  <%= form_for(Setting.new, url: admin_settings_registrations_path, method: :put) do |f| %>
     <section>
       <h3>Registrations</h3>
 

app/views/admin/settings/services/show.html.erb

@@ -1,9 +1,7 @@
 <%= render HeaderComponent.new(title: "Settings") %>
 
 <%= render MainWithSidenavComponent.new(sidenav_partial: 'shared/admin_sidenav_settings') do %>
-  <%= form_for(Setting.new, url: admin_settings_services_path) do |f| %>
-    <%= hidden_field_tag :service, @service %>
-
+  <%= form_for(Setting.new, url: admin_settings_service_path(@service), method: :put) do |f| %>
     <% if @errors && @errors.any? %>
       <section>
         <%= render partial: "admin/settings/errors", locals: { errors: @errors } %>

app/views/shared/_admin_sidenav_settings.html.erb

@@ -4,9 +4,9 @@
 ) %>
 <%= render SidenavLinkComponent.new(
   name: "Services", path: admin_settings_services_path, icon: "grid",
-  active: current_page?(admin_settings_services_path)
+  active: controller_name == "services"
 ) %>
-<% if current_page?(admin_settings_services_path) %>
+<% if controller_name == "services" %>
   <%= render partial: "shared/admin_sidenav_settings_services"  %>
 <% end %>
 <%= render SidenavLinkComponent.new(

app/views/shared/_admin_sidenav_settings_services.html.erb

@@ -1,77 +1,77 @@
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "BTCPay",
-  path: admin_settings_services_path(params: { s: "btcpay" }),
+  path: admin_settings_service_path("btcpay"),
   text_icon: Setting.btcpay_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "btcpay" })),
+  active: current_page?(admin_settings_service_path("btcpay")),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "Discourse",
-  path: admin_settings_services_path(params: { s: "discourse" }),
+  path: admin_settings_service_path("discourse"),
   text_icon: Setting.discourse_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "discourse" })),
+  active: current_page?(admin_settings_service_path("discourse")),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "Drone CI",
-  path: admin_settings_services_path(params: { s: "droneci" }),
+  path: admin_settings_service_path("droneci"),
   text_icon: Setting.droneci_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "droneci" })),
+  active: current_page?(admin_settings_service_path("droneci")),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "E-Mail",
-  path: admin_settings_services_path(params: { s: "email" }),
+  path: admin_settings_service_path("email"),
   text_icon: Setting.email_enabled? ? "◉" : "○",
   active: current_page?(admin_settings_services_path(params: { s: "email" })),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "ejabberd",
-  path: admin_settings_services_path(params: { s: "ejabberd" }),
+  path: admin_settings_service_path("ejabberd"),
   text_icon: Setting.ejabberd_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "ejabberd" })),
+  active: current_page?(admin_settings_service_path("ejabberd")),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "Gitea",
-  path: admin_settings_services_path(params: { s: "gitea" }),
+  path: admin_settings_service_path("gitea"),
   text_icon: Setting.gitea_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "gitea" })),
+  active: current_page?(admin_settings_service_path("gitea")),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "LNDHub",
-  path: admin_settings_services_path(params: { s: "lndhub" }),
+  path: admin_settings_service_path("lndhub"),
   text_icon: Setting.lndhub_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "lndhub" })),
+  active: current_page?(admin_settings_service_path("lndhub")),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "Mastodon",
-  path: admin_settings_services_path(params: { s: "mastodon" }),
+  path: admin_settings_service_path("mastodon"),
   text_icon: Setting.mastodon_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "mastodon" })),
+  active: current_page?(admin_settings_service_path("mastodon")),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "MediaWiki",
-  path: admin_settings_services_path(params: { s: "mediawiki" }),
+  path: admin_settings_service_path("mediawiki"),
   text_icon: Setting.mediawiki_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "mediawiki" })),
+  active: current_page?(admin_settings_service_path("mediawiki")),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "Nostr",
-  path: admin_settings_services_path(params: { s: "nostr" }),
+  path: admin_settings_service_path("nostr"),
   text_icon: Setting.nostr_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "nostr" })),
+  active: current_page?(admin_settings_service_path("nostr")),
 ) %>
 <%= render SidenavLinkComponent.new(
   level: 2,
   name: "RemoteStorage",
-  path: admin_settings_services_path(params: { s: "remotestorage" }),
+  path: admin_settings_service_path("remotestorage"),
   text_icon: Setting.remotestorage_enabled? ? "◉" : "○",
-  active: current_page?(admin_settings_services_path(params: { s: "remotestorage" })),
+  active: current_page?(admin_settings_service_path("remotestorage")),
 ) %>

config/routes.rb

@@ -93,8 +93,8 @@ Rails.application.routes.draw do
     end
 
     namespace :settings do
-      resources 'registrations', only: ['index', 'create']
-      resources 'services', only: ['index', 'create']
+      resource 'registrations', only: ['show', 'update']
+      resources 'services', param: 'service', only: ['index', 'show', 'update']
     end
   end
 

spec/features/admin/settings_spec.rb

@@ -23,35 +23,35 @@ RSpec.describe 'Admin/global settings', type: :feature do
     scenario "Opening service settings shows page for first service" do
       visit admin_settings_services_path
 
-      expect(current_url).to eq(admin_settings_services_url(params: { s: "btcpay" }))
+      expect(current_url).to eq(admin_settings_service_url("btcpay"))
     end
 
     scenario "View service settings" do
-      visit admin_settings_services_path(params: { s: "ejabberd" })
+      visit admin_settings_service_path("ejabberd")
 
       expect(page).to have_content("Enable ejabberd integration")
       expect(page).to have_field("API URL", with: "http://xmpp.example.com/api")
     end
 
     scenario "Disable a service integration" do
-      visit admin_settings_services_path(params: { s: "ejabberd" })
+      visit admin_settings_service_path("ejabberd")
       expect(page).to have_checked_field("setting[ejabberd_enabled]")
 
       uncheck "setting[ejabberd_enabled]"
       click_button "Save"
 
-      expect(current_url).to eq(admin_settings_services_url(params: { s: "ejabberd" }))
+      expect(current_url).to eq(admin_settings_service_url("ejabberd"))
       expect(page).to_not have_checked_field("setting[ejabberd_enabled]")
       expect(page).to_not have_field("API URL", disabled: true)
     end
 
     scenario "Resettable fields" do
-      visit admin_settings_services_path(params: { s: "ejabberd" })
+      visit admin_settings_service_path("ejabberd")
       expect(page).to have_field("API URL", with: "http://xmpp.example.com/api")
       expect(page).to_not have_css('input#setting_ejabberd_api_url+button')
 
       Setting.ejabberd_api_url = "http://example.com/foo"
-      visit admin_settings_services_path(params: { s: "ejabberd" })
+      visit admin_settings_service_path("ejabberd")
       expect(page).to have_field("API URL", with: "http://example.com/foo")
       expect(page).to have_css('input#setting_ejabberd_api_url+button')
     end