From d4242dd78ec3eaa10a7db5fd0453b9e12417a209 Mon Sep 17 00:00:00 2001 From: Riccardo Graziosi <31478034+riggraz@users.noreply.github.com> Date: Sat, 4 Feb 2023 15:43:15 +0100 Subject: [PATCH] Move tenant settings on separate model (#196) --- app/controllers/application_controller.rb | 1 + app/controllers/tenants_controller.rb | 8 ++++++- app/javascript/actions/Tenant/updateTenant.ts | 17 ++++++++++---- .../General/GeneralSiteSettingsP.tsx | 18 +++++++-------- .../containers/GeneralSiteSettings.tsx | 5 +++-- app/javascript/interfaces/ITenant.ts | 13 ----------- app/javascript/interfaces/ITenantSetting.ts | 17 ++++++++++++++ app/models/tenant.rb | 10 +++------ app/models/tenant_setting.rb | 12 ++++++++++ app/policies/tenant_policy.rb | 2 +- app/policies/tenant_setting_policy.rb | 13 +++++++++++ app/views/layouts/_header.html.erb | 6 ++--- app/views/site_settings/general.html.erb | 2 +- .../20230131194858_create_tenant_settings.rb | 14 ++++++++++++ db/schema.rb | 12 ++++++++-- spec/factories/tenant_settings.rb | 6 +++++ spec/models/tenant_setting_spec.rb | 22 +++++++++++++++++++ 17 files changed, 135 insertions(+), 43 deletions(-) create mode 100644 app/javascript/interfaces/ITenantSetting.ts create mode 100644 app/models/tenant_setting.rb create mode 100644 app/policies/tenant_setting_policy.rb create mode 100644 db/migrate/20230131194858_create_tenant_settings.rb create mode 100644 spec/factories/tenant_settings.rb create mode 100644 spec/models/tenant_setting_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bb4af517..39de667a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -41,6 +41,7 @@ class ApplicationController < ActionController::Base # Load tenant data @tenant = Current.tenant_or_raise! + @tenant_setting = TenantSetting.first_or_create @boards = Board.select(:id, :name).order(order: :asc) # Setup locale diff --git a/app/controllers/tenants_controller.rb b/app/controllers/tenants_controller.rb index 42982979..d6c352e8 100644 --- a/app/controllers/tenants_controller.rb +++ b/app/controllers/tenants_controller.rb @@ -70,6 +70,12 @@ class TenantsController < ApplicationController def tenant_update_params params .require(:tenant) - .permit(policy(@tenant).permitted_attributes_for_update) + .permit( + policy(@tenant) + .permitted_attributes_for_update + .concat([{ + tenant_setting_attributes: policy(@tenant.tenant_setting).permitted_attributes_for_update + }]) # in order to permit nested attributes for tenant_setting + ) end end \ No newline at end of file diff --git a/app/javascript/actions/Tenant/updateTenant.ts b/app/javascript/actions/Tenant/updateTenant.ts index 9ff3ed14..9653c084 100644 --- a/app/javascript/actions/Tenant/updateTenant.ts +++ b/app/javascript/actions/Tenant/updateTenant.ts @@ -3,6 +3,7 @@ import { ThunkAction } from "redux-thunk"; import HttpStatus from "../../constants/http_status"; import buildRequestHeaders from "../../helpers/buildRequestHeaders"; +import ITenantSetting from "../../interfaces/ITenantSetting"; import ITenantJSON from "../../interfaces/json/ITenant"; import { State } from "../../reducers/rootReducer"; @@ -47,7 +48,7 @@ const tenantUpdateFailure = (error: string): TenantUpdateFailureAction => ({ interface UpdateTenantParams { siteName?: string; siteLogo?: string; - brandDisplaySetting?: string; + tenantSetting?: ITenantSetting; locale?: string; authenticityToken: string; } @@ -55,7 +56,7 @@ interface UpdateTenantParams { export const updateTenant = ({ siteName = null, siteLogo = null, - brandDisplaySetting = null, + tenantSetting = null, locale = null, authenticityToken, }: UpdateTenantParams): ThunkAction> => async (dispatch) => { @@ -64,15 +65,23 @@ export const updateTenant = ({ const tenant = Object.assign({}, siteName !== null ? { site_name: siteName } : null, siteLogo !== null ? { site_logo: siteLogo } : null, - brandDisplaySetting !== null ? { brand_display_setting: brandDisplaySetting } : null, locale !== null ? { locale } : null ); try { + const body = JSON.stringify({ + tenant: { + ...tenant, + tenant_setting_attributes: tenantSetting, + }, + }); + + console.log(body) + const res = await fetch(`/tenants/0`, { method: 'PATCH', headers: buildRequestHeaders(authenticityToken), - body: JSON.stringify({ tenant }), + body, }); const json = await res.json(); diff --git a/app/javascript/components/SiteSettings/General/GeneralSiteSettingsP.tsx b/app/javascript/components/SiteSettings/General/GeneralSiteSettingsP.tsx index 6c0401e9..710b41c5 100644 --- a/app/javascript/components/SiteSettings/General/GeneralSiteSettingsP.tsx +++ b/app/javascript/components/SiteSettings/General/GeneralSiteSettingsP.tsx @@ -7,11 +7,11 @@ import SiteSettingsInfoBox from '../../common/SiteSettingsInfoBox'; import Button from '../../common/Button'; import HttpStatus from '../../../constants/http_status'; import { - TENANT_BRAND_NAME_AND_LOGO, - TENANT_BRAND_NAME_ONLY, - TENANT_BRAND_LOGO_ONLY, - TENANT_BRAND_NONE, -} from '../../../interfaces/ITenant'; + TENANT_SETTING_BRAND_DISPLAY_NAME_AND_LOGO, + TENANT_SETTING_BRAND_DISPLAY_NAME_ONLY, + TENANT_SETTING_BRAND_DISPLAY_LOGO_ONLY, + TENANT_SETTING_BRAND_DISPLAY_NONE, +} from '../../../interfaces/ITenantSetting'; import { DangerText } from '../../common/CustomTexts'; import { getLabel, getValidationMessage } from '../../../helpers/formUtils'; @@ -105,16 +105,16 @@ const GeneralSiteSettingsP = ({ id="brandSetting" className="selectPicker" > - - - - diff --git a/app/javascript/containers/GeneralSiteSettings.tsx b/app/javascript/containers/GeneralSiteSettings.tsx index 13190149..3fdbcaeb 100644 --- a/app/javascript/containers/GeneralSiteSettings.tsx +++ b/app/javascript/containers/GeneralSiteSettings.tsx @@ -3,6 +3,7 @@ import { connect } from "react-redux"; import GeneralSiteSettingsP from "../components/SiteSettings/General/GeneralSiteSettingsP"; import { updateTenant } from "../actions/Tenant/updateTenant"; import { State } from "../reducers/rootReducer"; +import { TenantSettingBrandDisplay } from "../interfaces/ITenantSetting"; const mapStateToProps = (state: State) => ({ areUpdating: state.siteSettings.general.areUpdating, @@ -13,14 +14,14 @@ const mapDispatchToProps = (dispatch: any) => ({ updateTenant( siteName: string, siteLogo: string, - brandDisplaySetting: string, + brandDisplaySetting: TenantSettingBrandDisplay, locale: string, authenticityToken: string ): Promise { return dispatch(updateTenant({ siteName, siteLogo, - brandDisplaySetting, + tenantSetting: { brand_display: brandDisplaySetting }, locale, authenticityToken, })); diff --git a/app/javascript/interfaces/ITenant.ts b/app/javascript/interfaces/ITenant.ts index 369d691e..9ddf6b11 100644 --- a/app/javascript/interfaces/ITenant.ts +++ b/app/javascript/interfaces/ITenant.ts @@ -1,20 +1,7 @@ -// Brand display setting -export const TENANT_BRAND_NAME_AND_LOGO = 'name_and_logo'; -export const TENANT_BRAND_NAME_ONLY = 'name_only'; -export const TENANT_BRAND_LOGO_ONLY = 'logo_only'; -export const TENANT_BRAND_NONE = 'no_name_no_logo'; - -export type TenantBrandDisplaySetting = - typeof TENANT_BRAND_NAME_AND_LOGO | - typeof TENANT_BRAND_NAME_ONLY | - typeof TENANT_BRAND_LOGO_ONLY | - typeof TENANT_BRAND_NONE; - interface ITenant { id: number; siteName: string; siteLogo: string; - brandDisplaySetting: TenantBrandDisplaySetting; locale: string; } diff --git a/app/javascript/interfaces/ITenantSetting.ts b/app/javascript/interfaces/ITenantSetting.ts new file mode 100644 index 00000000..286dc598 --- /dev/null +++ b/app/javascript/interfaces/ITenantSetting.ts @@ -0,0 +1,17 @@ +// Brand display setting +export const TENANT_SETTING_BRAND_DISPLAY_NAME_AND_LOGO = 'name_and_logo'; +export const TENANT_SETTING_BRAND_DISPLAY_NAME_ONLY = 'name_only'; +export const TENANT_SETTING_BRAND_DISPLAY_LOGO_ONLY = 'logo_only'; +export const TENANT_SETTING_BRAND_DISPLAY_NONE = 'no_name_no_logo'; + +export type TenantSettingBrandDisplay = + typeof TENANT_SETTING_BRAND_DISPLAY_NAME_AND_LOGO | + typeof TENANT_SETTING_BRAND_DISPLAY_NAME_ONLY | + typeof TENANT_SETTING_BRAND_DISPLAY_LOGO_ONLY | + typeof TENANT_SETTING_BRAND_DISPLAY_NONE; + +interface ITenantSetting { + brand_display?: TenantSettingBrandDisplay; +} + +export default ITenantSetting; \ No newline at end of file diff --git a/app/models/tenant.rb b/app/models/tenant.rb index 66389269..ff8ffe28 100644 --- a/app/models/tenant.rb +++ b/app/models/tenant.rb @@ -1,17 +1,11 @@ class Tenant < ApplicationRecord + has_one :tenant_setting has_many :boards has_many :o_auths has_many :post_statuses has_many :posts has_many :users - enum brand_display_setting: [ - :name_and_logo, - :name_only, - :logo_only, - :no_name_no_logo - ] - enum status: [:active, :pending, :blocked] after_initialize :set_default_status, if: :new_record? @@ -19,6 +13,8 @@ class Tenant < ApplicationRecord validates :site_name, presence: true validates :subdomain, presence: true, uniqueness: true + accepts_nested_attributes_for :tenant_setting, update_only: true + def set_default_status self.status ||= :pending end diff --git a/app/models/tenant_setting.rb b/app/models/tenant_setting.rb new file mode 100644 index 00000000..2620ca45 --- /dev/null +++ b/app/models/tenant_setting.rb @@ -0,0 +1,12 @@ +class TenantSetting < ApplicationRecord + include TenantOwnable + + belongs_to :tenant + + enum brand_display: [ + :name_and_logo, + :name_only, + :logo_only, + :no_name_no_logo + ] +end diff --git a/app/policies/tenant_policy.rb b/app/policies/tenant_policy.rb index d17432b8..9a3b81a0 100644 --- a/app/policies/tenant_policy.rb +++ b/app/policies/tenant_policy.rb @@ -5,7 +5,7 @@ class TenantPolicy < ApplicationPolicy def permitted_attributes_for_update if user.admin? - [:site_name, :site_logo, :brand_display_setting, :locale] + [:site_name, :site_logo, :locale] else [] end diff --git a/app/policies/tenant_setting_policy.rb b/app/policies/tenant_setting_policy.rb new file mode 100644 index 00000000..ae33df85 --- /dev/null +++ b/app/policies/tenant_setting_policy.rb @@ -0,0 +1,13 @@ +class TenantSettingPolicy < ApplicationPolicy + def permitted_attributes_for_update + if user.admin? + [:brand_display] + else + [] + end + end + + def update? + user.admin? and user.tenant_id == record.id + end +end \ No newline at end of file diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb index 3242ab24..70909dcd 100644 --- a/app/views/layouts/_header.html.erb +++ b/app/views/layouts/_header.html.erb @@ -5,11 +5,11 @@ app_name = content_tag :span, @tenant.site_name logo = image_tag(@tenant.site_logo ? @tenant.site_logo : "", class: 'logo') - if @tenant.brand_display_setting == "name_and_logo" + if @tenant_setting.brand_display == "name_and_logo" logo + app_name - elsif @tenant.brand_display_setting == "name_only" + elsif @tenant_setting.brand_display == "name_only" app_name - elsif @tenant.brand_display_setting == "logo_only" + elsif @tenant_setting.brand_display == "logo_only" logo end end diff --git a/app/views/site_settings/general.html.erb b/app/views/site_settings/general.html.erb index 1a24311c..97fb0060 100644 --- a/app/views/site_settings/general.html.erb +++ b/app/views/site_settings/general.html.erb @@ -8,7 +8,7 @@ originForm: { siteName: @tenant.site_name, siteLogo: @tenant.site_logo, - brandDisplaySetting: @tenant.brand_display_setting, + brandDisplaySetting: @tenant_setting.brand_display, locale: @tenant.locale }, authenticityToken: form_authenticity_token diff --git a/db/migrate/20230131194858_create_tenant_settings.rb b/db/migrate/20230131194858_create_tenant_settings.rb new file mode 100644 index 00000000..7efd9fce --- /dev/null +++ b/db/migrate/20230131194858_create_tenant_settings.rb @@ -0,0 +1,14 @@ +class CreateTenantSettings < ActiveRecord::Migration[6.0] + def change + create_table :tenant_settings do |t| + t.integer :brand_display, null: false, default: 0 + t.references :tenant, null: false, foreign_key: true + + t.timestamps + end + + change_table :tenants do |t| + t.remove :brand_display_setting + end + end +end diff --git a/db/schema.rb b/db/schema.rb index a8b5c59e..e97213e5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_07_27_094200) do +ActiveRecord::Schema.define(version: 2023_01_31_194858) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -124,6 +124,14 @@ ActiveRecord::Schema.define(version: 2022_07_27_094200) do t.index ["user_id"], name: "index_posts_on_user_id" end + create_table "tenant_settings", force: :cascade do |t| + t.integer "brand_display", default: 0, null: false + t.bigint "tenant_id", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["tenant_id"], name: "index_tenant_settings_on_tenant_id" + end + create_table "tenants", force: :cascade do |t| t.string "site_name", null: false t.string "site_logo" @@ -132,7 +140,6 @@ ActiveRecord::Schema.define(version: 2022_07_27_094200) do t.string "custom_url" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false - t.integer "brand_display_setting", default: 0 t.integer "status" end @@ -180,5 +187,6 @@ ActiveRecord::Schema.define(version: 2022_07_27_094200) do add_foreign_key "posts", "post_statuses" add_foreign_key "posts", "tenants" add_foreign_key "posts", "users" + add_foreign_key "tenant_settings", "tenants" add_foreign_key "users", "tenants" end diff --git a/spec/factories/tenant_settings.rb b/spec/factories/tenant_settings.rb new file mode 100644 index 00000000..1f659df7 --- /dev/null +++ b/spec/factories/tenant_settings.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :tenant_setting do + brand_display { 0 } + tenant + end +end diff --git a/spec/models/tenant_setting_spec.rb b/spec/models/tenant_setting_spec.rb new file mode 100644 index 00000000..11204d9e --- /dev/null +++ b/spec/models/tenant_setting_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +RSpec.describe TenantSetting, type: :model do + let(:tenant_setting) { FactoryBot.build(:tenant_setting) } + + it 'should be valid' do + expect(tenant_setting).to be_valid + end + + it 'has a setting brand_display' do + expect(tenant_setting.brand_display).to eq('name_and_logo') + + tenant_setting.brand_display = 'name_only' + expect(tenant_setting).to be_valid + + tenant_setting.brand_display = 'logo_only' + expect(tenant_setting).to be_valid + + tenant_setting.brand_display = 'no_name_no_logo' + expect(tenant_setting).to be_valid + end +end