diff --git a/src/support/slack/actions/onboard-llmo-modal.js b/src/support/slack/actions/onboard-llmo-modal.js index b49c18d80..83a17b8d1 100644 --- a/src/support/slack/actions/onboard-llmo-modal.js +++ b/src/support/slack/actions/onboard-llmo-modal.js @@ -670,8 +670,6 @@ export function onboardLLMOModal(lambdaContext) { originalThreadTs, }); - // eslint-disable-next-line max-statements-per-line - await new Promise((resolve) => { setTimeout(resolve, 500); }); await ack(); // Create a slack context for the onboarding process @@ -692,9 +690,12 @@ export function onboardLLMOModal(lambdaContext) { threadTs: responseThreadTs, }; - await onboardSite({ + // Return immediately after ack() to make sure the modal closes + onboardSite({ brandName, baseURL: brandURL, imsOrgId, deliveryType, - }, lambdaContext, slackContext); + }, lambdaContext, slackContext).catch((error) => { + log.error(`Error in background onboarding for site ${brandURL}:`, error); + }); log.debug(`Onboard LLMO modal processed for user ${user.id}, site ${brandURL}`); } catch (e) { @@ -759,14 +760,27 @@ export function addEntitlementsAction(lambdaContext) { }; /* c8 ignore end */ - await createEntitlementAndEnrollment(site, lambdaContext, slackContext); - await client.chat.postMessage({ - channel: originalChannel, - text: `:white_check_mark: Successfully ensured LLMO entitlements and enrollments for *${brandURL}* (brand: *${existingBrand}*).`, - thread_ts: originalThreadTs, - }); - - log.debug(`Added entitlements for site ${siteId} (${brandURL}) for user ${user.id}`); + // Fire-and-forget: Start the entitlement process without awaiting + // This allows the handler to return immediately after ack() while the + // entitlement process continues in the background + (async () => { + try { + await createEntitlementAndEnrollment(site, lambdaContext, slackContext); + await client.chat.postMessage({ + channel: originalChannel, + text: `:white_check_mark: Successfully ensured LLMO entitlements and enrollments for *${brandURL}* (brand: *${existingBrand}*).`, + thread_ts: originalThreadTs, + }); + log.debug(`Added entitlements for site ${siteId} (${brandURL}) for user ${user.id}`); + } catch (error) { + log.error(`Error in background entitlement process for site ${brandURL}:`, error); + await client.chat.postMessage({ + channel: originalChannel, + text: `:x: Failed to add LLMO entitlements: ${error.message}`, + thread_ts: originalThreadTs, + }); + } + })(); } catch (error) { log.error('Error adding entitlements:', error); } @@ -925,16 +939,30 @@ export function updateIMSOrgModal(lambdaContext) { }; /* c8 ignore end */ - await checkOrg(newImsOrgId, site, lambdaContext, slackContext); - await createEntitlementAndEnrollment(site, lambdaContext, slackContext); + // Fire-and-forget: Start the org update process without awaiting + // This allows the handler to return immediately after ack() while the + // update process continues in the background + (async () => { + try { + await checkOrg(newImsOrgId, site, lambdaContext, slackContext); + await createEntitlementAndEnrollment(site, lambdaContext, slackContext); - await client.chat.postMessage({ - channel: responseChannel, - text: `:white_check_mark: Successfully updated organization and applied LLMO entitlements for *${brandURL}* (brand: *${existingBrand}*)`, - thread_ts: responseThreadTs, - }); + await client.chat.postMessage({ + channel: responseChannel, + text: `:white_check_mark: Successfully updated organization and applied LLMO entitlements for *${brandURL}* (brand: *${existingBrand}*)`, + thread_ts: responseThreadTs, + }); - log.debug(`Updated org and applied entitlements for site ${siteId} (${brandURL}) for user ${user.id}`); + log.debug(`Updated org and applied entitlements for site ${siteId} (${brandURL}) for user ${user.id}`); + } catch (error) { + log.error(`Error in background org update for site ${brandURL}:`, error); + await client.chat.postMessage({ + channel: responseChannel, + text: `:x: Failed to update organization: ${error.message}`, + thread_ts: responseThreadTs, + }); + } + })(); } catch (error) { log.error('Error updating organization:', error); } diff --git a/test/support/slack/actions/onboard-llmo-modal.test.js b/test/support/slack/actions/onboard-llmo-modal.test.js index 7f6ba748a..c78e91a27 100644 --- a/test/support/slack/actions/onboard-llmo-modal.test.js +++ b/test/support/slack/actions/onboard-llmo-modal.test.js @@ -1188,7 +1188,62 @@ example-com: expect(mockAck).to.have.been.calledOnce; }); - it('should print error message if onboarding throws an error', async () => { + it('should return immediately after ack without waiting for onboarding to complete', async () => { + const mockBody = { + view: { + state: { + values: { + brand_name_input: { + brand_name: { value: 'Test Brand' }, + }, + ims_org_input: { + ims_org_id: { value: 'ABC123@AdobeOrg' }, + }, + delivery_type_input: { + delivery_type: { + selected_option: { value: 'aem_edge' }, + }, + }, + }, + }, + private_metadata: JSON.stringify({ + originalChannel: 'C1234567890', + originalThreadTs: '1234567890.123456', + brandURL: 'https://example.com', + }), + }, + user: { id: 'U1234567890' }, + }; + + const mockAck = sandbox.stub(); + const mockClient = { + chat: { + postMessage: sandbox.stub().resolves(), + }, + }; + + // Create a slow onboarding process to verify fire-and-forget behavior + const lambdaCtx = createDefaultMockLambdaCtx(sandbox); + const slowOnboardingPromise = new Promise((resolve) => { + setTimeout(resolve, 1000); // Simulate slow operation + }); + lambdaCtx.dataAccess.Site.findByBaseURL = sandbox.stub().returns(slowOnboardingPromise); + + const { onboardLLMOModal } = mockedModule; + const handler = onboardLLMOModal(lambdaCtx); + + const startTime = Date.now(); + await handler({ ack: mockAck, body: mockBody, client: mockClient }); + const endTime = Date.now(); + + // Handler should return quickly (well under 1 second), not wait for the slow operation + expect(endTime - startTime).to.be.lessThan(500); + expect(mockAck).to.have.been.calledOnce; + expect(mockAck).to.have.been.calledWith(); // Called without errors + expect(lambdaCtx.log.debug).to.have.been.calledWith('Onboard LLMO modal processed for user U1234567890, site https://example.com'); + }); + + it('should log error if onboarding throws an error in background', async () => { const mockBody = { view: { state: { @@ -1224,13 +1279,20 @@ example-com: await handler({ ack: mockAck, body: mockBody, client: mockClient }); - expect(lambdaCtx.log.error).to.have.been.calledWith('Error handling onboard site modal:', sinon.match.instanceOf(Error)); - expect(mockAck).to.have.been.calledWith({ - response_action: 'errors', - errors: { - brand_name_input: 'There was an error processing the onboarding request.', - }, + // The handler should acknowledge successfully even if onboarding will fail + expect(mockAck).to.have.been.calledOnce; + expect(mockAck).to.have.been.calledWith(); + + // Wait a bit for the background promise to settle + await new Promise((resolve) => { + setTimeout(resolve, 100); }); + + // The error should be logged by the background error handler + expect(lambdaCtx.log.error).to.have.been.calledWith( + sinon.match(/Error in background onboarding for site/), + sinon.match.instanceOf(Error), + ); }); it('should return validation error when IMS org ID is not provided', async () => { @@ -1572,9 +1634,10 @@ example-com: }; const mockAck = sandbox.stub(); + const postMessageStub = sandbox.stub().resolves(); const mockClient = { chat: { - postMessage: sandbox.stub(), + postMessage: postMessageStub, update: sandbox.stub().resolves(), }, views: { @@ -1593,6 +1656,7 @@ example-com: await handler({ ack: mockAck, body: mockBody, client: mockClient }); + // Verify immediate synchronous behavior expect(mockAck).to.have.been.called; // Check that the original message was updated to prevent re-triggering @@ -1603,8 +1667,8 @@ example-com: blocks: [], }); - // Check that the function completed successfully by verifying the debug log - expect(lambdaCtx.log.debug).to.have.been.calledWith('Added entitlements for site site123 (https://example.com) for user user123'); + // Verify site was looked up + expect(mockSiteModel.findById).to.have.been.calledWith('site123'); }); it('should handle errors during entitlement addition', async () => { @@ -2028,13 +2092,11 @@ example-com: await handler({ ack: mockAck, body: mockBody, client: mockClient }); + // Verify immediate synchronous behavior expect(mockAck).to.have.been.called; - expect(mockClient.chat.postMessage).to.have.been.calledWith({ - channel: 'channel123', - text: ':white_check_mark: Successfully updated organization and applied LLMO entitlements for *https://example.com* (brand: *Test Brand*)', - thread_ts: 'thread123', - }); - expect(lambdaCtx.log.debug).to.have.been.calledWith('Updated org and applied entitlements for site site123 (https://example.com) for user user123'); + + // Verify site was looked up + expect(mockSiteModel.findById).to.have.been.calledWith('site123'); }); it('should return validation error when IMS org ID is not provided', async () => {