Skip to content

Commit e41029c

Browse files
committed
Initially look for 5 reviewers instead of 2
1 parent d4668e2 commit e41029c

File tree

7 files changed

+62
-6
lines changed

7 files changed

+62
-6
lines changed

.aws/bin/HackerRankQueue.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ new HackerRankQueueStack(app, 'HackerRankQueueStack', {
4646
WORKDAY_END_HOUR: modeConfig.WORKDAY_END_HOUR,
4747
HACK_PARSER_BUCKET_NAME: modeConfig.HACK_PARSER_BUCKET_NAME,
4848
TZ: 'America/Chicago',
49+
NUMBER_OF_INITIAL_REVIEWERS: modeConfig.NUMBER_OF_INITIAL_REVIEWERS,
4950
},
5051
});
5152

.aws/cdk.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
"ENCRYPTED_GOOGLE_SERVICE_ACCOUNT_EMAIL": "AQICAHgmK7RLzJGLsX027nGL/3YoG67Jh4Mxgt5hOhZTg/kH0AEaRVPWlX2aVKL8w/25FRmuAAAAlDCBkQYJKoZIhvcNAQcGoIGDMIGAAgEAMHsGCSqGSIb3DQEHATAeBglghkgBZQMEAS4wEQQMKzMqXyLoCKEizJLXAgEQgE6KG7oJcvvJDkhnvxOR5Z2blZ0UH/ib7FTXBaDDQUASqASjyQhOyIVB+V7boljYMTgVhcUWZprkgMvS2hFbiS7gaRlC59KLgkPFP5+VmaE=",
1515
"WORKDAY_START_HOUR": "8",
1616
"WORKDAY_END_HOUR": "17",
17-
"HACK_PARSER_BUCKET_NAME": "hack-parser-dev"
17+
"HACK_PARSER_BUCKET_NAME": "hack-parser-dev",
18+
"NUMBER_OF_INITIAL_REVIEWERS": "5"
1819
},
1920
"prod": {
2021
"HOSTED_ZONE": "sourceallies.com",
@@ -29,7 +30,8 @@
2930
"ENCRYPTED_GOOGLE_SERVICE_ACCOUNT_EMAIL": "AQICAHj0rd9+A15pncr1JrWCjeFtR4hrNnZnqKAX0mW97lZXPAGr0HRhl9Anq+UoyGhRYnZbAAAAlDCBkQYJKoZIhvcNAQcGoIGDMIGAAgEAMHsGCSqGSIb3DQEHATAeBglghkgBZQMEAS4wEQQMHkGZcyEuoE5kdC9CAgEQgE5bFFe7hRWHrVDO9J90X2izrLD8fbLGAzU0gHtoSQwHvfg6YxEgpu5R4bnEH61AgkjkhsSKndMtyjZWnvc2yT+cRPFX8THuOiwt4zKtm+Y=",
3031
"WORKDAY_START_HOUR": "8",
3132
"WORKDAY_END_HOUR": "17",
32-
"HACK_PARSER_BUCKET_NAME": "hack-parser-prod"
33+
"HACK_PARSER_BUCKET_NAME": "hack-parser-prod",
34+
"NUMBER_OF_INITIAL_REVIEWERS": "5"
3335
}
3436
}
3537
}

.aws/lib/HackerRankQueueStack.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ interface HackerRankQueueStackProps extends StackProps {
2828
WORKDAY_END_HOUR: string;
2929
HACK_PARSER_BUCKET_NAME: string;
3030
TZ: string;
31+
NUMBER_OF_INITIAL_REVIEWERS: string;
3132
};
3233
}
3334

src/bot/__tests__/requestReview.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@ describe('requestReview', () => {
309309

310310
async function callCallback(param = buildParam()) {
311311
process.env.INTERVIEWING_CHANNEL_ID = 'some-channel-id';
312+
process.env.NUMBER_OF_INITIAL_REVIEWERS = '5';
312313

313314
chatService.sendRequestReviewMessage = jest.fn().mockResolvedValue('100');
314315
QueueService.getInitialUsersForReview = jest.fn().mockResolvedValueOnce([
@@ -374,7 +375,7 @@ _Candidate Identifier: some-identifier_
374375
it('should get next users to review', async () => {
375376
await callCallback();
376377

377-
expect(QueueService.getInitialUsersForReview).toBeCalledWith(['Go', 'Javascript'], '1');
378+
expect(QueueService.getInitialUsersForReview).toBeCalledWith(['Go', 'Javascript'], 5);
378379
});
379380

380381
it('should create a new active review row', async () => {

src/bot/requestReview.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,10 @@ export const requestReview = {
159159

160160
const user = body.user;
161161
const channel = process.env.INTERVIEWING_CHANNEL_ID;
162+
const numberOfInitialReviewers = Number(process.env.NUMBER_OF_INITIAL_REVIEWERS);
162163
const languages = blockUtils.getLanguageFromBody(body);
163164
const deadline = blockUtils.getBlockValue(body, ActionId.REVIEW_DEADLINE);
164-
const numberOfReviewers = blockUtils.getBlockValue(body, ActionId.NUMBER_OF_REVIEWERS);
165+
const numberOfRequestedReviewers = blockUtils.getBlockValue(body, ActionId.NUMBER_OF_REVIEWERS);
165166
const candidateIdentifier = blockUtils.getBlockValue(body, ActionId.CANDIDATE_IDENTIFIER);
166167

167168
let pdfIdentifier = '';
@@ -184,7 +185,7 @@ export const requestReview = {
184185
}
185186
}
186187

187-
const numberOfReviewersValue = numberOfReviewers.value;
188+
const numberOfReviewersValue = numberOfRequestedReviewers.value;
188189
const deadlineValue = deadline.selected_option.value;
189190
const deadlineDisplay = deadline.selected_option.text.text;
190191
const candidateIdentifierValue = candidateIdentifier.value;
@@ -221,7 +222,7 @@ export const requestReview = {
221222

222223
const reviewers = await QueueService.getInitialUsersForReview(
223224
languages,
224-
numberOfReviewersValue,
225+
numberOfInitialReviewers,
225226
);
226227

227228
if (reviewers.length < numberOfReviewersValue) {

src/services/ReviewCloser.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,16 @@ import { mention } from '@/utils/text';
44
import { App } from '@slack/bolt';
55
import { chatService } from '@/services/ChatService';
66
import { ActiveReview } from '@models/ActiveReview';
7+
import { expireRequest } from '@/services/RequestService';
78

89
export const reviewCloser = {
910
async closeReviewIfComplete(app: App, threadId: string): Promise<void> {
1011
const review = await activeReviewRepo.getReviewByThreadIdOrFail(threadId);
1112

1213
if (isCompleted(review)) {
14+
for (const user of review.pendingReviewers) {
15+
await expireRequest(app, review, user.userId);
16+
}
1317
await closeReview(
1418
app,
1519
review,

src/services/__tests__/reviewCloser.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ import { App } from '@slack/bolt';
66
import { buildMockApp } from '@utils/slackMocks';
77
import { reviewCloser } from '@/services/ReviewCloser';
88

9+
jest.mock('@/services/RequestService', () => ({
10+
...jest.requireActual('@/services/RequestService'),
11+
expireRequest: jest.fn(),
12+
}));
13+
14+
import { expireRequest } from '@/services/RequestService';
15+
916
describe('reviewCloser', () => {
1017
let app: App;
1118
beforeEach(() => {
@@ -98,6 +105,45 @@ describe('reviewCloser', () => {
98105
expect(chatService.replyToReviewThread).not.toHaveBeenCalled();
99106
expect(activeReviewRepo.remove).not.toHaveBeenCalled();
100107
});
108+
109+
it('should call expireRequest for all pending reviewers before closing the review', async () => {
110+
const threadId = '111';
111+
const requestorId = '123';
112+
const pendingReviewers = [
113+
{ userId: '123', expiresAt: 1, messageTimestamp: '456' },
114+
{ userId: '456', expiresAt: 2, messageTimestamp: '789' },
115+
];
116+
const review: ActiveReview = {
117+
threadId: threadId,
118+
requestorId: requestorId,
119+
languages: ['Java'],
120+
requestedAt: new Date(),
121+
dueBy: Deadline.MONDAY,
122+
candidateIdentifier: 'some-id',
123+
reviewersNeededCount: 2,
124+
acceptedReviewers: [acceptedUser('A'), acceptedUser('B')],
125+
declinedReviewers: [],
126+
pendingReviewers: pendingReviewers,
127+
pdfIdentifier: '',
128+
};
129+
130+
activeReviewRepo.getReviewByThreadIdOrFail = jest.fn().mockResolvedValue(review);
131+
132+
await reviewCloser.closeReviewIfComplete(app, threadId);
133+
134+
expect(expireRequest).toHaveBeenCalledTimes(pendingReviewers.length);
135+
pendingReviewers.forEach(pendingReviewer => {
136+
expect(expireRequest).toHaveBeenCalledWith(app, review, pendingReviewer.userId);
137+
});
138+
139+
// Verify the review is closed after expiring requests
140+
expect(chatService.replyToReviewThread).toHaveBeenCalledWith(
141+
app.client,
142+
threadId,
143+
`<@${requestorId}> all 2 reviewers have been found!`,
144+
);
145+
expect(activeReviewRepo.remove).toHaveBeenCalledWith(threadId);
146+
});
101147
});
102148
});
103149

0 commit comments

Comments
 (0)