Replace best_in_place editor on admin settings page (#2789)

* Remove best_in_place

* Replace best_in_place usage with rails helpers

* Move admin/settings#index to #edit action

* Remove click_to__edit from i18n

Gemfile

@@ -13,7 +13,6 @@ gem 'hamlit-rails'
 gem 'pg'
 gem 'pghero'
 gem 'dotenv-rails'
-gem 'best_in_place', '~> 3.0.1'
 
 gem 'aws-sdk', '>= 2.0'
 gem 'paperclip', '~> 5.1'

Gemfile.lock

@@ -65,9 +65,6 @@ GEM
       babel-source (>= 4.0, < 6)
       execjs (~> 2.0)
     bcrypt (3.1.11)
-    best_in_place (3.0.3)
-      actionpack (>= 3.2)
-      railties (>= 3.2)
     better_errors (2.1.1)
       coderay (>= 1.0.0)
       erubis (>= 2.6.6)
@@ -478,7 +475,6 @@ DEPENDENCIES
   addressable
   annotate
   aws-sdk (>= 2.0)
-  best_in_place (~> 3.0.1)
   better_errors
   binding_of_caller
   bullet

app/controllers/admin/settings_controller.rb

@@ -2,38 +2,43 @@
 
 module Admin
   class SettingsController < BaseController
+    ADMIN_SETTINGS = %w(
+      site_contact_username
+      site_contact_email
+      site_title
+      site_description
+      site_extended_description
+      open_registrations
+      closed_registrations_message
+    ).freeze
     BOOLEAN_SETTINGS = %w(open_registrations).freeze
 
-    def index
+    def edit
       @settings = Setting.all_as_records
     end
 
     def update
-      @setting = Setting.where(var: params[:id]).first_or_initialize(var: params[:id])
+      settings_params.each do |key, value|
-      @setting.update(value: value_for_update)
+        setting = Setting.where(var: key).first_or_initialize(var: key)
-
+        setting.update(value: value_for_update(key, value))
-      respond_to do |format|
-        format.html { redirect_to admin_settings_path }
-        format.json { respond_with_bip(@setting) }
       end
+
+      flash[:notice] = 'Success!'
+      redirect_to edit_admin_settings_path
     end
 
     private
 
     def settings_params
-      params.require(:setting).permit(:value)
+      params.permit(ADMIN_SETTINGS)
     end
 
-    def value_for_update
+    def value_for_update(key, value)
-      if updating_boolean_setting?
+      if BOOLEAN_SETTINGS.include?(key)
-        settings_params[:value] == 'true'
+        value == 'true'
       else
-        settings_params[:value]
+        value
       end
     end
-
-    def updating_boolean_setting?
-      BOOLEAN_SETTINGS.include?(params[:id])
-    end
   end
 end

app/javascript/styles/forms.scss

@@ -9,6 +9,12 @@ code {
   margin: 0 auto;
 }
 
+.admin {
+  input, textarea {
+    width: 100%;
+  }
+}
+
 .simple_form {
   .input {
     margin-bottom: 15px;

app/views/admin/settings/edit.html.haml

@@ -0,0 +1,58 @@
+- content_for :page_title do
+  = t('admin.settings.title')
+
+= form_tag(admin_settings_path, method: :put) do
+  %table.table
+    %thead
+      %tr
+        %th{width: '40%'}
+          = t('admin.settings.setting')
+        %th
+    %tbody
+      %tr
+        %td
+          %strong= t('admin.settings.contact_information.label')
+        %td= text_field_tag :site_contact_username,
+          @settings['site_contact_username'].value,
+          place_holder: t('admin.settings.contact_information.username')
+      %tr
+        %td
+          %strong= t('admin.accounts.email')
+        %td= text_field_tag :site_contact_email,
+          @settings['site_contact_email'].value,
+          place_holder: t('admin.settings.contact_information.email')
+      %tr
+        %td
+          %strong= t('admin.settings.site_title')
+        %td= text_field_tag :site_title,
+          @settings['site_title'].value
+      %tr
+        %td
+          %strong= t('admin.settings.site_description.title')
+          %p= t('admin.settings.site_description.desc_html')
+        %td= text_area_tag :site_description,
+          @settings['site_description'].value,
+          rows: 8
+      %tr
+        %td
+          %strong= t('admin.settings.site_description_extended.title')
+          %p= t('admin.settings.site_description_extended.desc_html')
+        %td= text_area_tag :site_extended_description,
+          @settings['site_extended_description'].value,
+          rows: 8
+      %tr
+        %td
+          %strong= t('admin.settings.registrations.open.title')
+        %td
+          = select_tag :open_registrations,
+          options_for_select({ t('admin.settings.registrations.open.disabled') => false, t('admin.settings.registrations.open.enabled') => true }, @settings['open_registrations'].value)
+      %tr
+        %td
+          %strong= t('admin.settings.registrations.closed_message.title')
+          %p= t('admin.settings.registrations.closed_message.desc_html')
+        %td= text_area_tag :closed_registrations_message,
+          @settings['closed_registrations_message'].value,
+          rows: 8
+
+  .simple_form.actions
+    = button_tag t('generic.save_changes'), type: :submit, class: :btn

app/views/admin/settings/index.html.haml

@@ -1,40 +0,0 @@
-- content_for :page_title do
-  = t('admin.settings.title')
-
-%table.table
-  %colgroup
-    %col{ width: '35%' }/
-  %thead
-    %tr
-      %th= t('admin.settings.setting')
-      %th= t('admin.settings.click_to_edit')
-  %tbody
-    %tr
-      %td{ rowspan: 2 }
-        %strong= t('admin.settings.contact_information.label')
-      %td= best_in_place @settings['site_contact_username'], :value, url: admin_setting_path(@settings['site_contact_username']), place_holder: t('admin.settings.contact_information.username')
-    %tr
-      %td= best_in_place @settings['site_contact_email'], :value, url: admin_setting_path(@settings['site_contact_email']), place_holder: t('admin.settings.contact_information.email')
-    %tr
-      %td
-        %strong= t('admin.settings.site_title')
-      %td= best_in_place @settings['site_title'], :value, url: admin_setting_path(@settings['site_title'])
-    %tr
-      %td
-        %strong= t('admin.settings.site_description.title')
-        %p= t('admin.settings.site_description.desc_html')
-      %td= best_in_place @settings['site_description'], :value, as: :textarea, url: admin_setting_path(@settings['site_description'])
-    %tr
-      %td
-        %strong= t('admin.settings.site_description_extended.title')
-        %p= t('admin.settings.site_description_extended.desc_html')
-      %td= best_in_place @settings['site_extended_description'], :value, as: :textarea, url: admin_setting_path(@settings['site_extended_description'])
-    %tr
-      %td
-        %strong= t('admin.settings.registrations.open.title')
-      %td= best_in_place @settings['open_registrations'], :value, as: :checkbox, collection: { false: t('admin.settings.registrations.open.disabled'), true: t('admin.settings.registrations.open.enabled')}, url: admin_setting_path(@settings['open_registrations'])
-    %tr
-      %td
-        %strong= t('admin.settings.registrations.closed_message.title')
-        %p= t('admin.settings.registrations.closed_message.desc_html')
-      %td= best_in_place @settings['closed_registrations_message'], :value, as: :textarea, url: admin_setting_path(@settings['closed_registrations_message'])

config/locales/de.yml

@@ -145,7 +145,6 @@ de:
       unresolved: Ungelöst
       view: Ansehen
     settings:
-      click_to_edit: Klicken zum Bearbeiten
       contact_information:
         email: Eine öffentliche E-Mail-Adresse angeben
         label: Kontaktinformationen

config/locales/en.yml

@@ -158,7 +158,6 @@ en:
       unresolved: Unresolved
       view: View
     settings:
-      click_to_edit: Click to edit
       contact_information:
         email: Enter a public e-mail address
         label: Contact information

config/locales/fa.yml

@@ -156,7 +156,6 @@ fa:
       unresolved: حل‌نشده
       view: نمایش
     settings:
-      click_to_edit: برای ویرایش کلیک کنید
       contact_information:
         email: یک نشانی ایمیل عمومی وارد کنید
         label: اطلاعات تماس

config/locales/fr.yml

@@ -136,7 +136,6 @@ fr:
       unresolved: Non résolus
       view: Voir
     settings:
-      click_to_edit: Cliquez pour éditer
       contact_information:
         email: Entrez une adresse courriel publique
         label: Informations de contact

config/locales/he.yml

@@ -157,7 +157,6 @@ he:
       unresolved: לא פתור
       view: תצוגה
     settings:
-      click_to_edit: לחיצה כדי לערוך
       contact_information:
         email: 'נא להקליד כתובת דוא"ל פומבית'
         label: פרטי התקשרות

config/locales/id.yml

@@ -156,7 +156,6 @@ id:
       unresolved: Belum Terseleseikan
       view: Tampilan
     settings:
-      click_to_edit: Klik untuk mengubah
       contact_information:
         email: Masukkan alamat email
         label: Informasi kontak

config/locales/io.yml

@@ -144,7 +144,6 @@ io:
       unresolved: Unresolved
       view: View
     settings:
-      click_to_edit: Click to edit
       contact_information:
         email: Enter a public e-mail address
         label: Contact information

config/locales/ja.yml

@@ -157,7 +157,6 @@ ja:
       unresolved: 未解決
       view: 表示
     settings:
-      click_to_edit: クリックして編集
       contact_information:
         email: 公開するメールアドレスを入力
         label: 連絡先情報

config/locales/nl.yml

@@ -41,7 +41,6 @@ nl:
     unfollow: Ontvolgen
   admin:
     settings:
-      click_to_edit: Klik om te bewerken
       contact_information:
         email: Vul een openbaar gebruikt e-mailadres in
         label: Contactgegevens

config/locales/oc.yml

@@ -146,7 +146,6 @@ oc:
       unresolved: Pas resolguts
       view: Veire
     settings:
-      click_to_edit: Clicatz per modificar
       contact_information:
         email: Picatz una adreça de corrièl
         label: Informacions de contacte

config/locales/pl.yml

@@ -158,7 +158,6 @@ pl:
       unresolved: Nierozwiązane
       view: Wyświetl
     settings:
-      click_to_edit: Naciśnij, aby edytować
       contact_information:
         email: Wprowadź publiczny adres e-mail
         label: Informacje kontaktowe

config/locales/pt-BR.yml

@@ -157,7 +157,6 @@ pt-BR:
       unresolved: Unresolved
       view: View
     settings:
-      click_to_edit: Clique para editar
       contact_information:
         email: Entre um endereço de email público
         label: Informação de contato

config/locales/pt.yml

@@ -152,7 +152,6 @@ pt:
       unresolved: Por resolver
       view: Ver
     settings:
-      click_to_edit: Clique para editar
       contact_information:
         email: Inserir um endereço de email para tornar público
         label: Informação de contacto

config/locales/ru.yml

@@ -139,7 +139,6 @@ ru:
       unresolved: Неразрешенные
       view: Просмотреть
     settings:
-      click_to_edit: Нажмите для изменения
       contact_information:
         email: Введите публичный e-mail
         label: Контактная информация

config/locales/th.yml

@@ -157,7 +157,6 @@ th:
       unresolved: Unresolved
       view: วิว
     settings:
-      click_to_edit: คลิ๊กเพื่อแก้ไข
       contact_information:
         email: กรอกที่อยู่อีเมล์สาธารณะ
         label: ข้อมูลที่ติดต่อ

config/locales/zh-CN.yml

@@ -154,7 +154,6 @@ zh-CN:
       unresolved: 未处理
       view: 查看
     settings:
-      click_to_edit: 点击编辑
       contact_information:
         email: 输入一个公开的电邮地址
         label: 联系数据

config/locales/zh-HK.yml

@@ -141,7 +141,6 @@ zh-HK:
       unresolved: 未處理
       view: 檢視
     settings:
-      click_to_edit: 點擊編輯
       contact_information:
         email: 輸入一個公開的電郵地址
         label: 聯絡資料

config/locales/zh-TW.yml

@@ -118,7 +118,6 @@ zh-TW:
       unresolved: 未解決
       view: 檢視
     settings:
-      click_to_edit: 點選以編輯
       contact_information:
         email: 請輸入輸入一個公開電子信箱
         label: 聯絡資訊

config/navigation.rb

@@ -23,7 +23,7 @@ SimpleNavigation::Configuration.run do |navigation|
       admin.item :domain_blocks, safe_join([fa_icon('lock fw'), t('admin.domain_blocks.title')]), admin_domain_blocks_url, highlights_on: %r{/admin/domain_blocks}
       admin.item :sidekiq, safe_join([fa_icon('diamond fw'), 'Sidekiq']), sidekiq_url, link_html: { target: 'sidekiq' }
       admin.item :pghero, safe_join([fa_icon('database fw'), 'PgHero']), pghero_url, link_html: { target: 'pghero' }
-      admin.item :settings, safe_join([fa_icon('cogs fw'), t('admin.settings.title')]), admin_settings_url
+      admin.item :settings, safe_join([fa_icon('cogs fw'), t('admin.settings.title')]), edit_admin_settings_url
     end
 
     primary.item :logout, safe_join([fa_icon('sign-out fw'), t('auth.logout')]), destroy_user_session_url, link_html: { 'data-method' => 'delete' }

config/routes.rb

@@ -76,7 +76,7 @@ Rails.application.routes.draw do
   namespace :admin do
     resources :pubsubhubbub, only: [:index]
     resources :domain_blocks, only: [:index, :new, :create, :show, :destroy]
-    resources :settings, only: [:index, :update]
+    resource :settings, only: [:edit, :update]
     resources :instances, only: [:index]
 
     resources :reports, only: [:index, :show, :update] do

lib/tasks/mastodon.rake

@@ -22,7 +22,7 @@ namespace :mastodon do
     user = Account.find_local(ENV.fetch('USERNAME')).user
     user.update(admin: true)
 
-    puts "Congrats! #{user.account.username} is now an admin. \\o/\nNavigate to #{admin_settings_url} to get started"
+    puts "Congrats! #{user.account.username} is now an admin. \\o/\nNavigate to #{edit_admin_settings_url} to get started"
   end
 
   desc 'Manually confirms a user with associated user email address stored in USER_EMAIL environment variable.'

spec/controllers/admin/settings_controller_spec.rb

@@ -1,51 +1,65 @@
+# frozen_string_literal: true
+
 require 'rails_helper'
 
 RSpec.describe Admin::SettingsController, type: :controller do
   render_views
 
+  before do
+    Rails.cache.clear
+  end
+
   describe 'When signed in as an admin' do
     before do
       sign_in Fabricate(:user, admin: true), scope: :user
     end
 
-    describe 'GET #index' do
+    describe 'GET #edit' do
       it 'returns http success' do
-        get :index
+        get :edit
 
         expect(response).to have_http_status(:success)
       end
     end
 
     describe 'PUT #update' do
-
       describe 'for a record that doesnt exist' do
         after do
           Setting.new_setting_key = nil
         end
 
-        it 'creates a settings value that didnt exist before' do
+        it 'cannot create a setting value for a non-admin key' do
           expect(Setting.new_setting_key).to be_nil
 
-          patch :update, params: { id: 'new_setting_key', setting: { value: 'New key value' } }
+          patch :update, params: { new_setting_key: 'New key value' }
 
-          expect(response).to redirect_to(admin_settings_path)
+          expect(response).to redirect_to(edit_admin_settings_path)
-          expect(Setting.new_setting_key).to eq 'New key value'
+          expect(Setting.new_setting_key).to be_nil
+        end
+
+        it 'creates a settings value that didnt exist before for eligible key' do
+          expect(Setting.site_extended_description).to be_blank
+
+          patch :update, params: { site_extended_description: 'New key value' }
+
+          expect(response).to redirect_to(edit_admin_settings_path)
+          expect(Setting.site_extended_description).to eq 'New key value'
         end
       end
 
       it 'updates a settings value' do
         Setting.site_title = 'Original'
-        patch :update, params: { id: 'site_title', setting: { value: 'New title' } }
+        patch :update, params: { site_title: 'New title' }
 
-        expect(response).to redirect_to(admin_settings_path)
+        expect(response).to redirect_to(edit_admin_settings_path)
         expect(Setting.site_title).to eq 'New title'
       end
 
       it 'typecasts open_registrations to boolean' do
         Setting.open_registrations = false
-        patch :update, params: { id: 'open_registrations', setting: { value: 'true' } }
+        patch :update, params: { open_registrations: 'true' }
 
-        expect(response).to redirect_to(admin_settings_path)
+        expect(response).to redirect_to(edit_admin_settings_path)
         expect(Setting.open_registrations).to eq true
       end
     end