chore: add specs

app/controllers/github/callbacks_controller.rb

@@ -2,6 +2,9 @@ class Github::CallbacksController < ApplicationController
   include Github::IntegrationHelper
 
   def show
+    # Validate account context early for all flows that require it
+    account if params[:code].present?
+
     if params[:installation_id].present? && params[:code].present?
       # Both installation and OAuth code present - handle both
       handle_installation_with_oauth
@@ -78,36 +81,38 @@ class Github::CallbacksController < ApplicationController
   end
 
   def handle_response(installation_id = nil)
+    settings = build_hook_settings(installation_id)
+    hook = create_integration_hook(settings)
+    hook.save!
+
+    cleanup_session_data
+    redirect_to github_redirect_uri
+  rescue StandardError => e
+    Rails.logger.error("Github callback error: #{e.message}")
+    redirect_to fallback_redirect_uri
+  end
+
+  def build_hook_settings(installation_id)
     settings = {
       token_type: parsed_body['token_type'],
       scope: parsed_body['scope']
     }
 
-    # Add installation_id from parameter or session
-    if installation_id
-      settings[:installation_id] = installation_id
-    elsif session[:github_installation_id]
-      settings[:installation_id] = session[:github_installation_id]
+    settings[:installation_id] = installation_id || session[:github_installation_id]
+    settings.compact
   end
 
-    # Use account from state parameter - this should work for both flows now
-    target_account = account
-
-    hook = target_account.hooks.new(
+  def create_integration_hook(settings)
+    account.hooks.new(
       access_token: parsed_body['access_token'],
       status: 'enabled',
       app_id: 'github',
       settings: settings
     )
-    hook.save!
+  end
 
-    # Clear session data
+  def cleanup_session_data
     session.delete(:github_installation_id)
-
-    redirect_to github_redirect_uri
-  rescue StandardError => e
-    Rails.logger.error("Github callback error: #{e.message}")
-    redirect_to fallback_redirect_uri
   end
 
   def account

app/models/integrations/app.rb

@@ -63,7 +63,7 @@ class Integrations::App
     when 'linear'
       GlobalConfigService.load('LINEAR_CLIENT_ID', nil).present?
     when 'github'
-      GlobalConfigService.load('GITHUB_CLIENT_ID', nil).present? && GlobalConfigService.load('GITHUB_CLIENT_SECRET', nil).present?
+      github_enabled?
     when 'shopify'
       shopify_enabled?(account)
     when 'leadsquared'
@@ -143,6 +143,10 @@ class Integrations::App
 
   private
 
+  def github_enabled?
+    GlobalConfigService.load('GITHUB_CLIENT_ID', nil).present? && GlobalConfigService.load('GITHUB_CLIENT_SECRET', nil).present?
+  end
+
   def shopify_enabled?(account)
     account.feature_enabled?('shopify_integration') && GlobalConfigService.load('SHOPIFY_CLIENT_ID', nil).present?
   end

spec/controllers/github/callbacks_controller_spec.rb

@@ -0,0 +1,204 @@
+require 'rails_helper'
+
+RSpec.describe Github::CallbacksController do
+  let(:account) { create(:account) }
+  let(:signed_account_id) { account.to_signed_global_id(expires_in: 1.hour) }
+
+  before do
+    allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_ID', nil).and_return('test_client_id')
+    allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_SECRET', nil).and_return('test_client_secret')
+
+    # Configure test environment
+    @original_frontend_url = ENV.fetch('FRONTEND_URL', nil)
+    ENV['FRONTEND_URL'] = 'http://localhost:3000'
+
+    # Configure request.host to match our test URL
+    allow_any_instance_of(ActionDispatch::Request).to receive(:host).and_return('localhost')
+    allow_any_instance_of(ActionDispatch::Request).to receive(:port).and_return(3000)
+    allow_any_instance_of(ActionDispatch::Request).to receive(:protocol).and_return('http://')
+
+    # Mock all GitHub OAuth API calls to prevent WebMock errors
+    stub_request(:post, 'https://github.com/login/oauth/access_token')
+      .to_return(status: 200, body: '', headers: {})
+  end
+
+  after do
+    ENV['FRONTEND_URL'] = @original_frontend_url
+  end
+
+  describe 'GET #show' do
+    context 'when handling installation with OAuth (both installation_id and code present)' do
+      let(:oauth_response) do
+        double('OAuth2::Response', response: double(parsed: {
+                                                      'access_token' => 'test_token',
+                                                      'token_type' => 'bearer',
+                                                      'scope' => 'repo,read:org'
+                                                    }))
+      end
+
+      before do
+        oauth_client = double('OAuth2::Client')
+        auth_code = double('OAuth2::Strategy::AuthCode')
+        allow(controller).to receive(:oauth_client).and_return(oauth_client)
+        allow(oauth_client).to receive(:auth_code).and_return(auth_code)
+        allow(auth_code).to receive(:get_token).and_return(oauth_response)
+      end
+
+      it 'creates integration hook with installation_id and redirects' do
+        get :show, params: {
+          code: 'test_code',
+          installation_id: '12345',
+          setup_action: 'install',
+          state: signed_account_id
+        }
+
+        hook = account.hooks.find_by(app_id: 'github')
+        expect(hook).to be_present
+        expect(hook.access_token).to eq('test_token')
+        expect(hook.settings['installation_id']).to eq('12345')
+        expect(hook.settings['token_type']).to eq('bearer')
+        expect(hook.settings['scope']).to eq('repo,read:org')
+        expect(response).to redirect_to(a_string_including('/settings/integrations/github'))
+      end
+    end
+
+    context 'when handling installation only (only installation_id present)' do
+      it 'redirects to OAuth URL' do
+        get :show, params: {
+          installation_id: '12345',
+          setup_action: 'install',
+          state: signed_account_id
+        }
+
+        expect(response).to be_redirect
+        expect(response.location).to include('github?setup_action=install&installation_id=12345')
+      end
+    end
+
+    context 'when handling authorization only (only code present)' do
+      let(:oauth_response) do
+        double('OAuth2::Response', response: double(parsed: {
+                                                      'access_token' => 'test_token',
+                                                      'token_type' => 'bearer',
+                                                      'scope' => 'repo,read:org'
+                                                    }))
+      end
+
+      before do
+        oauth_client = double('OAuth2::Client')
+        auth_code = double('OAuth2::Strategy::AuthCode')
+        allow(controller).to receive(:oauth_client).and_return(oauth_client)
+        allow(oauth_client).to receive(:auth_code).and_return(auth_code)
+        allow(auth_code).to receive(:get_token).and_return(oauth_response)
+      end
+
+      it 'creates integration hook without installation_id and redirects' do
+        get :show, params: {
+          code: 'test_code',
+          state: signed_account_id
+        }
+
+        hook = account.hooks.find_by(app_id: 'github')
+        expect(hook).to be_present
+        expect(hook.access_token).to eq('test_token')
+        expect(hook.settings['installation_id']).to be_nil
+        expect(response).to redirect_to(a_string_including('/settings/integrations/github'))
+      end
+    end
+
+    context 'when state parameter is missing' do
+      it 'handles error gracefully and redirects to fallback URI' do
+        get :show, params: {
+          code: 'test_code',
+          installation_id: '12345',
+          setup_action: 'install'
+        }
+
+        expect(response).to be_redirect
+      end
+    end
+
+    context 'when state parameter is invalid' do
+      it 'handles error gracefully and redirects to fallback URI' do
+        get :show, params: {
+          code: 'test_code',
+          installation_id: '12345',
+          setup_action: 'install',
+          state: 'invalid_state'
+        }
+
+        expect(response).to be_redirect
+      end
+    end
+
+    context 'when OAuth fails' do
+      before do
+        oauth_client = double('OAuth2::Client')
+        auth_code = double('OAuth2::Strategy::AuthCode')
+        allow(controller).to receive(:oauth_client).and_return(oauth_client)
+        allow(oauth_client).to receive(:auth_code).and_return(auth_code)
+        allow(auth_code).to receive(:get_token).and_raise(StandardError, 'OAuth failed')
+      end
+
+      it 'logs error and redirects to fallback URI' do
+        expect(Rails.logger).to receive(:error).with('Github callback error: OAuth failed')
+
+        get :show, params: {
+          code: 'test_code',
+          installation_id: '12345',
+          setup_action: 'install',
+          state: signed_account_id
+        }
+
+        expect(response).to be_redirect
+      end
+    end
+  end
+
+  describe 'private methods' do
+    let(:controller_instance) { described_class.new }
+
+    before do
+      allow(controller_instance).to receive(:account).and_return(account)
+      allow(controller_instance).to receive(:parsed_body).and_return({
+                                                                       'access_token' => 'test_token',
+                                                                       'token_type' => 'bearer',
+                                                                       'scope' => 'repo,read:org'
+                                                                     })
+      # Mock session for private method tests
+      allow(controller_instance).to receive(:session).and_return({})
+    end
+
+    describe '#build_hook_settings' do
+      it 'builds settings with installation_id parameter' do
+        settings = controller_instance.send(:build_hook_settings, '12345')
+
+        expect(settings[:token_type]).to eq('bearer')
+        expect(settings[:scope]).to eq('repo,read:org')
+        expect(settings[:installation_id]).to eq('12345')
+      end
+
+      it 'builds settings without installation_id when nil' do
+        settings = controller_instance.send(:build_hook_settings, nil)
+
+        expect(settings[:token_type]).to eq('bearer')
+        expect(settings[:scope]).to eq('repo,read:org')
+        expect(settings.key?(:installation_id)).to be false
+      end
+    end
+
+    describe '#create_integration_hook' do
+      it 'creates a new hook with correct attributes' do
+        settings = { token_type: 'bearer', scope: 'repo,read:org', installation_id: '12345' }
+        hook = controller_instance.send(:create_integration_hook, settings)
+
+        expect(hook.access_token).to eq('test_token')
+        expect(hook.status).to eq('enabled')
+        expect(hook.app_id).to eq('github')
+        # Convert expected settings to string keys to match actual storage format
+        expect(hook.settings).to eq(settings.stringify_keys)
+        expect(hook.account).to eq(account)
+      end
+    end
+  end
+end

spec/models/integrations/app_spec.rb

@@ -38,6 +38,24 @@ RSpec.describe Integrations::App do
         end
       end
     end
+
+    context 'when the app is github' do
+      let(:app_name) { 'github' }
+
+      it 'returns the GitHub App installation URL with state parameter' do
+        with_modified_env GITHUB_CLIENT_ID: 'dummy_client_id', GITHUB_APP_NAME: 'test-app' do
+          expect(app.action).to include('https://github.com/apps/test-app/installations/new')
+          expect(app.action).to include('state=')
+        end
+      end
+
+      it 'uses default app name when GITHUB_APP_NAME is not set' do
+        # Mock GlobalConfigService to ensure consistent behavior
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_ID', nil).and_return('dummy_client_id')
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_NAME', 'chatwoot-qa').and_return('chatwoot-qa')
+        expect(app.action).to include('https://github.com/apps/chatwoot-qa/installations/new')
+      end
+    end
   end
 
   describe '#active?' do
@@ -87,6 +105,34 @@ RSpec.describe Integrations::App do
       end
     end
 
+    context 'when the app is github' do
+      let(:app_name) { 'github' }
+
+      it 'returns false if github credentials are not present' do
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_ID', nil).and_return(nil)
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_SECRET', nil).and_return(nil)
+        expect(app.active?(account)).to be false
+      end
+
+      it 'returns false if only client_id is present' do
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_ID', nil).and_return('client_id')
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_SECRET', nil).and_return(nil)
+        expect(app.active?(account)).to be false
+      end
+
+      it 'returns false if only client_secret is present' do
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_ID', nil).and_return(nil)
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_SECRET', nil).and_return('client_secret')
+        expect(app.active?(account)).to be false
+      end
+
+      it 'returns true if both client_id and client_secret are present' do
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_ID', nil).and_return('client_id')
+        allow(GlobalConfigService).to receive(:load).with('GITHUB_CLIENT_SECRET', nil).and_return('client_secret')
+        expect(app.active?(account)).to be true
+      end
+    end
+
     context 'when other apps are queried' do
       let(:app_name) { 'webhook' }