From 059d8402720b591e4c81263f9a53f36d53ca1917 Mon Sep 17 00:00:00 2001 From: Aakash Bakhle <48802744+aakashb95@users.noreply.github.com> Date: Mon, 18 May 2026 14:08:26 +0530 Subject: [PATCH] feat: Refresh llm settings when superadmin configs change [AI-151] (#14388) # Pull Request Template ## Description fixes: https://linear.app/chatwoot/issue/AI-151/captains-super-admin-config-dont-get-applied-into-rails-without ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration. specs and locally To test locally: go to super admin -> settings -> captain -> Change endpoint to something incorrect go to local app -> captain -> playground -> try chatting (should fail due to incorrect endpoint) now in super admin captain settings, set the correct endpoint then chat in playground. Now it should work. Current develop code doesn't reflect the changes in installation config for captain instantly, needs a server restart. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published in downstream modules --- .../super_admin/app_configs_controller.rb | 17 ++++++++- .../installation_configs_controller.rb | 37 +++++++++++++++++++ app/models/installation_config.rb | 13 +++++++ .../super_admin/app_config_controller_spec.rb | 14 +++++++ .../installation_configs_controller_spec.rb | 30 +++++++++++++++ 5 files changed, 110 insertions(+), 1 deletion(-) diff --git a/app/controllers/super_admin/app_configs_controller.rb b/app/controllers/super_admin/app_configs_controller.rb index 67d58aef1..86d1b70ef 100644 --- a/app/controllers/super_admin/app_configs_controller.rb +++ b/app/controllers/super_admin/app_configs_controller.rb @@ -27,7 +27,7 @@ class SuperAdmin::AppConfigsController < SuperAdmin::ApplicationController if errors.any? redirect_to super_admin_app_config_path(config: @config), alert: errors.join(', ') else - redirect_to super_admin_settings_path, notice: "App Configs - #{@config.titleize} updated successfully" + redirect_to super_admin_settings_path, flash: success_flash end end @@ -58,6 +58,21 @@ class SuperAdmin::AppConfigsController < SuperAdmin::ApplicationController %w[ENABLE_ACCOUNT_SIGNUP FIREBASE_PROJECT_ID FIREBASE_CREDENTIALS WEBHOOK_TIMEOUT MAXIMUM_FILE_UPLOAD_SIZE WIDGET_TOKEN_EXPIRY] ) end + + def success_notice + message = "#{@config.titleize} settings updated successfully" + return message unless restart_required_config_saved? + + "#{message.delete_suffix('.')}. Restart Chatwoot web and worker processes to apply this change everywhere." + end + + def success_flash + restart_required_config_saved? ? { success: success_notice } : { notice: success_notice } + end + + def restart_required_config_saved? + params.fetch('app_config', {}).keys.intersect?(InstallationConfig::RESTART_REQUIRED_CONFIG_KEYS) + end end SuperAdmin::AppConfigsController.prepend_mod_with('SuperAdmin::AppConfigsController') diff --git a/app/controllers/super_admin/installation_configs_controller.rb b/app/controllers/super_admin/installation_configs_controller.rb index b1f15b518..e888b91e7 100644 --- a/app/controllers/super_admin/installation_configs_controller.rb +++ b/app/controllers/super_admin/installation_configs_controller.rb @@ -25,6 +25,29 @@ class SuperAdmin::InstallationConfigsController < SuperAdmin::ApplicationControl resource_class.editable end + def create + resource = new_resource(resource_params) + authorize_resource(resource) + + if resource.save + redirect_to after_resource_created_path(resource), flash: success_flash(resource) + else + render :new, locals: { + page: Administrate::Page::Form.new(dashboard, resource) + }, status: :unprocessable_entity + end + end + + def update + if requested_resource.update(resource_params) + redirect_to after_resource_updated_path(requested_resource), flash: success_flash(requested_resource) + else + render :edit, locals: { + page: Administrate::Page::Form.new(dashboard, requested_resource) + }, status: :unprocessable_entity + end + end + # Override `resource_params` if you want to transform the submitted # data before it's persisted. For example, the following would turn all # empty values into nil values. It uses other APIs such as `resource_class` @@ -42,6 +65,20 @@ class SuperAdmin::InstallationConfigsController < SuperAdmin::ApplicationControl .transform_values { |value| value == '' ? nil : value }.merge(locked: false) end + private + + def success_flash(resource) + message = translate_with_resource('update.success') + message = translate_with_resource('create.success') if action_name == 'create' + return { notice: message } unless restart_required_config?(resource) + + { success: "#{message.delete_suffix('.')}. Restart Chatwoot web and worker processes to apply this change everywhere." } + end + + def restart_required_config?(resource) + resource.name.in?(InstallationConfig::RESTART_REQUIRED_CONFIG_KEYS) + end + # See https://administrate-prototype.herokuapp.com/customizing_controller_actions # for more information end diff --git a/app/models/installation_config.rb b/app/models/installation_config.rb index 19252de50..ef349799f 100644 --- a/app/models/installation_config.rb +++ b/app/models/installation_config.rb @@ -15,6 +15,19 @@ # index_installation_configs_on_name_and_created_at (name,created_at) UNIQUE # class InstallationConfig < ApplicationRecord + CAPTAIN_LLM_CONFIG_KEYS = %w[ + CAPTAIN_OPEN_AI_API_KEY + CAPTAIN_OPEN_AI_ENDPOINT + CAPTAIN_OPEN_AI_MODEL + ].freeze + + RESTART_REQUIRED_CONFIG_KEYS = (CAPTAIN_LLM_CONFIG_KEYS + %w[ + LANGFUSE_BASE_URL + LANGFUSE_PUBLIC_KEY + LANGFUSE_SECRET_KEY + OTEL_PROVIDER + ]).freeze + # https://stackoverflow.com/questions/72970170/upgrading-to-rails-6-1-6-1-causes-psychdisallowedclass-tried-to-load-unspecif # https://discuss.rubyonrails.org/t/cve-2022-32224-possible-rce-escalation-bug-with-serialized-columns-in-active-record/81017 # FIX ME : fixes breakage of installation config. we need to migrate. diff --git a/spec/controllers/super_admin/app_config_controller_spec.rb b/spec/controllers/super_admin/app_config_controller_spec.rb index f517009eb..c57955dfc 100644 --- a/spec/controllers/super_admin/app_config_controller_spec.rb +++ b/spec/controllers/super_admin/app_config_controller_spec.rb @@ -38,10 +38,24 @@ RSpec.describe 'Super Admin Application Config API', type: :request do expect(response).to have_http_status(:found) expect(response).to redirect_to(super_admin_settings_path) + expect(flash[:notice]).to be_present + expect(flash[:alert]).to be_blank + expect(flash[:success]).to be_blank config = GlobalConfig.get('FB_APP_ID') expect(config['FB_APP_ID']).to eq('FB_APP_ID') end + + it 'asks admins to restart web and worker processes for runtime config changes' do + sign_in(super_admin, scope: :super_admin) + post '/super_admin/app_config?config=captain', params: { app_config: { CAPTAIN_OPEN_AI_ENDPOINT: 'https://api.openai.com' } } + + expect(response).to have_http_status(:found) + expect(response).to redirect_to(super_admin_settings_path) + expect(flash[:success]).to be_present + expect(flash[:alert]).to be_blank + expect(flash[:notice]).to be_blank + end end end end diff --git a/spec/controllers/super_admin/installation_configs_controller_spec.rb b/spec/controllers/super_admin/installation_configs_controller_spec.rb index f1b6c82d3..3a8f56d41 100644 --- a/spec/controllers/super_admin/installation_configs_controller_spec.rb +++ b/spec/controllers/super_admin/installation_configs_controller_spec.rb @@ -39,4 +39,34 @@ RSpec.describe 'Super Admin Installation Config API', type: :request do end end end + + describe 'PATCH /super_admin/installation_configs/:id' do + context 'when it is an authenticated super admin' do + it 'shows a regular success notice for config that does not require restart' do + sign_in(super_admin, scope: :super_admin) + config = create(:installation_config, name: 'TESTCONFIG', value: 'TESTVALUE', locked: false) + + patch "/super_admin/installation_configs/#{config.id}", params: { + installation_config: { name: config.name, value: 'UPDATEDVALUE' } + } + + expect(response).to have_http_status(:found) + expect(flash[:notice]).to be_present + expect(flash[:success]).to be_blank + end + + it 'shows a restart success notice for runtime config changes' do + sign_in(super_admin, scope: :super_admin) + config = create(:installation_config, name: 'OTEL_PROVIDER', value: 'langfuse', locked: false) + + patch "/super_admin/installation_configs/#{config.id}", params: { + installation_config: { name: config.name, value: 'langfuse' } + } + + expect(response).to have_http_status(:found) + expect(flash[:success]).to be_present + expect(flash[:notice]).to be_blank + end + end + end end