feat: add GlobalConfigService to support env vars migration (#3288)
This commit is contained in:
parent
03b1a3d045
commit
97ee1bfa97
7 changed files with 104 additions and 39 deletions
|
@ -55,7 +55,7 @@ class Api::V1::AccountsController < Api::BaseController
|
|||
end
|
||||
|
||||
def check_signup_enabled
|
||||
raise ActionController::RoutingError, 'Not Found' if ENV.fetch('ENABLE_ACCOUNT_SIGNUP', true) == 'false'
|
||||
raise ActionController::RoutingError, 'Not Found' if GlobalConfig.get_value('ENABLE_ACCOUNT_SIGNUP') == 'false'
|
||||
end
|
||||
|
||||
def pundit_user
|
||||
|
|
|
@ -27,7 +27,8 @@ class DashboardController < ActionController::Base
|
|||
'ANALYTICS_TOKEN',
|
||||
'ANALYTICS_HOST'
|
||||
).merge(
|
||||
APP_VERSION: Chatwoot.config[:version]
|
||||
APP_VERSION: Chatwoot.config[:version],
|
||||
ENABLE_ACCOUNT_SIGNUP: GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false')
|
||||
)
|
||||
end
|
||||
|
||||
|
|
|
@ -34,7 +34,7 @@
|
|||
window.chatwootConfig = {
|
||||
hostURL: '<%= ENV.fetch('FRONTEND_URL', '') %>',
|
||||
fbAppId: '<%= ENV.fetch('FB_APP_ID', nil) %>',
|
||||
signupEnabled: '<%= ENV.fetch('ENABLE_ACCOUNT_SIGNUP', true) %>',
|
||||
signupEnabled: '<%= @global_config['ENABLE_ACCOUNT_SIGNUP'] %>',
|
||||
<% if ENV['VAPID_PUBLIC_KEY'] %>
|
||||
vapidPublicKey: new Uint8Array(<%= Base64.urlsafe_decode64(ENV['VAPID_PUBLIC_KEY']).bytes %>),
|
||||
<% end %>
|
||||
|
|
|
@ -15,6 +15,10 @@ class GlobalConfig
|
|||
config.with_indifferent_access
|
||||
end
|
||||
|
||||
def get_value(arg)
|
||||
load_from_cache(arg)
|
||||
end
|
||||
|
||||
def clear_cache
|
||||
cached_keys = $alfred.keys("#{VERSION}:#{KEY_PREFIX}:*")
|
||||
(cached_keys || []).each do |cached_key|
|
||||
|
|
17
lib/global_config_service.rb
Normal file
17
lib/global_config_service.rb
Normal file
|
@ -0,0 +1,17 @@
|
|||
class GlobalConfigService
|
||||
def self.load(config_key, default_value)
|
||||
config = GlobalConfig.get(config_key)
|
||||
return config[config_key] if config[config_key].present?
|
||||
|
||||
# To support migrating existing instance relying on env variables
|
||||
# TODO: deprecate this later down the line
|
||||
config_value = ENV[config_key] || default_value
|
||||
|
||||
return if config_value.blank?
|
||||
|
||||
i = InstallationConfig.where(name: config_key).first_or_create(value: config_value)
|
||||
# To clear a nil value that might have been cached in the previous call
|
||||
GlobalConfig.clear_cache
|
||||
i.value
|
||||
end
|
||||
end
|
|
@ -5,6 +5,11 @@ RSpec.describe 'Accounts API', type: :request do
|
|||
let(:email) { Faker::Internet.email }
|
||||
let(:user_full_name) { Faker::Name.name_with_middle }
|
||||
|
||||
before do
|
||||
# to clear redis cache
|
||||
GlobalConfig.clear_cache
|
||||
end
|
||||
|
||||
context 'when posting to accounts with correct parameters' do
|
||||
let(:account_builder) { double }
|
||||
let(:account) { create(:account) }
|
||||
|
@ -15,62 +20,56 @@ RSpec.describe 'Accounts API', type: :request do
|
|||
end
|
||||
|
||||
it 'calls account builder' do
|
||||
with_modified_env ENABLE_ACCOUNT_SIGNUP: nil do
|
||||
allow(account_builder).to receive(:perform).and_return([user, account])
|
||||
GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'nil')
|
||||
allow(account_builder).to receive(:perform).and_return([user, account])
|
||||
|
||||
params = { account_name: 'test', email: email, user: nil, user_full_name: user_full_name, password: 'Password1!' }
|
||||
params = { account_name: 'test', email: email, user: nil, user_full_name: user_full_name, password: 'Password1!' }
|
||||
|
||||
post api_v1_accounts_url,
|
||||
params: params,
|
||||
as: :json
|
||||
post api_v1_accounts_url,
|
||||
params: params,
|
||||
as: :json
|
||||
|
||||
expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password]))
|
||||
expect(account_builder).to have_received(:perform)
|
||||
expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid')
|
||||
end
|
||||
expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password]))
|
||||
expect(account_builder).to have_received(:perform)
|
||||
expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid')
|
||||
end
|
||||
|
||||
it 'renders error response on invalid params' do
|
||||
with_modified_env ENABLE_ACCOUNT_SIGNUP: nil do
|
||||
allow(account_builder).to receive(:perform).and_return(nil)
|
||||
GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'nil')
|
||||
allow(account_builder).to receive(:perform).and_return(nil)
|
||||
|
||||
params = { account_name: nil, email: nil, user: nil, user_full_name: nil }
|
||||
params = { account_name: nil, email: nil, user: nil, user_full_name: nil }
|
||||
|
||||
post api_v1_accounts_url,
|
||||
params: params,
|
||||
as: :json
|
||||
post api_v1_accounts_url,
|
||||
params: params,
|
||||
as: :json
|
||||
|
||||
expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password]))
|
||||
expect(account_builder).to have_received(:perform)
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json)
|
||||
end
|
||||
expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password]))
|
||||
expect(account_builder).to have_received(:perform)
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when ENABLE_ACCOUNT_SIGNUP env variable is set to false' do
|
||||
it 'responds 404 on requests' do
|
||||
params = { account_name: 'test', email: email, user_full_name: user_full_name }
|
||||
with_modified_env ENABLE_ACCOUNT_SIGNUP: 'false' do
|
||||
post api_v1_accounts_url,
|
||||
params: params,
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:not_found)
|
||||
end
|
||||
params = { account_name: 'test', email: email, user_full_name: user_full_name, password: 'Password1!' }
|
||||
GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false')
|
||||
post api_v1_accounts_url,
|
||||
params: params,
|
||||
as: :json
|
||||
expect(response).to have_http_status(:not_found)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when ENABLE_ACCOUNT_SIGNUP env variable is set to api_only' do
|
||||
it 'does not respond 404 on requests' do
|
||||
params = { account_name: 'test', email: email, user_full_name: user_full_name, password: 'Password1!' }
|
||||
with_modified_env ENABLE_ACCOUNT_SIGNUP: 'api_only' do
|
||||
post api_v1_accounts_url,
|
||||
params: params,
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'api_only')
|
||||
post api_v1_accounts_url,
|
||||
params: params,
|
||||
as: :json
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
44
spec/lib/global_config_service_spec.rb
Normal file
44
spec/lib/global_config_service_spec.rb
Normal file
|
@ -0,0 +1,44 @@
|
|||
require 'rails_helper'
|
||||
|
||||
describe GlobalConfigService do
|
||||
subject(:trigger) { described_class }
|
||||
|
||||
describe 'execute' do
|
||||
context 'when called with default options' do
|
||||
before do
|
||||
# to clear redis cache
|
||||
GlobalConfig.clear_cache
|
||||
end
|
||||
|
||||
it 'set default value if not found on db nor env var' do
|
||||
value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
|
||||
expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq nil
|
||||
|
||||
described_class.load('ENABLE_ACCOUNT_SIGNUP', 'true')
|
||||
|
||||
value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
|
||||
expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'true'
|
||||
expect(InstallationConfig.find_by(name: 'ENABLE_ACCOUNT_SIGNUP')&.value).to eq 'true'
|
||||
end
|
||||
|
||||
it 'get value from env variable if not found on DB' do
|
||||
with_modified_env ENABLE_ACCOUNT_SIGNUP: 'false' do
|
||||
expect(InstallationConfig.find_by(name: 'ENABLE_ACCOUNT_SIGNUP')&.value).to eq nil
|
||||
described_class.load('ENABLE_ACCOUNT_SIGNUP', 'true')
|
||||
value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
|
||||
expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'false'
|
||||
end
|
||||
end
|
||||
|
||||
it 'get value from DB if found' do
|
||||
# Set a value in db first and make sure this value
|
||||
# is not respected even when load() method is called with
|
||||
# another value.
|
||||
InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').first_or_create(value: 'true')
|
||||
described_class.load('ENABLE_ACCOUNT_SIGNUP', 'false')
|
||||
value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
|
||||
expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'true'
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in a new issue