Skip to content

Commit e3d7c2b

Browse files
didineledidinele
committed
chore: another review pass
1 parent c406099 commit e3d7c2b

4 files changed

Lines changed: 172 additions & 168 deletions

File tree

‎services/ama-bot/src/components/guestApprove.ts‎

Lines changed: 54 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -18,60 +18,48 @@ export default class GuestApproveComponent implements ComponentHandler<string> {
1818
// finishes via editReply/followUp instead of reply/updateMessage.
1919
await client.api.interactions.deferMessageUpdate(interaction.id, interaction.token);
2020

21-
const [question] = await getContext().db<AmaQuestions[]>`
22-
SELECT * FROM ama_questions WHERE id = ${questionId}
23-
`;
24-
25-
if (!question) {
26-
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
27-
content: 'Question not found. It may have been deleted.',
28-
flags: MessageFlags.Ephemeral,
29-
});
30-
return;
31-
}
32-
33-
const [session] = await getContext().db<AmaSessions[]>`
34-
SELECT * FROM ama_sessions WHERE id = ${question.amaId}
35-
`;
36-
37-
if (!session) {
38-
throw new Error(`No AMA session found for id ${question.amaId}`);
39-
}
40-
41-
if (session.ended) {
42-
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
43-
content: 'This AMA session has ended.',
44-
flags: MessageFlags.Ephemeral,
45-
});
46-
return;
47-
}
48-
49-
// Only claims from PENDING_GUEST_REVIEW so a concurrent answer/skip can't both win.
50-
const [claimed] = await getContext().db<AmaQuestions[]>`
51-
UPDATE ama_questions
52-
SET state = 'APPROVED', updated_at = now()
53-
WHERE id = ${question.id} AND state = 'PENDING_GUEST_REVIEW'
54-
RETURNING *
55-
`;
56-
57-
if (!claimed) {
58-
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
59-
content: 'This question was already handled by someone else.',
60-
flags: MessageFlags.Ephemeral,
61-
});
62-
return;
63-
}
21+
try {
22+
const [question] = await getContext().db<AmaQuestions[]>`
23+
SELECT * FROM ama_questions WHERE id = ${questionId}
24+
`;
6425

65-
const user = await client.api.users.get(question.authorId);
66-
const member = interaction.guild_id
67-
? await client.api.guilds.getMember(interaction.guild_id, question.authorId).catch(() => undefined)
68-
: undefined;
26+
if (!question) {
27+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
28+
content: 'Question not found. It may have been deleted.',
29+
flags: MessageFlags.Ephemeral,
30+
});
31+
return;
32+
}
6933

70-
// Attachments aren't persisted on the row, so we carry them forward off the source message; the
71-
// question text itself comes straight from the DB (the source message's text has a footer baked in).
72-
const attachments = interaction.message.attachments ?? [];
34+
const [session] = await getContext().db<AmaSessions[]>`
35+
SELECT * FROM ama_sessions WHERE id = ${question.amaId}
36+
`;
7337

74-
try {
38+
if (!session) {
39+
throw new Error(`No AMA session found for id ${question.amaId}`);
40+
}
41+
42+
if (session.ended) {
43+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
44+
content: 'This AMA session has ended.',
45+
flags: MessageFlags.Ephemeral,
46+
});
47+
return;
48+
}
49+
50+
const user = await client.api.users.get(question.authorId);
51+
const member = interaction.guild_id
52+
? await client.api.guilds.getMember(interaction.guild_id, question.authorId).catch(() => undefined)
53+
: undefined;
54+
55+
// Attachments aren't persisted on the row, so we carry them forward off the source message; the
56+
// question text itself comes straight from the DB (the source message's text has a footer baked in).
57+
const attachments = interaction.message.attachments ?? [];
58+
59+
// Post first, claim second: if the post throws, the row is never touched and stays
60+
// PENDING_GUEST_REVIEW, so the button remains retryable. If we lose a claim race after posting
61+
// (someone else got there first), clean up the message we just created instead of leaving a
62+
// stray duplicate.
7563
const msg = await postToAnswersChannel({
7664
attachments,
7765
content: question.content,
@@ -81,10 +69,23 @@ export default class GuestApproveComponent implements ComponentHandler<string> {
8169
user,
8270
});
8371

84-
await getContext().db`
85-
UPDATE ama_questions SET answers_message_id = ${msg.id} WHERE id = ${question.id}
72+
const [claimed] = await getContext().db<AmaQuestions[]>`
73+
UPDATE ama_questions
74+
SET state = 'APPROVED', answers_message_id = ${msg.id}, updated_at = now()
75+
WHERE id = ${question.id} AND state = 'PENDING_GUEST_REVIEW'
76+
RETURNING *
8677
`;
8778

79+
if (!claimed) {
80+
// eslint-disable-next-line promise/prefer-await-to-then
81+
void client.api.channels.deleteMessage(session.answersChannelId, msg.id).catch(() => null);
82+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
83+
content: 'This question was already handled by someone else.',
84+
flags: MessageFlags.Ephemeral,
85+
});
86+
return;
87+
}
88+
8889
await client.api.interactions.editReply(interaction.application_id, interaction.token, {
8990
components: [
9091
{
@@ -102,9 +103,6 @@ export default class GuestApproveComponent implements ComponentHandler<string> {
102103
],
103104
});
104105
} catch (error) {
105-
// The row is already claimed (state flipped) at this point; if the answers-channel post itself
106-
// failed, the question is stuck claimed with no downstream message and needs manual follow-up —
107-
// logged loudly here rather than attempting a rollback/saga for what should be a rare failure mode.
108106
getContext().logger.error({ error, questionId }, 'Failed to approve question');
109107
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
110108
content: 'Failed to approve question. Please try again.',

‎services/ama-bot/src/components/guestSkip.ts‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,35 +17,35 @@ export default class GuestSkipComponent implements ComponentHandler<string> {
1717
// finishes via editReply/followUp instead of reply/updateMessage.
1818
await client.api.interactions.deferMessageUpdate(interaction.id, interaction.token);
1919

20-
const [question] = await getContext().db<AmaQuestions[]>`
21-
SELECT * FROM ama_questions WHERE id = ${questionId}
22-
`;
20+
try {
21+
const [question] = await getContext().db<AmaQuestions[]>`
22+
SELECT * FROM ama_questions WHERE id = ${questionId}
23+
`;
2324

24-
if (!question) {
25-
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
26-
content: 'Question not found. It may have been deleted.',
27-
flags: MessageFlags.Ephemeral,
28-
});
29-
return;
30-
}
25+
if (!question) {
26+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
27+
content: 'Question not found. It may have been deleted.',
28+
flags: MessageFlags.Ephemeral,
29+
});
30+
return;
31+
}
3132

32-
const [session] = await getContext().db<AmaSessions[]>`
33-
SELECT * FROM ama_sessions WHERE id = ${question.amaId}
34-
`;
33+
const [session] = await getContext().db<AmaSessions[]>`
34+
SELECT * FROM ama_sessions WHERE id = ${question.amaId}
35+
`;
3536

36-
if (!session) {
37-
throw new Error(`No AMA session found for id ${question.amaId}`);
38-
}
37+
if (!session) {
38+
throw new Error(`No AMA session found for id ${question.amaId}`);
39+
}
3940

40-
if (session.ended) {
41-
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
42-
content: 'This AMA session has ended.',
43-
flags: MessageFlags.Ephemeral,
44-
});
45-
return;
46-
}
41+
if (session.ended) {
42+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
43+
content: 'This AMA session has ended.',
44+
flags: MessageFlags.Ephemeral,
45+
});
46+
return;
47+
}
4748

48-
try {
4949
// Only skips from PENDING_GUEST_REVIEW so a concurrent answer/skip can't both win.
5050
const [skipped] = await getContext().db<AmaQuestions[]>`
5151
UPDATE ama_questions

‎services/ama-bot/src/components/modApprove.ts‎

Lines changed: 69 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -18,67 +18,61 @@ export default class ModApproveComponent implements ComponentHandler<string> {
1818
// finishes via editReply/followUp instead of reply/updateMessage.
1919
await client.api.interactions.deferMessageUpdate(interaction.id, interaction.token);
2020

21-
const [question] = await getContext().db<AmaQuestions[]>`
22-
SELECT * FROM ama_questions WHERE id = ${questionId}
23-
`;
24-
25-
if (!question) {
26-
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
27-
content: 'Question not found. It may have been deleted.',
28-
flags: MessageFlags.Ephemeral,
29-
});
30-
return;
31-
}
32-
33-
const [session] = await getContext().db<AmaSessions[]>`
34-
SELECT * FROM ama_sessions WHERE id = ${question.amaId}
35-
`;
36-
37-
if (!session) {
38-
throw new Error(`No AMA session found for id ${question.amaId}`);
39-
}
40-
41-
if (session.ended) {
42-
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
43-
content: 'This AMA session has ended.',
44-
flags: MessageFlags.Ephemeral,
45-
});
46-
return;
47-
}
48-
49-
// Determine the next queue up front so the claim below can move the row straight to its target
50-
// state; this also doubles as a lock — only one concurrent click can win the row.
51-
const nextQueue = getNextQueue(CurrentlyInQueue.mod, session);
52-
const targetState = nextQueue?.kind === CurrentlyInQueue.guest ? 'PENDING_GUEST_REVIEW' : 'APPROVED';
21+
try {
22+
const [question] = await getContext().db<AmaQuestions[]>`
23+
SELECT * FROM ama_questions WHERE id = ${questionId}
24+
`;
25+
26+
if (!question) {
27+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
28+
content: 'Question not found. It may have been deleted.',
29+
flags: MessageFlags.Ephemeral,
30+
});
31+
return;
32+
}
5333

54-
const [claimed] = await getContext().db<AmaQuestions[]>`
55-
UPDATE ama_questions
56-
SET state = ${targetState}, updated_at = now()
57-
WHERE id = ${question.id} AND state = 'PENDING_MOD_REVIEW'
58-
RETURNING *
59-
`;
34+
const [session] = await getContext().db<AmaSessions[]>`
35+
SELECT * FROM ama_sessions WHERE id = ${question.amaId}
36+
`;
6037

61-
if (!claimed) {
62-
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
63-
content: 'This question was already handled by another moderator.',
64-
flags: MessageFlags.Ephemeral,
65-
});
66-
return;
67-
}
38+
if (!session) {
39+
throw new Error(`No AMA session found for id ${question.amaId}`);
40+
}
6841

69-
// Get user details from the interaction
70-
const user = await client.api.users.get(question.authorId);
71-
const member = interaction.guild_id
72-
? await client.api.guilds.getMember(interaction.guild_id, question.authorId).catch(() => undefined)
73-
: undefined;
42+
if (session.ended) {
43+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
44+
content: 'This AMA session has ended.',
45+
flags: MessageFlags.Ephemeral,
46+
});
47+
return;
48+
}
7449

75-
// Attachments aren't persisted on the row, so we carry them forward off the source message; the
76-
// question text itself comes straight from the DB (the source message's text has a footer baked in).
77-
const attachments = interaction.message.attachments ?? [];
50+
// Get user details from the interaction
51+
const user = await client.api.users.get(question.authorId);
52+
const member = interaction.guild_id
53+
? await client.api.guilds.getMember(interaction.guild_id, question.authorId).catch(() => undefined)
54+
: undefined;
55+
56+
// Attachments aren't persisted on the row, so we carry them forward off the source message; the
57+
// question text itself comes straight from the DB (the source message's text has a footer baked in).
58+
const attachments = interaction.message.attachments ?? [];
59+
60+
const nextQueue = getNextQueue(CurrentlyInQueue.mod, session);
61+
62+
// Post first, claim second: if the post throws, the row is never touched and stays
63+
// PENDING_MOD_REVIEW, so the button remains retryable. If we lose a claim race after posting
64+
// (another moderator got there first), we clean up the message we just created instead of
65+
// leaving a stray duplicate.
66+
const reportLostRace = async (channelId: string, messageId: string) => {
67+
// eslint-disable-next-line promise/prefer-await-to-then
68+
void client.api.channels.deleteMessage(channelId, messageId).catch(() => null);
69+
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
70+
content: 'This question was already handled by another moderator.',
71+
flags: MessageFlags.Ephemeral,
72+
});
73+
};
7874

79-
try {
8075
if (nextQueue?.kind === CurrentlyInQueue.guest) {
81-
// Post to guest queue
8276
const msg = await postToGuestQueue({
8377
attachments,
8478
content: question.content,
@@ -88,11 +82,18 @@ export default class ModApproveComponent implements ComponentHandler<string> {
8882
user,
8983
});
9084

91-
await getContext().db`
92-
UPDATE ama_questions SET guest_queue_message_id = ${msg.id} WHERE id = ${question.id}
85+
const [claimed] = await getContext().db<AmaQuestions[]>`
86+
UPDATE ama_questions
87+
SET state = 'PENDING_GUEST_REVIEW', guest_queue_message_id = ${msg.id}, updated_at = now()
88+
WHERE id = ${question.id} AND state = 'PENDING_MOD_REVIEW'
89+
RETURNING *
9390
`;
91+
92+
if (!claimed) {
93+
await reportLostRace(session.guestQueueId!, msg.id);
94+
return;
95+
}
9496
} else {
95-
// Post directly to answers channel
9697
const msg = await postToAnswersChannel({
9798
attachments,
9899
content: question.content,
@@ -102,9 +103,17 @@ export default class ModApproveComponent implements ComponentHandler<string> {
102103
user,
103104
});
104105

105-
await getContext().db`
106-
UPDATE ama_questions SET answers_message_id = ${msg.id} WHERE id = ${question.id}
106+
const [claimed] = await getContext().db<AmaQuestions[]>`
107+
UPDATE ama_questions
108+
SET state = 'APPROVED', answers_message_id = ${msg.id}, updated_at = now()
109+
WHERE id = ${question.id} AND state = 'PENDING_MOD_REVIEW'
110+
RETURNING *
107111
`;
112+
113+
if (!claimed) {
114+
await reportLostRace(session.answersChannelId, msg.id);
115+
return;
116+
}
108117
}
109118

110119
// Update the message to show it was approved
@@ -125,9 +134,6 @@ export default class ModApproveComponent implements ComponentHandler<string> {
125134
],
126135
});
127136
} catch (error) {
128-
// The row is already claimed (state flipped) at this point; if the queue post itself failed, the
129-
// question is stuck claimed with no downstream message and needs manual follow-up — logged loudly
130-
// here rather than attempting a rollback/saga for what should be a rare failure mode.
131137
getContext().logger.error({ error, questionId }, 'Failed to approve question');
132138
await client.api.interactions.followUp(interaction.application_id, interaction.token, {
133139
content: 'Failed to approve question. Please try again.',

0 commit comments

Comments
 (0)