Skip to content

Commit 6412382

Browse files
committed
Differentiate notification reason review_requested from team_review_requested
Closes #204
1 parent 871872e commit 6412382

9 files changed

Lines changed: 119 additions & 18 deletions

File tree

packages/components/src/components/cards/BaseCard.shared.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import {
3535
getUserURLFromObject,
3636
GitHubIcon,
3737
GitHubIssueOrPullRequest,
38-
GitHubNotificationReason,
3938
GitHubPullRequest,
4039
GitHubPushEvent,
4140
isDraft,
@@ -142,7 +141,7 @@ export interface BaseCardProps extends AdditionalCardProps {
142141
link: string
143142
nodeIdOrId: string
144143
reason?: {
145-
reason: GitHubNotificationReason
144+
reason: EnhancedGitHubNotification['reason']
146145
color: keyof ThemeColors
147146
label: string
148147
tooltip?: string

packages/components/src/redux/actions/columns.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import {
44
ColumnOptions,
55
ColumnsAndSubscriptions,
66
ColumnSubscriptionCreation,
7+
EnhancedGitHubNotification,
78
GitHubEventAction,
89
GitHubEventSubjectType,
910
GitHubIssueOrPullRequestSubjectType,
10-
GitHubNotificationReason,
1111
GitHubNotificationSubjectType,
1212
GitHubStateType,
1313
NotificationColumnFilters,
@@ -105,7 +105,7 @@ export function setColumnLabelFilter(payload: {
105105
}
106106

107107
export function setColumnReasonFilter<
108-
T extends GitHubNotificationReason
108+
T extends EnhancedGitHubNotification['reason']
109109
>(payload: {
110110
columnId: string
111111
reason: T

packages/components/src/redux/migrations.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
isItemRead,
1515
isItemSaved,
1616
IssueOrPullRequestColumnSubscription,
17+
NotificationColumn,
1718
removeUselessURLsFromResponseItem,
1819
} from '@devhub/core'
1920
import immer from 'immer'
@@ -536,4 +537,28 @@ export default {
536537
draft.github.installations.lastFetchSuccessAt = (draft.github
537538
.installations as any).lastFetchedSuccessfullyAt
538539
}),
540+
17: (state: RootState) =>
541+
immer(state, draft => {
542+
draft.columns = draft.columns || {}
543+
draft.columns.byId = draft.columns.byId || {}
544+
545+
const columnIds = Object.keys(draft.columns.byId)
546+
columnIds.forEach(columnId => {
547+
const column = draft.columns.byId![columnId] as NotificationColumn
548+
549+
if (!(column && column.type === 'notifications')) return
550+
551+
if (
552+
column &&
553+
column.filters &&
554+
column.filters.notifications &&
555+
column.filters.notifications.reasons &&
556+
typeof column.filters.notifications.reasons.review_requested ===
557+
'boolean'
558+
) {
559+
column.filters.notifications.reasons.team_review_requested =
560+
column.filters.notifications.reasons.review_requested
561+
}
562+
})
563+
}),
539564
}

packages/components/src/redux/sagas/subscriptions.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,7 @@ function* onFetchRequest(
425425
repoName: string | undefined,
426426
) =>
427427
selectors.getPrivateTokenByRepoSelector(state, ownerName, repoName),
428+
githubLogin: loggedUsername,
428429
githubToken,
429430
},
430431
)

packages/components/src/redux/store.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export function configureStore(key = 'root') {
6060
},
6161
storage,
6262
throttle: 500,
63-
version: 16,
63+
version: 17,
6464
}
6565
const persistedReducer = persistReducer(persistConfig, rootReducer)
6666

packages/core/src/helpers/github/notifications.ts

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import {
1010
GitHubIssueOrPullRequest,
1111
GitHubLabel,
1212
GitHubNotification,
13-
GitHubNotificationReason,
1413
GitHubNotificationSubjectType,
1514
GitHubPullRequest,
15+
GitHubUser,
1616
NotificationPayloadEnhancement,
1717
ThemeColors,
1818
UserPlan,
@@ -36,7 +36,9 @@ import {
3636
getRepoFullNameFromObject,
3737
} from './url'
3838

39-
export const notificationReasons: GitHubNotificationReason[] = [
39+
export const notificationReasons: Array<
40+
EnhancedGitHubNotification['reason']
41+
> = [
4042
'assign',
4143
'author',
4244
'comment',
@@ -48,6 +50,7 @@ export const notificationReasons: GitHubNotificationReason[] = [
4850
'state_change',
4951
'subscribed',
5052
'team_mention',
53+
'team_review_requested',
5154
]
5255

5356
export const notificationSubjectTypes: GitHubNotificationSubjectType[] = [
@@ -60,7 +63,7 @@ export const notificationSubjectTypes: GitHubNotificationSubjectType[] = [
6063
]
6164

6265
export function getNotificationSubjectType(
63-
notification: GitHubNotification,
66+
notification: Pick<GitHubNotification, 'subject'>,
6467
): GitHubNotificationSubjectType | null {
6568
if (!(notification && notification.subject && notification.subject.type))
6669
return null
@@ -69,7 +72,7 @@ export function getNotificationSubjectType(
6972
}
7073

7174
export function getNotificationIconAndColor(
72-
notification: GitHubNotification,
75+
notification: Pick<GitHubNotification, 'subject'>,
7376
payload: GitHubIssueOrPullRequest | undefined,
7477
): { icon: GitHubIcon; color?: keyof ThemeColors; tooltip: string } {
7578
const { subject } = notification
@@ -105,7 +108,7 @@ export function getNotificationIconAndColor(
105108
}
106109

107110
export function getNotificationReasonMetadata<
108-
T extends GitHubNotificationReason
111+
T extends EnhancedGitHubNotification['reason']
109112
>(
110113
reason: T,
111114
): {
@@ -201,12 +204,22 @@ export function getNotificationReasonMetadata<
201204
case 'review_requested':
202205
return {
203206
reason,
204-
color: 'yellow',
207+
color: 'orange',
205208
fullDescription: 'Someone requested your review in the pull request',
206209
// smallDescription: 'Review requested',
207210
label: 'Review requested',
208211
}
209212

213+
case 'team_review_requested':
214+
return {
215+
reason,
216+
color: 'yellow',
217+
fullDescription:
218+
"Someone requested your team's review in the pull request",
219+
// smallDescription: 'Team review requested',
220+
label: 'Team review requested',
221+
}
222+
210223
case 'security_alert':
211224
return {
212225
reason,
@@ -274,7 +287,9 @@ export function mergeNotificationPreservingEnhancement(
274287
newItem.last_unsaved_at,
275288
]),
276289
pullRequest: existingItem.pullRequest,
290+
reason: existingItem.reason,
277291
release: existingItem.release,
292+
requestedMyReview: existingItem.requestedMyReview,
278293
updated_at: _.max([existingItem.updated_at, newItem.updated_at])!,
279294
}
280295

@@ -307,15 +322,19 @@ export async function getNotificationsEnhancementMap(
307322
cache = new Map(),
308323
getGitHubPrivateTokenForRepo,
309324
githubToken: _githubToken,
325+
githubLogin: _githubLogin,
310326
}: {
311327
cache: EnhancementCache | undefined | undefined
312328
getGitHubPrivateTokenForRepo: (
313329
owner: string | undefined,
314330
repo: string | undefined,
315331
) => string | undefined
316332
githubToken: string
333+
githubLogin: string
317334
},
318335
): Promise<Record<string, NotificationPayloadEnhancement>> {
336+
const githubLogin = `${_githubLogin || ''}`.toLowerCase().trim()
337+
319338
const promises = notifications.map(async notification => {
320339
if (!(notification.repository && notification.repository.full_name)) return
321340

@@ -376,6 +395,30 @@ export async function getNotificationsEnhancementMap(
376395
if (!enhance.enhanced) enhance.enhanced = false
377396
return
378397
}
398+
399+
// if (
400+
// subjectField === 'pullRequest' &&
401+
// enhance.pullRequest &&
402+
// notification.reason === 'review_requested' &&
403+
// !enhance.requestedMyReview
404+
// ) {
405+
// try {
406+
// const { data } = await axios.get(
407+
// `${notification.subject.url}/reviews?access_token=${githubToken}`,
408+
// )
409+
410+
// if (data && data.length) {
411+
// enhance.requestedMyReview = !!data.find(
412+
// (item: { user?: GitHubUser }) =>
413+
// item &&
414+
// item.user &&
415+
// githubLogin === `${item.user.login || ''}`.toLowerCase().trim(),
416+
// )
417+
// }
418+
// } catch (error) {
419+
// //
420+
// }
421+
// }
379422
} else if (hasSubjectCache) {
380423
if (subjectCache && subjectCache.data) {
381424
enhance[subjectField] = subjectCache.data
@@ -413,6 +456,25 @@ export async function getNotificationsEnhancementMap(
413456
} else if (!enhance.enhanced) enhance.enhanced = false
414457
}
415458

459+
if (
460+
githubLogin &&
461+
enhance.pullRequest &&
462+
enhance.pullRequest.requested_reviewers
463+
) {
464+
if (!enhance.requestedMyReview) {
465+
enhance.requestedMyReview = !!enhance.pullRequest.requested_reviewers.find(
466+
u => githubLogin === `${u.login || ''}`.toLowerCase().trim(),
467+
)
468+
}
469+
}
470+
471+
if (
472+
notification.reason === 'review_requested' &&
473+
enhance.requestedMyReview === false
474+
) {
475+
enhance.reason = 'team_review_requested'
476+
}
477+
416478
if (!Object.keys(enhance).length) return
417479

418480
return { id: notification.id, enhance }

packages/core/src/helpers/shared.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,9 @@ export function isEventPrivate(event: EnhancedGitHubEvent) {
189189
)
190190
}
191191

192-
export function isNotificationPrivate(notification: GitHubNotification) {
192+
export function isNotificationPrivate(
193+
notification: Pick<GitHubNotification, 'repository'>,
194+
) {
193195
if (!notification) return false
194196
return !!(notification.repository && notification.repository.private)
195197
}

packages/core/src/types/devhub.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ import { GraphQLUserPlan } from './graphql'
2727

2828
type octokit = InstanceType<typeof Octokit>
2929

30+
export type EnhancedGitHubNotificationReason =
31+
| GitHubNotificationReason
32+
| 'team_review_requested'
33+
3034
export interface ArchivedEnhancement {
3135
// archived_at?: string
3236
}
@@ -51,10 +55,12 @@ export interface BaseEnhancement
5155
export interface NotificationPayloadEnhancement extends BaseEnhancement {
5256
comment?: GitHubComment
5357
commit?: GitHubCommit
58+
enhanced?: boolean
5459
issue?: GitHubIssue
5560
pullRequest?: GitHubPullRequest
61+
reason?: EnhancedGitHubNotificationReason
5662
release?: GitHubRelease
57-
enhanced?: boolean
63+
requestedMyReview?: boolean
5864
}
5965

6066
export interface IssuePayloadEnhancement extends BaseEnhancement {
@@ -74,8 +80,10 @@ export type IssueOrPullRequestPayloadEnhancement =
7480
| PullRequestPayloadEnhancement
7581

7682
export interface EnhancedGitHubNotification
77-
extends GitHubNotification,
78-
NotificationPayloadEnhancement {}
83+
extends Omit<GitHubNotification, 'reason'>,
84+
NotificationPayloadEnhancement {
85+
reason: EnhancedGitHubNotificationReason
86+
}
7987

8088
export interface GitHubEnhancedEventBase {
8189
merged: string[]
@@ -262,7 +270,7 @@ export interface IssueOrPullRequestColumnFilters extends BaseColumnFilters {
262270
export interface NotificationColumnFilters extends BaseColumnFilters {
263271
notifications?: {
264272
participating?: boolean
265-
reasons?: Partial<Record<GitHubNotificationReason, boolean>>
273+
reasons?: Partial<Record<EnhancedGitHubNotification['reason'], boolean>>
266274
}
267275
subjectTypes?: Partial<Record<GitHubNotificationSubjectType, boolean>>
268276
}
@@ -480,7 +488,10 @@ export interface ItemsFilterMetadata {
480488
Record<GitHubItemSubjectType, ItemFilterCountMetadata | undefined>
481489
>
482490
subscriptionReason: Partial<
483-
Record<GitHubNotificationReason, ItemFilterCountMetadata | undefined>
491+
Record<
492+
EnhancedGitHubNotification['reason'],
493+
ItemFilterCountMetadata | undefined
494+
>
484495
>
485496
eventAction: Partial<Record<GitHubEventAction, ItemFilterCountMetadata>>
486497
privacy: Record<GitHubPrivacy, ItemFilterCountMetadata>

packages/core/src/types/github.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ export interface GitHubPullRequest {
243243
merged_by: GitHubUser // User
244244
comments: number // 0
245245
review_comments: number // 0
246+
requested_reviewers?: GitHubUser[] | null // []
246247
commits: number // 2
247248
additions: number // 4
248249
deletions: number // 4
@@ -864,7 +865,7 @@ export type GitHubNotificationReason =
864865
| 'invitation' // You accepted an invitation to contribute to the repository.
865866
| 'manual' // You subscribed to the thread (via an Issue or Pull Request).
866867
| 'mention' // You were specifically @mentioned in the content.
867-
| 'review_requested' // Someone requested your review on a pull request
868+
| 'review_requested' // Someone requested your review (or your team's) on a pull request
868869
| 'security_alert' // Potential security vulnerability alert
869870
| 'state_change' // You changed the thread state (for example, closing an Issue or merging a PR).
870871
| 'subscribed' // You're watching the repository.

0 commit comments

Comments
 (0)