Skip to content

Commit dca728f

Browse files
committed
feat(slack/onboard-llmo): acknowledge and process asynchronously
1 parent 8214b95 commit dca728f

File tree

2 files changed

+35
-16
lines changed

2 files changed

+35
-16
lines changed

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

Lines changed: 30 additions & 12 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 modal 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,10 +678,6 @@ 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); });
675-
await ack();
676-
677681
// Create a slack context for the onboarding process
678682
// Use original channel/thread if available, otherwise fall back to DM
679683
const responseChannel = originalChannel || body.user.id;
@@ -692,19 +696,33 @@ export function onboardLLMOModal(lambdaContext) {
692696
threadTs: responseThreadTs,
693697
};
694698

699+
// Perform the onboarding operation BEFORE acknowledging the modal
700+
// This way, if it fails, we can show the error in the modal
695701
await onboardSite({
696702
brandName, baseURL: brandURL, imsOrgId, deliveryType,
697703
}, lambdaContext, slackContext);
698704

699-
log.debug(`Onboard LLMO modal processed for user ${user.id}, site ${brandURL}`);
705+
// Only acknowledge (and close) the modal if onboarding was successful
706+
await ack();
707+
hasAcknowledged = true;
708+
709+
log.debug(`Onboard LLMO modal processed successfully for user ${user.id}, site ${brandURL}`);
700710
} catch (e) {
701711
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-
});
712+
713+
// Only try to acknowledge with errors if we haven't already acknowledged
714+
if (!hasAcknowledged) {
715+
await ack({
716+
response_action: 'errors',
717+
errors: {
718+
brand_name_input: 'There was an error processing the onboarding request. Please try again.',
719+
},
720+
});
721+
} else {
722+
// If we already acknowledged, we can't show modal errors anymore
723+
// The error should have been handled by onboardSite's error handling
724+
log.error('Error occurred after modal was acknowledged - error handling was done via Slack messages');
725+
}
708726
}
709727
};
710728
}

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

Lines changed: 5 additions & 4 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,7 +1185,7 @@ 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;
11891190
});
11901191

@@ -1228,7 +1229,7 @@ example-com:
12281229
expect(mockAck).to.have.been.calledWith({
12291230
response_action: 'errors',
12301231
errors: {
1231-
brand_name_input: 'There was an error processing the onboarding request.',
1232+
brand_name_input: 'There was an error processing the onboarding request. Please try again.',
12321233
},
12331234
});
12341235
});

0 commit comments

Comments
 (0)