chore: revert GlobalConfig changes (#3702)

Priority to GlobalConfig changes are reverted and for the time being, env vars will take precedence if set.

Fixes #3699

app/controllers/api/v1/accounts_controller.rb

@@ -55,7 +55,7 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def check_signup_enabled
-    raise ActionController::RoutingError, 'Not Found' if GlobalConfig.get_value('ENABLE_ACCOUNT_SIGNUP') == 'false'
+    raise ActionController::RoutingError, 'Not Found' if GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false') == 'false'
   end
 
   def pundit_user

lib/global_config_service.rb

@@ -1,7 +1,7 @@
 class GlobalConfigService
   def self.load(config_key, default_value)
-    config = GlobalConfig.get(config_key)
+    config = ENV[config_key] || GlobalConfig.get(config_key)[config_key]
-    return config[config_key] if config[config_key].present?
+    return config if config.present?
 
     # To support migrating existing instance relying on env variables
     # TODO: deprecate this later down the line

spec/controllers/api/v1/accounts_controller_spec.rb

@@ -5,11 +5,6 @@ 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) }
@@ -20,56 +15,62 @@ RSpec.describe 'Accounts API', type: :request do
       end
 
       it 'calls account builder' do
-        GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'nil')
+        with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do
-        allow(account_builder).to receive(:perform).and_return([user, account])
+          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,
+          post api_v1_accounts_url,
-             params: params,
+               params: params,
-             as: :json
+               as: :json
 
-        expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password]))
+          expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password]))
-        expect(account_builder).to have_received(:perform)
+          expect(account_builder).to have_received(:perform)
-        expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid')
+          expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid')
+        end
       end
 
       it 'renders error response on invalid params' do
-        GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'nil')
+        with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do
-        allow(account_builder).to receive(:perform).and_return(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,
+          post api_v1_accounts_url,
-             params: params,
+               params: params,
-             as: :json
+               as: :json
 
-        expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password]))
+          expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password]))
-        expect(account_builder).to have_received(:perform)
+          expect(account_builder).to have_received(:perform)
-        expect(response).to have_http_status(:forbidden)
+          expect(response).to have_http_status(:forbidden)
-        expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json)
+          expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json)
+        end
       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, password: 'Password1!' }
+        params = { account_name: 'test', email: email, user_full_name: user_full_name }
-        GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false')
+        with_modified_env ENABLE_ACCOUNT_SIGNUP: 'false' do
-        post api_v1_accounts_url,
+          post api_v1_accounts_url,
-             params: params,
+               params: params,
-             as: :json
+               as: :json
-        expect(response).to have_http_status(:not_found)
+
+          expect(response).to have_http_status(:not_found)
+        end
       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!' }
-        GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'api_only')
+        with_modified_env ENABLE_ACCOUNT_SIGNUP: 'api_only' do
-        post api_v1_accounts_url,
+          post api_v1_accounts_url,
-             params: params,
+               params: params,
-             as: :json
+               as: :json
-        expect(response).to have_http_status(:success)
+
+          expect(response).to have_http_status(:success)
+        end
       end
     end
   end

spec/lib/global_config_service_spec.rb

@@ -10,35 +10,34 @@ describe GlobalConfigService do
         GlobalConfig.clear_cache
       end
 
-      it 'set default value if not found on db nor env var' do
+      # it 'set default value if not found on db nor env var' do
-        value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
+      #   value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
-        expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq nil
+      #   expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq nil
 
-        described_class.load('ENABLE_ACCOUNT_SIGNUP', 'true')
+      #   described_class.load('ENABLE_ACCOUNT_SIGNUP', 'true')
 
-        value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
+      #   value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
-        expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'true'
+      #   expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'true'
-        expect(InstallationConfig.find_by(name: 'ENABLE_ACCOUNT_SIGNUP')&.value).to eq 'true'
+      #   expect(InstallationConfig.find_by(name: 'ENABLE_ACCOUNT_SIGNUP')&.value).to eq 'true'
-      end
+      # end
 
-      it 'get value from env variable if not found on DB' do
+      it 'get value from env variable even if present 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 = described_class.load('ENABLE_ACCOUNT_SIGNUP', 'true')
-          value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
+          expect(value).to eq 'false'
-          expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'false'
         end
       end
 
-      it 'get value from DB if found' do
+      # it 'get value from DB if found' do
-        # Set a value in db first and make sure this value
+      #   # Set a value in db first and make sure this value
-        # is not respected even when load() method is called with
+      #   # is not respected even when load() method is called with
-        # another value.
+      #   # another value.
-        InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').first_or_create(value: 'true')
+      #   InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').first_or_create(value: 'true')
-        described_class.load('ENABLE_ACCOUNT_SIGNUP', 'false')
+      #   described_class.load('ENABLE_ACCOUNT_SIGNUP', 'false')
-        value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
+      #   value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP')
-        expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'true'
+      #   expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'true'
-      end
+      # end
     end
   end
 end