Skip to content

Commit 342cd22

Browse files
committed
feat(slack/onboard-llmo): acknowledge and process asynchronously
1 parent 695c46e commit 342cd22

File tree

2 files changed

+53
-18
lines changed

2 files changed

+53
-18
lines changed

src/support/slack/actions/onboard-llmo-modal.js

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ export async function onboardSite(input, lambdaCtx, slackCtx) {
602602
await sqs.sendMessage(configuration.getQueues().audits, sqsTriggerMesasage);
603603

604604
const message = `:white_check_mark: *LLMO onboarding completed successfully!*
605-
605+
606606
:link: *Site:* ${baseURL}
607607
:identification_card: *Site ID:* ${siteId}
608608
:file_folder: *Data Folder:* ${dataFolder}
@@ -615,10 +615,14 @@ The LLMO Customer Analysis handler has been triggered. It will take a few minute
615615
} catch (error) {
616616
log.error(`Error saving LLMO config for site ${siteId}: ${error.message}`);
617617
await say(`:x: Failed to save LLMO configuration: ${error.message}`);
618+
// Re-throw the error so it can be caught by the outer error handler
619+
throw new Error(`Failed to save LLMO configuration: ${error.message}`);
618620
}
619621
} catch (error) {
620622
log.error('Error in LLMO onboarding:', error);
621623
await postErrorMessage(say, error);
624+
// Re-throw the error so it can be caught by the modal handler
625+
throw error;
622626
}
623627
}
624628

@@ -627,6 +631,8 @@ export function onboardLLMOModal(lambdaContext) {
627631
const { log } = lambdaContext;
628632

629633
return async ({ ack, body, client }) => {
634+
let hasAcknowledged = false;
635+
630636
try {
631637
log.debug('Starting onboarding process...');
632638
const { view, user } = body;
@@ -650,6 +656,7 @@ export function onboardLLMOModal(lambdaContext) {
650656
const imsOrgId = values.ims_org_input.ims_org_id.value;
651657
const deliveryType = values.delivery_type_input?.delivery_type?.selected_option?.value;
652658

659+
// Validate required fields before proceeding
653660
if (!brandName || !imsOrgId) {
654661
await ack({
655662
response_action: 'errors',
@@ -658,6 +665,7 @@ export function onboardLLMOModal(lambdaContext) {
658665
ims_org_input: !imsOrgId ? 'IMS Org ID is required' : undefined,
659666
},
660667
});
668+
hasAcknowledged = true;
661669
return;
662670
}
663671

@@ -670,9 +678,9 @@ export function onboardLLMOModal(lambdaContext) {
670678
originalThreadTs,
671679
});
672680

673-
// eslint-disable-next-line max-statements-per-line
674-
await new Promise((resolve) => { setTimeout(resolve, 500); });
681+
// Acknowledge the modal immediately to close it and provide good UX
675682
await ack();
683+
hasAcknowledged = true;
676684

677685
// Create a slack context for the onboarding process
678686
// Use original channel/thread if available, otherwise fall back to DM
@@ -692,19 +700,32 @@ export function onboardLLMOModal(lambdaContext) {
692700
threadTs: responseThreadTs,
693701
};
694702

703+
// Post immediate status message to let user know processing has started
704+
await slackContext.say(`:hourglass_flowing_sand: Starting LLMO onboarding for *${brandName}* (${brandURL})...`);
705+
706+
// Perform the onboarding operation asynchronously
707+
// Any errors will be posted to the Slack channel via the slackContext
695708
await onboardSite({
696709
brandName, baseURL: brandURL, imsOrgId, deliveryType,
697710
}, lambdaContext, slackContext);
698711

699-
log.debug(`Onboard LLMO modal processed for user ${user.id}, site ${brandURL}`);
712+
log.debug(`Onboard LLMO modal processed successfully for user ${user.id}, site ${brandURL}`);
700713
} catch (e) {
701714
log.error('Error handling onboard site modal:', e);
702-
await ack({
703-
response_action: 'errors',
704-
errors: {
705-
brand_name_input: 'There was an error processing the onboarding request.',
706-
},
707-
});
715+
716+
// Only try to acknowledge with errors if we haven't already acknowledged
717+
if (!hasAcknowledged) {
718+
await ack({
719+
response_action: 'errors',
720+
errors: {
721+
brand_name_input: 'There was an error processing the onboarding request. Please try again.',
722+
},
723+
});
724+
} else {
725+
// If we already acknowledged, we can't show modal errors anymore
726+
// The error should have been handled by onboardSite's error handling via Slack messages
727+
log.error('Error occurred after modal was acknowledged - error was posted to Slack channel');
728+
}
708729
}
709730
};
710731
}

test/support/slack/actions/onboard-llmo-modal.test.js

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -709,8 +709,9 @@ describe('onboard-llmo-modal', () => {
709709
// Make SQS sendMessage throw an error
710710
lambdaCtx.sqs.sendMessage.rejects(new Error('SQS service unavailable'));
711711

712-
// Execute the function
713-
await onboardSite(input, lambdaCtx, slackCtx);
712+
// Execute the function and expect it to throw
713+
await expect(onboardSite(input, lambdaCtx, slackCtx))
714+
.to.be.rejectedWith('Failed to save LLMO configuration: SQS service unavailable');
714715

715716
// Verify that SQS sendMessage was called but failed
716717
expect(lambdaCtx.sqs.sendMessage).to.have.been.called;
@@ -1184,8 +1185,15 @@ example-com:
11841185
originalChannel: 'C1234567890',
11851186
originalThreadTs: '1234567890.123456',
11861187
});
1187-
expect(lambdaCtx.log.debug).to.have.been.calledWith('Onboard LLMO modal processed for user U1234567890, site https://example.com');
1188+
expect(lambdaCtx.log.debug).to.have.been.calledWith('Onboard LLMO modal processed successfully for user U1234567890, site https://example.com');
11881189
expect(mockAck).to.have.been.calledOnce;
1190+
1191+
// Verify that the status message was posted
1192+
expect(mockClient.chat.postMessage).to.have.been.calledWith({
1193+
channel: 'C1234567890',
1194+
text: ':hourglass_flowing_sand: Starting LLMO onboarding for *Test Brand* (https://example.com)...',
1195+
thread_ts: '1234567890.123456',
1196+
});
11891197
});
11901198

11911199
it('should print error message if onboarding throws an error', async () => {
@@ -1225,11 +1233,17 @@ example-com:
12251233
await handler({ ack: mockAck, body: mockBody, client: mockClient });
12261234

12271235
expect(lambdaCtx.log.error).to.have.been.calledWith('Error handling onboard site modal:', sinon.match.instanceOf(Error));
1228-
expect(mockAck).to.have.been.calledWith({
1229-
response_action: 'errors',
1230-
errors: {
1231-
brand_name_input: 'There was an error processing the onboarding request.',
1232-
},
1236+
expect(lambdaCtx.log.error).to.have.been.calledWith('Error occurred after modal was acknowledged - error was posted to Slack channel');
1237+
1238+
// Modal should be acknowledged immediately (closed)
1239+
expect(mockAck).to.have.been.calledOnce;
1240+
expect(mockAck).to.not.have.been.calledWith(sinon.match.object);
1241+
1242+
// Status message should be posted
1243+
expect(mockClient.chat.postMessage).to.have.been.calledWith({
1244+
channel: 'C1234567890',
1245+
text: ':hourglass_flowing_sand: Starting LLMO onboarding for *Test Brand* (this is not a valid URL)...',
1246+
thread_ts: '1234567890.123456',
12331247
});
12341248
});
12351249

0 commit comments

Comments
 (0)