Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Business hour logic to routing and triage #365

Merged
merged 26 commits into from
Dec 14, 2022
Merged

Add Business hour logic to routing and triage #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
17aea74
holiday config
hubertdeng123 Nov 28, 2022
b788ef1
add capability to add multiple offices per channel
hubertdeng123 Nov 29, 2022
90a50f9
add comment about using office for notify-for-triage
hubertdeng123 Nov 29, 2022
2e6a908
Merge branch 'hubertdeng123/allow-multiple-offices-per-channel' into …
hubertdeng123 Nov 29, 2022
96621bf
add more holiday logic
hubertdeng123 Dec 1, 2022
aa8ee23
merging master
hubertdeng123 Dec 1, 2022
2607bab
clean up moment tz installation
hubertdeng123 Dec 1, 2022
578eef6
lot of tests :)
hubertdeng123 Dec 1, 2022
6105f54
use 2023 chrimstas instead of 2022
hubertdeng123 Dec 1, 2022
1a096d1
don't add date, just subtract loop counter for holidays
hubertdeng123 Dec 1, 2022
0060b69
add test case for rerouted issues
hubertdeng123 Dec 1, 2022
397d415
Merge branch 'main' into hubertdeng123/add-business-hours
hubertdeng123 Dec 7, 2022
aa73337
Merge branch 'main' into hubertdeng123/add-business-hours
hubertdeng123 Dec 8, 2022
a3d2a37
initial businessHour logic seems to work
hubertdeng123 Dec 8, 2022
ccecd7b
tests for business hours
hubertdeng123 Dec 9, 2022
a1c81d6
fix case of last day of month
hubertdeng123 Dec 9, 2022
786a317
fix flaky issue notifier tests
hubertdeng123 Dec 9, 2022
5a79a68
remove it.only
hubertdeng123 Dec 9, 2022
728c39b
add some comments
hubertdeng123 Dec 9, 2022
36745f7
clean up some logic, variable renaming
hubertdeng123 Dec 12, 2022
645a36c
fix bug related to utc time conversion, use moment for most of the logic
hubertdeng123 Dec 13, 2022
99554f6
see if this fixes warnings
hubertdeng123 Dec 13, 2022
3bb8a59
add full iso string to tests
hubertdeng123 Dec 13, 2022
a3eccc4
completely refactor logic
hubertdeng123 Dec 14, 2022
368edb3
fix typescript errors
hubertdeng123 Dec 14, 2022
51358ef
rename some functions
hubertdeng123 Dec 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .pnp.cjs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file added .yarn/cache/moment-npm-2.29.4-902943305d-0ec3f9c2bc.zip
View file
Open in desktop
Binary file not shown.
Binary file added .yarn/cache/moment-timezone-npm-0.5.39-e9aea4996d-9f972d3a29.zip
View file
Open in desktop
Binary file not shown.
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
"knex": "^0.95.12",
"middie": "^5.3.0",
"module-alias": "^2.2.2",
"moment-timezone": "^0.5.39",
"pg": "^8.5.1",
"source-map-support": "^0.5.21",
"tar": "^6.1.11",
Expand Down
33 changes: 11 additions & 22 deletions src/brain/issueNotifier/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'This channel is set to receive notifications for: Team: Test (no office specified)'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fixes flaky tests, id here isn't what we care about

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find!

{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: null,
},
Expand All @@ -202,10 +201,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'Add office location sfo on the current channel (test) for Team: Test'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['sfo'],
},
Expand All @@ -222,10 +220,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'This channel is set to receive notifications for: Team: Test (sfo)'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['sfo'],
},
Expand All @@ -242,10 +239,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'Add office location sea on the current channel (test) for Team: Test'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['sfo', 'sea'],
},
Expand All @@ -262,10 +258,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'This channel is set to receive notifications for: Team: Test (sfo, sea)'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['sfo', 'sea'],
},
Expand All @@ -282,10 +277,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'Add office location vie on the current channel (test) for Team: Test'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['sfo', 'sea', 'vie'],
},
Expand All @@ -302,10 +296,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'Add office location yyz on the current channel (test) for Team: Test'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['sfo', 'sea', 'vie', 'yyz'],
},
Expand All @@ -322,10 +315,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'This channel (test) will no longer get notifications for Team: Test during sea business hours.'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['sfo', 'vie', 'yyz'],
},
Expand All @@ -342,10 +334,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'This channel (test) is not subscribed to Team: Test during sea business hours.'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['sfo', 'vie', 'yyz'],
},
Expand All @@ -362,10 +353,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'This channel (test) will no longer get notifications for Team: Test during yyz business hours.'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['sfo', 'vie'],
},
Expand All @@ -382,10 +372,9 @@ describe('issueNotifier Tests', function () {
expect(say).lastCalledWith(
'This channel (test) will no longer get notifications for Team: Test during sfo business hours.'
);
expect(await getLabelsTable().where({ channel_id })).toEqual([
expect(await getLabelsTable().where({ channel_id })).toMatchObject([
{
channel_id: 'CHNLIDRND1',
id: 1,
label_name: 'Team: Test',
offices: ['vie'],
},
Expand Down
4 changes: 2 additions & 2 deletions src/brain/issueNotifier/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { EmitterWebhookEvent } from '@octokit/webhooks';
import { TEAM_LABEL_PREFIX, UNROUTED_LABEL, UNTRIAGED_LABEL } from '@/config';
import { githubEvents } from '@api/github';
import { bolt } from '@api/slack';
import { cacheOfficesForTeam } from '@utils/businessHours';
import { cacheOffices } from '@utils/businessHours';
import { db } from '@utils/db';
import { wrapHandler } from '@utils/wrapHandler';

Expand Down Expand Up @@ -212,7 +212,7 @@ export const slackHandler = async ({ command, ack, say, respond, client }) => {
break;
}
// Update cache for the offices mapped to each team
await cacheOfficesForTeam(label_name);
await cacheOffices(label_name);
}
await Promise.all(pending);
};
Expand Down
12 changes: 12 additions & 0 deletions src/config/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,15 @@ export const MAX_ROUTE_DAYS = 1;
* Personal Access Token for the Sentry bot used to do things that aren't possible with the App account, e.g. querying org membership
*/
export const GH_USER_TOKEN = process.env.GH_USER_TOKEN || '';

/**
* Business Hours by Office
*/

export const OFFICE_TIME_ZONES = {
vie: 'Europe/Vienna',
ams: 'Europe/Amsterdam',
yyz: 'America/Toronto',
sfo: 'America/Los_Angeles',
sea: 'America/Los_Angeles',
};
Loading