Skip to content

Commit c85feb4

Browse files
committed
refactor: clean up query merging implementation
- Remove commented-out debug code from notifications.ts - Fix FIXME by using isAnsweredDiscussionFeatureSupported with account - Remove unused extras field from GraphQLMergedQueryConfig type - Add mergeQueryConfig tests for issue, pullRequest, discussion, and default handlers - Document why integration test is skipped (complex mocking requirements)
1 parent 02866a2 commit c85feb4

File tree

10 files changed

+61
-38
lines changed

10 files changed

+61
-38
lines changed

src/renderer/hooks/useNotifications.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ describe('renderer/hooks/useNotifications.ts', () => {
115115
expect(result.current.notifications[1].notifications.length).toBe(2);
116116
});
117117

118+
// Note: This integration test is skipped because the query merging functionality
119+
// requires complex mocking of multiple endpoints. The merging functionality is
120+
// tested via unit tests in notifications.test.ts and individual handler tests.
118121
it.skip('should fetch detailed notifications with success', async () => {
119122
const mockRepository = {
120123
name: 'notifications-test',

src/renderer/utils/notifications/handlers/default.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ import {
1212
import { defaultHandler } from './default';
1313

1414
describe('renderer/utils/notifications/handlers/default.ts', () => {
15+
describe('mergeQueryConfig', () => {
16+
it('should return undefined (no merge query support)', () => {
17+
const config = defaultHandler.mergeQueryConfig();
18+
expect(config).toBeUndefined();
19+
});
20+
});
21+
1522
describe('enrich', () => {
1623
it('unhandled subject details', async () => {
1724
const mockNotification = createPartialMockNotification({

src/renderer/utils/notifications/handlers/discussion.test.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ import {
1414
IconColor,
1515
type Link,
1616
} from '../../../types';
17-
import type {
18-
DiscussionDetailsFragment,
19-
DiscussionStateReason,
17+
import {
18+
type DiscussionDetailsFragment,
19+
DiscussionDetailsFragmentDoc,
20+
DiscussionMergeQueryFragmentDoc,
21+
type DiscussionStateReason,
2022
} from '../../api/graphql/generated/graphql';
2123
import { discussionHandler } from './discussion';
2224

@@ -25,6 +27,16 @@ const mockCommenter = createMockGraphQLAuthor('discussion-commenter');
2527
const mockReplier = createMockGraphQLAuthor('discussion-replier');
2628

2729
describe('renderer/utils/notifications/handlers/discussion.ts', () => {
30+
describe('mergeQueryConfig', () => {
31+
it('should return the correct query and response fragments', () => {
32+
const config = discussionHandler.mergeQueryConfig();
33+
34+
expect(config).toBeDefined();
35+
expect(config.queryFragment).toBe(DiscussionMergeQueryFragmentDoc);
36+
expect(config.responseFragment).toBe(DiscussionDetailsFragmentDoc);
37+
});
38+
});
39+
2840
describe('enrich', () => {
2941
const mockNotification = createPartialMockNotification({
3042
title: 'This is a mock discussion',

src/renderer/utils/notifications/handlers/discussion.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,6 @@ class DiscussionHandler extends DefaultHandler {
3737
return {
3838
queryFragment: DiscussionMergeQueryFragmentDoc,
3939
responseFragment: DiscussionDetailsFragmentDoc,
40-
extras: [
41-
{ name: 'lastComments', type: 'Int', defaultValue: 100 },
42-
{ name: 'lastReplies', type: 'Int', defaultValue: 100 },
43-
{ name: 'firstLabels', type: 'Int', defaultValue: 100 },
44-
{ name: 'includeIsAnswered', type: 'Boolean!', defaultValue: true },
45-
],
4640
} as GraphQLMergedQueryConfig;
4741
}
4842

src/renderer/utils/notifications/handlers/issue.test.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,29 @@ import {
1414
IconColor,
1515
type Link,
1616
} from '../../../types';
17-
import type {
18-
IssueDetailsFragment,
19-
IssueState,
20-
IssueStateReason,
17+
import {
18+
type IssueDetailsFragment,
19+
IssueDetailsFragmentDoc,
20+
IssueMergeQueryFragmentDoc,
21+
type IssueState,
22+
type IssueStateReason,
2123
} from '../../api/graphql/generated/graphql';
2224
import { issueHandler } from './issue';
2325

2426
const mockAuthor = createMockGraphQLAuthor('issue-author');
2527
const mockCommenter = createMockGraphQLAuthor('issue-commenter');
2628

2729
describe('renderer/utils/notifications/handlers/issue.ts', () => {
30+
describe('mergeQueryConfig', () => {
31+
it('should return the correct query and response fragments', () => {
32+
const config = issueHandler.mergeQueryConfig();
33+
34+
expect(config).toBeDefined();
35+
expect(config.queryFragment).toBe(IssueMergeQueryFragmentDoc);
36+
expect(config.responseFragment).toBe(IssueDetailsFragmentDoc);
37+
});
38+
});
39+
2840
describe('enrich', () => {
2941
let mockNotification: GitifyNotification;
3042

src/renderer/utils/notifications/handlers/issue.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,7 @@ class IssueHandler extends DefaultHandler {
3232
mergeQueryConfig() {
3333
return {
3434
queryFragment: IssueMergeQueryFragmentDoc,
35-
3635
responseFragment: IssueDetailsFragmentDoc,
37-
extras: [
38-
{ name: 'lastComments', type: 'Int', defaultValue: 100 },
39-
{ name: 'firstLabels', type: 'Int', defaultValue: 100 },
40-
],
4136
} as GraphQLMergedQueryConfig;
4237
}
4338

src/renderer/utils/notifications/handlers/pullRequest.test.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ import {
1414
IconColor,
1515
type Link,
1616
} from '../../../types';
17-
import type {
18-
PullRequestDetailsFragment,
19-
PullRequestReviewState,
20-
PullRequestState,
17+
import {
18+
type PullRequestDetailsFragment,
19+
PullRequestDetailsFragmentDoc,
20+
PullRequestMergeQueryFragmentDoc,
21+
type PullRequestReviewState,
22+
type PullRequestState,
2123
} from '../../api/graphql/generated/graphql';
2224
import { getLatestReviewForReviewers, pullRequestHandler } from './pullRequest';
2325

@@ -37,6 +39,16 @@ describe('renderer/utils/notifications/handlers/pullRequest.ts', () => {
3739
});
3840
});
3941

42+
describe('mergeQueryConfig', () => {
43+
it('should return the correct query and response fragments', () => {
44+
const config = pullRequestHandler.mergeQueryConfig();
45+
46+
expect(config).toBeDefined();
47+
expect(config.queryFragment).toBe(PullRequestMergeQueryFragmentDoc);
48+
expect(config.responseFragment).toBe(PullRequestDetailsFragmentDoc);
49+
});
50+
});
51+
4052
describe('enrich', () => {
4153
beforeEach(() => {
4254
// axios will default to using the XHR adapter which can't be intercepted

src/renderer/utils/notifications/handlers/pullRequest.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,6 @@ class PullRequestHandler extends DefaultHandler {
3636
return {
3737
queryFragment: PullRequestMergeQueryFragmentDoc,
3838
responseFragment: PullRequestDetailsFragmentDoc,
39-
extras: [
40-
{ name: 'firstLabels', type: 'Int', defaultValue: 100 },
41-
{ name: 'lastComments', type: 'Int', defaultValue: 100 },
42-
{ name: 'lastReviews', type: 'Int', defaultValue: 100 },
43-
{ name: 'firstClosingIssues', type: 'Int', defaultValue: 100 },
44-
],
4539
} as GraphQLMergedQueryConfig;
4640
}
4741

src/renderer/utils/notifications/handlers/types.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,6 @@ import type { TypedDocumentString } from '../../api/graphql/generated/graphql';
1414
export type GraphQLMergedQueryConfig = {
1515
queryFragment: TypedDocumentString<unknown, unknown>;
1616
responseFragment: TypedDocumentString<unknown, unknown>;
17-
extras: Array<{
18-
name: string;
19-
type: string;
20-
defaultValue: number | boolean;
21-
}>;
2217
};
2318

2419
export interface NotificationTypeHandler<TFragment = unknown> {

src/renderer/utils/notifications/notifications.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
} from '../api/graphql/utils';
2323
import { transformNotification } from '../api/transform';
2424
import { getNumberFromUrl } from '../api/utils';
25+
import { isAnsweredDiscussionFeatureSupported } from '../features';
2526
import { rendererLogError, rendererLogWarn } from '../logger';
2627
import {
2728
filterBaseNotifications,
@@ -185,7 +186,6 @@ export async function enrichNotifications(
185186
notification.subject.type === 'PullRequest';
186187

187188
const alias = `node${index}`;
188-
// const queryFragmentBody = getQueryFragmentBody(config.queryFragment) ?? '';
189189
const queryFragmentBody = getQueryFragmentBody(
190190
BatchMergedDetailsQueryFragmentDoc,
191191
);
@@ -245,12 +245,11 @@ export async function enrichNotifications(
245245
lastComments: 1,
246246
lastThreadedComments: 10,
247247
lastReplies: 10,
248-
includeIsAnswered: true,
248+
includeIsAnswered: isAnsweredDiscussionFeatureSupported(
249+
notifications[0].account,
250+
),
249251
firstClosingIssues: 100,
250252
lastReviews: 100,
251-
// FIXME includeIsAnswered: isAnsweredDiscussionFeatureSupported(
252-
// notification.account,
253-
// ),
254253
};
255254

256255
let mergedData: Record<string, unknown> | null = null;

0 commit comments

Comments
 (0)