From c3a80f0e23b8b91c05c6a41d09011dd163a1dbac Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 19 Aug 2025 09:26:53 -0400 Subject: [PATCH 1/4] refactor: use handler for icon color Signed-off-by: Adam Setch --- .../notifications/NotificationRow.tsx | 3 +- .../utils/__snapshots__/icons.test.ts.snap | 30 ----- src/renderer/utils/icons.test.ts | 119 +----------------- src/renderer/utils/icons.ts | 25 +--- .../notifications/handlers/checkSuite.ts | 4 +- .../utils/notifications/handlers/commit.ts | 4 +- .../notifications/handlers/default.test.ts | 28 +++++ .../utils/notifications/handlers/default.ts | 22 +++- .../notifications/handlers/discussion.ts | 4 +- .../utils/notifications/handlers/issue.ts | 4 +- .../notifications/handlers/pullRequest.ts | 4 +- .../utils/notifications/handlers/release.ts | 10 +- .../repositoryDependabotAlertsThread.ts | 20 +-- .../handlers/repositoryInvitation.ts | 18 +-- .../handlers/repositoryVulnerabilityAlert.ts | 18 +-- .../utils/notifications/handlers/types.ts | 3 + .../notifications/handlers/workflowRun.ts | 4 +- 17 files changed, 83 insertions(+), 237 deletions(-) diff --git a/src/renderer/components/notifications/NotificationRow.tsx b/src/renderer/components/notifications/NotificationRow.tsx index 9ff012f82..6b1c122cf 100644 --- a/src/renderer/components/notifications/NotificationRow.tsx +++ b/src/renderer/components/notifications/NotificationRow.tsx @@ -9,7 +9,6 @@ import type { Notification } from '../../typesGitHub'; import { cn } from '../../utils/cn'; import { isMarkAsDoneFeatureSupported } from '../../utils/features'; import { formatForDisplay } from '../../utils/helpers'; -import { getNotificationTypeIconColor } from '../../utils/icons'; import { openNotification } from '../../utils/links'; import { createNotificationHandler } from '../../utils/notifications/handlers'; import { HoverButton } from '../primitives/HoverButton'; @@ -74,7 +73,7 @@ export const NotificationRow: FC = ({ const handler = createNotificationHandler(notification); const NotificationIcon = handler.getIcon(notification.subject); - const iconColor = getNotificationTypeIconColor(notification.subject); + const iconColor = handler.getIconColor(notification.subject); const notificationType = formatForDisplay([ notification.subject.state, diff --git a/src/renderer/utils/__snapshots__/icons.test.ts.snap b/src/renderer/utils/__snapshots__/icons.test.ts.snap index c5ea6da45..fba756f8d 100644 --- a/src/renderer/utils/__snapshots__/icons.test.ts.snap +++ b/src/renderer/utils/__snapshots__/icons.test.ts.snap @@ -34,33 +34,3 @@ exports[` 5`] = ` "render": [Function], } `; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for check suite 1`] = `"text-gitify-icon-muted"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for check suite 2`] = `"text-gitify-icon-closed"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for check suite 3`] = `"text-gitify-icon-muted"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for check suite 4`] = `"text-gitify-icon-open"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for check suite 5`] = `"text-gitify-icon-muted"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 1`] = `"text-gitify-icon-open"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 2`] = `"text-gitify-icon-closed"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 3`] = `"text-gitify-icon-done"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 4`] = `"text-gitify-icon-muted"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 5`] = `"text-gitify-icon-done"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 6`] = `"text-gitify-icon-muted"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 7`] = `"text-gitify-icon-open"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 8`] = `"text-gitify-icon-open"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 9`] = `"text-gitify-icon-done"`; - -exports[`renderer/utils/icons.ts getNotificationTypeIconColor should format the notification color for state 10`] = `"text-gitify-icon-muted"`; diff --git a/src/renderer/utils/icons.test.ts b/src/renderer/utils/icons.test.ts index 4ecf91c4a..1ca83f48e 100644 --- a/src/renderer/utils/icons.test.ts +++ b/src/renderer/utils/icons.test.ts @@ -8,118 +8,15 @@ import { } from '@primer/octicons-react'; import { IconColor } from '../types'; -import type { - GitifyPullRequestReview, - StateType, - Subject, - SubjectType, -} from '../typesGitHub'; +import type { GitifyPullRequestReview } from '../typesGitHub'; import { getAuthMethodIcon, getDefaultUserIcon, - getNotificationTypeIconColor, getPlatformIcon, getPullRequestReviewIcon, } from './icons'; describe('renderer/utils/icons.ts', () => { - describe('getNotificationTypeIconColor', () => { - it('should format the notification color for check suite', () => { - expect( - getNotificationTypeIconColor( - createSubjectMock({ - type: 'CheckSuite', - state: 'cancelled', - }), - ), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor( - createSubjectMock({ - type: 'CheckSuite', - state: 'failure', - }), - ), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor( - createSubjectMock({ - type: 'CheckSuite', - state: 'skipped', - }), - ), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor( - createSubjectMock({ - type: 'CheckSuite', - state: 'success', - }), - ), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor( - createSubjectMock({ - type: 'CheckSuite', - state: null, - }), - ), - ).toMatchSnapshot(); - }); - - it('should format the notification color for state', () => { - expect( - getNotificationTypeIconColor(createSubjectMock({ state: 'ANSWERED' })), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor(createSubjectMock({ state: 'closed' })), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor(createSubjectMock({ state: 'completed' })), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor(createSubjectMock({ state: 'draft' })), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor(createSubjectMock({ state: 'merged' })), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor( - createSubjectMock({ state: 'not_planned' }), - ), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor(createSubjectMock({ state: 'open' })), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor(createSubjectMock({ state: 'reopened' })), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor(createSubjectMock({ state: 'RESOLVED' })), - ).toMatchSnapshot(); - - expect( - getNotificationTypeIconColor( - createSubjectMock({ - state: 'something_else_unknown' as StateType, - }), - ), - ).toMatchSnapshot(); - }); - }); - describe('getPullRequestReviewIcon', () => { let mockReviewSingleReviewer: GitifyPullRequestReview; let mockReviewMultipleReviewer: GitifyPullRequestReview; @@ -235,17 +132,3 @@ describe('renderer/utils/icons.ts', () => { expect(getDefaultUserIcon('User')).toBe(FeedPersonIcon); }); }); - -function createSubjectMock(mocks: { - title?: string; - type?: SubjectType; - state?: StateType; -}): Subject { - return { - title: mocks.title ?? 'Mock Subject', - type: mocks.type ?? ('Unknown' as SubjectType), - state: mocks.state ?? ('Unknown' as StateType), - url: null, - latest_comment_url: null, - }; -} diff --git a/src/renderer/utils/icons.ts b/src/renderer/utils/icons.ts index c9048aca8..1f472ff5d 100644 --- a/src/renderer/utils/icons.ts +++ b/src/renderer/utils/icons.ts @@ -15,32 +15,9 @@ import { } from '@primer/octicons-react'; import { IconColor, type PullRequestApprovalIcon } from '../types'; -import type { - GitifyPullRequestReview, - Subject, - UserType, -} from '../typesGitHub'; +import type { GitifyPullRequestReview, UserType } from '../typesGitHub'; import type { AuthMethod, PlatformType } from './auth/types'; -export function getNotificationTypeIconColor(subject: Subject): IconColor { - switch (subject.state) { - case 'open': - case 'reopened': - case 'ANSWERED': - case 'success': - return IconColor.GREEN; - case 'closed': - case 'failure': - return IconColor.RED; - case 'completed': - case 'RESOLVED': - case 'merged': - return IconColor.PURPLE; - default: - return IconColor.GRAY; - } -} - export function getPullRequestReviewIcon( review: GitifyPullRequestReview, ): PullRequestApprovalIcon | null { diff --git a/src/renderer/utils/notifications/handlers/checkSuite.ts b/src/renderer/utils/notifications/handlers/checkSuite.ts index 1f129b480..358945ac4 100644 --- a/src/renderer/utils/notifications/handlers/checkSuite.ts +++ b/src/renderer/utils/notifications/handlers/checkSuite.ts @@ -17,9 +17,9 @@ import type { Notification, Subject, } from '../../../typesGitHub'; -import type { NotificationTypeHandler } from './types'; +import { DefaultHandler } from './default'; -class CheckSuiteHandler implements NotificationTypeHandler { +class CheckSuiteHandler extends DefaultHandler { readonly type = 'CheckSuite'; async enrich( diff --git a/src/renderer/utils/notifications/handlers/commit.ts b/src/renderer/utils/notifications/handlers/commit.ts index a2f8c47e1..ab339aafc 100644 --- a/src/renderer/utils/notifications/handlers/commit.ts +++ b/src/renderer/utils/notifications/handlers/commit.ts @@ -13,10 +13,10 @@ import type { } from '../../../typesGitHub'; import { getCommit, getCommitComment } from '../../api/client'; import { isStateFilteredOut } from '../filters/filter'; -import type { NotificationTypeHandler } from './types'; +import { DefaultHandler } from './default'; import { getSubjectUser } from './utils'; -class CommitHandler implements NotificationTypeHandler { +class CommitHandler extends DefaultHandler { readonly type = 'Commit'; async enrich( diff --git a/src/renderer/utils/notifications/handlers/default.test.ts b/src/renderer/utils/notifications/handlers/default.test.ts index 8f1db8780..2ae052b77 100644 --- a/src/renderer/utils/notifications/handlers/default.test.ts +++ b/src/renderer/utils/notifications/handlers/default.test.ts @@ -1,6 +1,8 @@ import { createSubjectMock } from '../../../__mocks__/notifications-mocks'; import { partialMockNotification } from '../../../__mocks__/partial-mocks'; import { mockSettings } from '../../../__mocks__/state-mocks'; +import { IconColor } from '../../../types'; +import type { StateType } from '../../../typesGitHub'; import { defaultHandler } from './default'; describe('renderer/utils/notifications/handlers/default.ts', () => { @@ -26,4 +28,30 @@ describe('renderer/utils/notifications/handlers/default.ts', () => { 'QuestionIcon', ); }); + + describe('getIconColor', () => { + const cases: Array<[StateType | null, IconColor]> = [ + ['open' as StateType, IconColor.GREEN], + ['reopened' as StateType, IconColor.GREEN], + ['ANSWERED' as StateType, IconColor.GREEN], + ['success' as StateType, IconColor.GREEN], + ['closed' as StateType, IconColor.RED], + ['failure' as StateType, IconColor.RED], + ['completed' as StateType, IconColor.PURPLE], + ['RESOLVED' as StateType, IconColor.PURPLE], + ['merged' as StateType, IconColor.PURPLE], + ['not_planned' as StateType, IconColor.GRAY], + ['draft' as StateType, IconColor.GRAY], + ['skipped' as StateType, IconColor.GRAY], + ['cancelled' as StateType, IconColor.GRAY], + ['unknown' as StateType, IconColor.GRAY], + [null, IconColor.GRAY], + [undefined, IconColor.GRAY], + ]; + + it.each(cases)('returns correct color for state %s', (state, expected) => { + const subject = createSubjectMock({ state }); + expect(defaultHandler.getIconColor(subject)).toBe(expected); + }); + }); }); diff --git a/src/renderer/utils/notifications/handlers/default.ts b/src/renderer/utils/notifications/handlers/default.ts index 8c43c407f..55c364aa9 100644 --- a/src/renderer/utils/notifications/handlers/default.ts +++ b/src/renderer/utils/notifications/handlers/default.ts @@ -4,6 +4,7 @@ import type { OcticonProps } from '@primer/octicons-react'; import { QuestionIcon } from '@primer/octicons-react'; import type { SettingsState } from '../../../types'; +import { IconColor } from '../../../types'; import type { GitifySubject, Notification, @@ -11,7 +12,7 @@ import type { } from '../../../typesGitHub'; import type { NotificationTypeHandler } from './types'; -class DefaultHandler implements NotificationTypeHandler { +export class DefaultHandler implements NotificationTypeHandler { async enrich( _notification: Notification, _settings: SettingsState, @@ -22,6 +23,25 @@ class DefaultHandler implements NotificationTypeHandler { getIcon(_subject: Subject): FC | null { return QuestionIcon; } + + getIconColor(subject: Subject): IconColor { + switch (subject.state) { + case 'open': + case 'reopened': + case 'ANSWERED': + case 'success': + return IconColor.GREEN; + case 'closed': + case 'failure': + return IconColor.RED; + case 'completed': + case 'RESOLVED': + case 'merged': + return IconColor.PURPLE; + default: + return IconColor.GRAY; + } + } } export const defaultHandler = new DefaultHandler(); diff --git a/src/renderer/utils/notifications/handlers/discussion.ts b/src/renderer/utils/notifications/handlers/discussion.ts index 755e69176..f5476388f 100644 --- a/src/renderer/utils/notifications/handlers/discussion.ts +++ b/src/renderer/utils/notifications/handlers/discussion.ts @@ -21,9 +21,9 @@ import type { } from '../../../typesGitHub'; import { getLatestDiscussion } from '../../api/client'; import { isStateFilteredOut } from '../filters/filter'; -import type { NotificationTypeHandler } from './types'; +import { DefaultHandler } from './default'; -class DiscussionHandler implements NotificationTypeHandler { +class DiscussionHandler extends DefaultHandler { readonly type = 'Discussion'; async enrich( diff --git a/src/renderer/utils/notifications/handlers/issue.ts b/src/renderer/utils/notifications/handlers/issue.ts index 46f1dbffd..1c885cdac 100644 --- a/src/renderer/utils/notifications/handlers/issue.ts +++ b/src/renderer/utils/notifications/handlers/issue.ts @@ -18,10 +18,10 @@ import type { } from '../../../typesGitHub'; import { getIssue, getIssueOrPullRequestComment } from '../../api/client'; import { isStateFilteredOut } from '../filters/filter'; -import type { NotificationTypeHandler } from './types'; +import { DefaultHandler } from './default'; import { getSubjectUser } from './utils'; -class IssueHandler implements NotificationTypeHandler { +class IssueHandler extends DefaultHandler { readonly type = 'Issue'; async enrich( diff --git a/src/renderer/utils/notifications/handlers/pullRequest.ts b/src/renderer/utils/notifications/handlers/pullRequest.ts index baf53b3ee..b8de4c5a1 100644 --- a/src/renderer/utils/notifications/handlers/pullRequest.ts +++ b/src/renderer/utils/notifications/handlers/pullRequest.ts @@ -25,10 +25,10 @@ import { getPullRequestReviews, } from '../../api/client'; import { isStateFilteredOut, isUserFilteredOut } from '../filters/filter'; -import type { NotificationTypeHandler } from './types'; +import { DefaultHandler } from './default'; import { getSubjectUser } from './utils'; -class PullRequestHandler implements NotificationTypeHandler { +class PullRequestHandler extends DefaultHandler { readonly type = 'PullRequest' as const; async enrich( diff --git a/src/renderer/utils/notifications/handlers/release.ts b/src/renderer/utils/notifications/handlers/release.ts index 6400ef362..76b42ab98 100644 --- a/src/renderer/utils/notifications/handlers/release.ts +++ b/src/renderer/utils/notifications/handlers/release.ts @@ -3,7 +3,7 @@ import type { FC } from 'react'; import type { OcticonProps } from '@primer/octicons-react'; import { TagIcon } from '@primer/octicons-react'; -import type { SettingsState } from '../../../types'; +import { IconColor, type SettingsState } from '../../../types'; import type { GitifySubject, Notification, @@ -12,10 +12,10 @@ import type { } from '../../../typesGitHub'; import { getRelease } from '../../api/client'; import { isStateFilteredOut } from '../filters/filter'; -import type { NotificationTypeHandler } from './types'; +import { DefaultHandler } from './default'; import { getSubjectUser } from './utils'; -class ReleaseHandler implements NotificationTypeHandler { +class ReleaseHandler extends DefaultHandler { readonly type = 'Release'; async enrich( @@ -42,6 +42,10 @@ class ReleaseHandler implements NotificationTypeHandler { getIcon(_subject: Subject): FC | null { return TagIcon; } + + getIconColor(_subject: Subject): IconColor { + return IconColor.GREEN; + } } export const releaseHandler = new ReleaseHandler(); diff --git a/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.ts b/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.ts index 6fd5a49f5..98b386b63 100644 --- a/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.ts +++ b/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.ts @@ -3,26 +3,12 @@ import type { FC } from 'react'; import type { OcticonProps } from '@primer/octicons-react'; import { AlertIcon } from '@primer/octicons-react'; -import type { SettingsState } from '../../../types'; -import type { - GitifySubject, - Notification, - Subject, -} from '../../../typesGitHub'; -import type { NotificationTypeHandler } from './types'; +import type { Subject } from '../../../typesGitHub'; +import { DefaultHandler } from './default'; -class RepositoryDependabotAlertsThreadHandler - implements NotificationTypeHandler -{ +class RepositoryDependabotAlertsThreadHandler extends DefaultHandler { readonly type = 'RepositoryDependabotAlertsThread'; - async enrich( - _notification: Notification, - _settings: SettingsState, - ): Promise { - return; - } - getIcon(_subject: Subject): FC | null { return AlertIcon; } diff --git a/src/renderer/utils/notifications/handlers/repositoryInvitation.ts b/src/renderer/utils/notifications/handlers/repositoryInvitation.ts index 6eac000fc..35f46bd73 100644 --- a/src/renderer/utils/notifications/handlers/repositoryInvitation.ts +++ b/src/renderer/utils/notifications/handlers/repositoryInvitation.ts @@ -2,24 +2,12 @@ import type { FC } from 'react'; import { MailIcon, type OcticonProps } from '@primer/octicons-react'; -import type { SettingsState } from '../../../types'; -import type { - GitifySubject, - Notification, - Subject, -} from '../../../typesGitHub'; -import type { NotificationTypeHandler } from './types'; +import type { Subject } from '../../../typesGitHub'; +import { DefaultHandler } from './default'; -class RepositoryInvitationHandler implements NotificationTypeHandler { +class RepositoryInvitationHandler extends DefaultHandler { readonly type = 'RepositoryInvitation'; - async enrich( - _notification: Notification, - _settings: SettingsState, - ): Promise { - return; - } - getIcon(_subject: Subject): FC | null { return MailIcon; } diff --git a/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.ts b/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.ts index 5647d6ebe..fe679ca82 100644 --- a/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.ts +++ b/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.ts @@ -2,24 +2,12 @@ import type { FC } from 'react'; import { AlertIcon, type OcticonProps } from '@primer/octicons-react'; -import type { SettingsState } from '../../../types'; -import type { - GitifySubject, - Notification, - Subject, -} from '../../../typesGitHub'; -import type { NotificationTypeHandler } from './types'; +import type { Subject } from '../../../typesGitHub'; +import { DefaultHandler } from './default'; -class RepositoryVulnerabilityAlertHandler implements NotificationTypeHandler { +class RepositoryVulnerabilityAlertHandler extends DefaultHandler { readonly type = 'RepositoryVulnerabilityAlert'; - async enrich( - _notification: Notification, - _settings: SettingsState, - ): Promise { - return; - } - getIcon(_subject: Subject): FC | null { return AlertIcon; } diff --git a/src/renderer/utils/notifications/handlers/types.ts b/src/renderer/utils/notifications/handlers/types.ts index 7f377298b..436d548e2 100644 --- a/src/renderer/utils/notifications/handlers/types.ts +++ b/src/renderer/utils/notifications/handlers/types.ts @@ -23,4 +23,7 @@ export interface NotificationTypeHandler { /** Return an icon component for this notification type. */ getIcon(subject: Subject): FC | null; + + /** Return a color for this notification type. */ + getIconColor(subject: Subject): string | undefined; } diff --git a/src/renderer/utils/notifications/handlers/workflowRun.ts b/src/renderer/utils/notifications/handlers/workflowRun.ts index 9dbd4a493..ab242450f 100644 --- a/src/renderer/utils/notifications/handlers/workflowRun.ts +++ b/src/renderer/utils/notifications/handlers/workflowRun.ts @@ -11,9 +11,9 @@ import type { Subject, WorkflowRunAttributes, } from '../../../typesGitHub'; -import type { NotificationTypeHandler } from './types'; +import { DefaultHandler } from './default'; -class WorkflowRunHandler implements NotificationTypeHandler { +class WorkflowRunHandler extends DefaultHandler { readonly type = 'WorkflowRun'; async enrich( From cbb9e9e8233452a7ce16d28a6522879ac4832488 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 19 Aug 2025 11:00:07 -0400 Subject: [PATCH 2/4] refactor: handler enhancements Signed-off-by: Adam Setch --- .../notifications/NotificationRow.tsx | 21 ++++---------- .../notifications/handlers/checkSuite.test.ts | 12 ++++---- .../notifications/handlers/checkSuite.ts | 2 +- .../notifications/handlers/commit.test.ts | 4 +-- .../utils/notifications/handlers/commit.ts | 2 +- .../notifications/handlers/default.test.ts | 8 +++--- .../utils/notifications/handlers/default.ts | 28 +++++++++++++++++-- .../notifications/handlers/discussion.test.ts | 10 +++---- .../notifications/handlers/discussion.ts | 2 +- .../notifications/handlers/issue.test.ts | 17 +++++------ .../utils/notifications/handlers/issue.ts | 2 +- .../handlers/pullRequest.test.ts | 10 +++---- .../notifications/handlers/pullRequest.ts | 2 +- .../notifications/handlers/release.test.ts | 4 +-- .../utils/notifications/handlers/release.ts | 8 ++---- .../repositoryDependabotAlertsThread.test.ts | 4 +-- .../repositoryDependabotAlertsThread.ts | 2 +- .../handlers/repositoryInvitation.test.ts | 4 +-- .../handlers/repositoryInvitation.ts | 2 +- .../repositoryVulnerabilityAlert.test.ts | 4 +-- .../handlers/repositoryVulnerabilityAlert.ts | 2 +- .../utils/notifications/handlers/types.ts | 27 +++++++++++++++--- .../handlers/workflowRun.test.ts | 4 +-- .../notifications/handlers/workflowRun.ts | 2 +- 24 files changed, 106 insertions(+), 77 deletions(-) diff --git a/src/renderer/components/notifications/NotificationRow.tsx b/src/renderer/components/notifications/NotificationRow.tsx index 6b1c122cf..ef90c0466 100644 --- a/src/renderer/components/notifications/NotificationRow.tsx +++ b/src/renderer/components/notifications/NotificationRow.tsx @@ -8,7 +8,6 @@ import { GroupBy, Opacity, Size } from '../../types'; import type { Notification } from '../../typesGitHub'; import { cn } from '../../utils/cn'; import { isMarkAsDoneFeatureSupported } from '../../utils/features'; -import { formatForDisplay } from '../../utils/helpers'; import { openNotification } from '../../utils/links'; import { createNotificationHandler } from '../../utils/notifications/handlers'; import { HoverButton } from '../primitives/HoverButton'; @@ -72,21 +71,11 @@ export const NotificationRow: FC = ({ const handler = createNotificationHandler(notification); - const NotificationIcon = handler.getIcon(notification.subject); - const iconColor = handler.getIconColor(notification.subject); - - const notificationType = formatForDisplay([ - notification.subject.state, - notification.subject.type, - ]); - - const notificationNumber = notification.subject?.number - ? `#${notification.subject.number}` - : ''; - - const notificationTitle = notificationNumber - ? `${notification.subject.title} [${notificationNumber}]` - : notification.subject.title; + const NotificationIcon = handler.iconType(notification.subject); + const iconColor = handler.iconColor(notification.subject); + const notificationType = handler.formattedNotificationType(notification); + const notificationNumber = handler.formattedNotificationNumber(notification); + const notificationTitle = handler.formattedNotificationTitle(notification); const groupByDate = settings.groupBy === GroupBy.DATE; diff --git a/src/renderer/utils/notifications/handlers/checkSuite.test.ts b/src/renderer/utils/notifications/handlers/checkSuite.test.ts index caaa96639..8f50f8af1 100644 --- a/src/renderer/utils/notifications/handlers/checkSuite.test.ts +++ b/src/renderer/utils/notifications/handlers/checkSuite.test.ts @@ -136,15 +136,15 @@ describe('renderer/utils/notifications/handlers/checkSuite.ts', () => { }); }); - it('getIcon', () => { + it('iconType', () => { expect( - checkSuiteHandler.getIcon( + checkSuiteHandler.iconType( createSubjectMock({ type: 'CheckSuite', state: null }), ).displayName, ).toBe('RocketIcon'); expect( - checkSuiteHandler.getIcon( + checkSuiteHandler.iconType( createSubjectMock({ type: 'CheckSuite', state: 'cancelled', @@ -153,7 +153,7 @@ describe('renderer/utils/notifications/handlers/checkSuite.ts', () => { ).toBe('StopIcon'); expect( - checkSuiteHandler.getIcon( + checkSuiteHandler.iconType( createSubjectMock({ type: 'CheckSuite', state: 'failure', @@ -162,7 +162,7 @@ describe('renderer/utils/notifications/handlers/checkSuite.ts', () => { ).toBe('XIcon'); expect( - checkSuiteHandler.getIcon( + checkSuiteHandler.iconType( createSubjectMock({ type: 'CheckSuite', state: 'skipped', @@ -171,7 +171,7 @@ describe('renderer/utils/notifications/handlers/checkSuite.ts', () => { ).toBe('SkipIcon'); expect( - checkSuiteHandler.getIcon( + checkSuiteHandler.iconType( createSubjectMock({ type: 'CheckSuite', state: 'success', diff --git a/src/renderer/utils/notifications/handlers/checkSuite.ts b/src/renderer/utils/notifications/handlers/checkSuite.ts index 358945ac4..df38346b5 100644 --- a/src/renderer/utils/notifications/handlers/checkSuite.ts +++ b/src/renderer/utils/notifications/handlers/checkSuite.ts @@ -38,7 +38,7 @@ class CheckSuiteHandler extends DefaultHandler { return null; } - getIcon(subject: Subject): FC | null { + iconType(subject: Subject): FC | null { switch (subject.state) { case 'cancelled': return StopIcon; diff --git a/src/renderer/utils/notifications/handlers/commit.test.ts b/src/renderer/utils/notifications/handlers/commit.test.ts index 854cb579c..bf9d26ba9 100644 --- a/src/renderer/utils/notifications/handlers/commit.test.ts +++ b/src/renderer/utils/notifications/handlers/commit.test.ts @@ -97,9 +97,9 @@ describe('renderer/utils/notifications/handlers/commit.ts', () => { }); }); - it('getIcon', () => { + it('iconType', () => { expect( - commitHandler.getIcon(createSubjectMock({ type: 'Commit' })).displayName, + commitHandler.iconType(createSubjectMock({ type: 'Commit' })).displayName, ).toBe('GitCommitIcon'); }); }); diff --git a/src/renderer/utils/notifications/handlers/commit.ts b/src/renderer/utils/notifications/handlers/commit.ts index ab339aafc..882134f03 100644 --- a/src/renderer/utils/notifications/handlers/commit.ts +++ b/src/renderer/utils/notifications/handlers/commit.ts @@ -55,7 +55,7 @@ class CommitHandler extends DefaultHandler { }; } - getIcon(_subject: Subject): FC | null { + iconType(_subject: Subject): FC | null { return GitCommitIcon; } } diff --git a/src/renderer/utils/notifications/handlers/default.test.ts b/src/renderer/utils/notifications/handlers/default.test.ts index 2ae052b77..84333512d 100644 --- a/src/renderer/utils/notifications/handlers/default.test.ts +++ b/src/renderer/utils/notifications/handlers/default.test.ts @@ -23,13 +23,13 @@ describe('renderer/utils/notifications/handlers/default.ts', () => { }); }); - it('getIcon', () => { - expect(defaultHandler.getIcon(createSubjectMock({})).displayName).toBe( + it('iconType', () => { + expect(defaultHandler.iconType(createSubjectMock({})).displayName).toBe( 'QuestionIcon', ); }); - describe('getIconColor', () => { + describe('iconColor', () => { const cases: Array<[StateType | null, IconColor]> = [ ['open' as StateType, IconColor.GREEN], ['reopened' as StateType, IconColor.GREEN], @@ -51,7 +51,7 @@ describe('renderer/utils/notifications/handlers/default.ts', () => { it.each(cases)('returns correct color for state %s', (state, expected) => { const subject = createSubjectMock({ state }); - expect(defaultHandler.getIconColor(subject)).toBe(expected); + expect(defaultHandler.iconColor(subject)).toBe(expected); }); }); }); diff --git a/src/renderer/utils/notifications/handlers/default.ts b/src/renderer/utils/notifications/handlers/default.ts index 55c364aa9..f1634e3ee 100644 --- a/src/renderer/utils/notifications/handlers/default.ts +++ b/src/renderer/utils/notifications/handlers/default.ts @@ -9,10 +9,14 @@ import type { GitifySubject, Notification, Subject, + SubjectType, } from '../../../typesGitHub'; +import { formatForDisplay } from '../../helpers'; import type { NotificationTypeHandler } from './types'; export class DefaultHandler implements NotificationTypeHandler { + type?: SubjectType; + async enrich( _notification: Notification, _settings: SettingsState, @@ -20,11 +24,11 @@ export class DefaultHandler implements NotificationTypeHandler { return null; } - getIcon(_subject: Subject): FC | null { + iconType(_subject: Subject): FC | null { return QuestionIcon; } - getIconColor(subject: Subject): IconColor { + iconColor(subject: Subject): IconColor { switch (subject.state) { case 'open': case 'reopened': @@ -42,6 +46,26 @@ export class DefaultHandler implements NotificationTypeHandler { return IconColor.GRAY; } } + + formattedNotificationType(notification: Notification): string { + return formatForDisplay([ + notification.subject.state, + notification.subject.type, + ]); + } + formattedNotificationNumber(notification: Notification): string { + return notification.subject?.number + ? `#${notification.subject.number}` + : ''; + } + formattedNotificationTitle(notification: Notification): string { + let title = notification.subject.title; + + if (notification.subject?.number) { + title = `${title} [${this.formattedNotificationNumber(notification)}]`; + } + return title; + } } export const defaultHandler = new DefaultHandler(); diff --git a/src/renderer/utils/notifications/handlers/discussion.test.ts b/src/renderer/utils/notifications/handlers/discussion.test.ts index 5f03ddb93..e81878886 100644 --- a/src/renderer/utils/notifications/handlers/discussion.test.ts +++ b/src/renderer/utils/notifications/handlers/discussion.test.ts @@ -279,26 +279,26 @@ describe('renderer/utils/notifications/handlers/discussion.ts', () => { }); }); - it('getIcon', () => { + it('iconType', () => { expect( - discussionHandler.getIcon(createSubjectMock({ type: 'Discussion' })) + discussionHandler.iconType(createSubjectMock({ type: 'Discussion' })) .displayName, ).toBe('CommentDiscussionIcon'); expect( - discussionHandler.getIcon( + discussionHandler.iconType( createSubjectMock({ type: 'Discussion', state: 'DUPLICATE' }), ).displayName, ).toBe('DiscussionDuplicateIcon'); expect( - discussionHandler.getIcon( + discussionHandler.iconType( createSubjectMock({ type: 'Discussion', state: 'OUTDATED' }), ).displayName, ).toBe('DiscussionOutdatedIcon'); expect( - discussionHandler.getIcon( + discussionHandler.iconType( createSubjectMock({ type: 'Discussion', state: 'RESOLVED' }), ).displayName, ).toBe('DiscussionClosedIcon'); diff --git a/src/renderer/utils/notifications/handlers/discussion.ts b/src/renderer/utils/notifications/handlers/discussion.ts index f5476388f..255cd68b1 100644 --- a/src/renderer/utils/notifications/handlers/discussion.ts +++ b/src/renderer/utils/notifications/handlers/discussion.ts @@ -77,7 +77,7 @@ class DiscussionHandler extends DefaultHandler { }; } - getIcon(subject: Subject): FC | null { + iconType(subject: Subject): FC | null { switch (subject.state) { case 'DUPLICATE': return DiscussionDuplicateIcon; diff --git a/src/renderer/utils/notifications/handlers/issue.test.ts b/src/renderer/utils/notifications/handlers/issue.test.ts index cf437b490..511aaeee6 100644 --- a/src/renderer/utils/notifications/handlers/issue.test.ts +++ b/src/renderer/utils/notifications/handlers/issue.test.ts @@ -294,18 +294,19 @@ describe('renderer/utils/notifications/handlers/issue.ts', () => { }); }); - it('getIcon', () => { + it('iconType', () => { expect( - issueHandler.getIcon(createSubjectMock({ type: 'Issue' })).displayName, + issueHandler.iconType(createSubjectMock({ type: 'Issue' })).displayName, ).toBe('IssueOpenedIcon'); expect( - issueHandler.getIcon(createSubjectMock({ type: 'Issue', state: 'draft' })) - .displayName, + issueHandler.iconType( + createSubjectMock({ type: 'Issue', state: 'draft' }), + ).displayName, ).toBe('IssueDraftIcon'); expect( - issueHandler.getIcon( + issueHandler.iconType( createSubjectMock({ type: 'Issue', state: 'closed', @@ -314,7 +315,7 @@ describe('renderer/utils/notifications/handlers/issue.ts', () => { ).toBe('IssueClosedIcon'); expect( - issueHandler.getIcon( + issueHandler.iconType( createSubjectMock({ type: 'Issue', state: 'completed', @@ -323,7 +324,7 @@ describe('renderer/utils/notifications/handlers/issue.ts', () => { ).toBe('IssueClosedIcon'); expect( - issueHandler.getIcon( + issueHandler.iconType( createSubjectMock({ type: 'Issue', state: 'not_planned', @@ -332,7 +333,7 @@ describe('renderer/utils/notifications/handlers/issue.ts', () => { ).toBe('SkipIcon'); expect( - issueHandler.getIcon( + issueHandler.iconType( createSubjectMock({ type: 'Issue', state: 'reopened', diff --git a/src/renderer/utils/notifications/handlers/issue.ts b/src/renderer/utils/notifications/handlers/issue.ts index 1c885cdac..74b637aad 100644 --- a/src/renderer/utils/notifications/handlers/issue.ts +++ b/src/renderer/utils/notifications/handlers/issue.ts @@ -61,7 +61,7 @@ class IssueHandler extends DefaultHandler { }; } - getIcon(subject: Subject): FC | null { + iconType(subject: Subject): FC | null { switch (subject.state) { case 'draft': return IssueDraftIcon; diff --git a/src/renderer/utils/notifications/handlers/pullRequest.test.ts b/src/renderer/utils/notifications/handlers/pullRequest.test.ts index 6c0a440e2..c70f264bd 100644 --- a/src/renderer/utils/notifications/handlers/pullRequest.test.ts +++ b/src/renderer/utils/notifications/handlers/pullRequest.test.ts @@ -438,14 +438,14 @@ describe('renderer/utils/notifications/handlers/pullRequest.ts', () => { }); }); - it('getIcon', () => { + it('iconType', () => { expect( - pullRequestHandler.getIcon(createSubjectMock({ type: 'PullRequest' })) + pullRequestHandler.iconType(createSubjectMock({ type: 'PullRequest' })) .displayName, ).toBe('GitPullRequestIcon'); expect( - pullRequestHandler.getIcon( + pullRequestHandler.iconType( createSubjectMock({ type: 'PullRequest', state: 'draft', @@ -454,7 +454,7 @@ describe('renderer/utils/notifications/handlers/pullRequest.ts', () => { ).toBe('GitPullRequestDraftIcon'); expect( - pullRequestHandler.getIcon( + pullRequestHandler.iconType( createSubjectMock({ type: 'PullRequest', state: 'closed', @@ -463,7 +463,7 @@ describe('renderer/utils/notifications/handlers/pullRequest.ts', () => { ).toBe('GitPullRequestClosedIcon'); expect( - pullRequestHandler.getIcon( + pullRequestHandler.iconType( createSubjectMock({ type: 'PullRequest', state: 'merged', diff --git a/src/renderer/utils/notifications/handlers/pullRequest.ts b/src/renderer/utils/notifications/handlers/pullRequest.ts index b8de4c5a1..2d0617c5c 100644 --- a/src/renderer/utils/notifications/handlers/pullRequest.ts +++ b/src/renderer/utils/notifications/handlers/pullRequest.ts @@ -87,7 +87,7 @@ class PullRequestHandler extends DefaultHandler { }; } - getIcon(subject: Subject): FC | null { + iconType(subject: Subject): FC | null { switch (subject.state) { case 'draft': return GitPullRequestDraftIcon; diff --git a/src/renderer/utils/notifications/handlers/release.test.ts b/src/renderer/utils/notifications/handlers/release.test.ts index fb34d840c..49e6f87a4 100644 --- a/src/renderer/utils/notifications/handlers/release.test.ts +++ b/src/renderer/utils/notifications/handlers/release.test.ts @@ -67,9 +67,9 @@ describe('renderer/utils/notifications/handlers/release.ts', () => { }); }); - it('getIcon', () => { + it('iconType', () => { expect( - releaseHandler.getIcon( + releaseHandler.iconType( createSubjectMock({ type: 'Release', }), diff --git a/src/renderer/utils/notifications/handlers/release.ts b/src/renderer/utils/notifications/handlers/release.ts index 76b42ab98..9caadc1a7 100644 --- a/src/renderer/utils/notifications/handlers/release.ts +++ b/src/renderer/utils/notifications/handlers/release.ts @@ -3,7 +3,7 @@ import type { FC } from 'react'; import type { OcticonProps } from '@primer/octicons-react'; import { TagIcon } from '@primer/octicons-react'; -import { IconColor, type SettingsState } from '../../../types'; +import type { SettingsState } from '../../../types'; import type { GitifySubject, Notification, @@ -39,13 +39,9 @@ class ReleaseHandler extends DefaultHandler { }; } - getIcon(_subject: Subject): FC | null { + iconType(_subject: Subject): FC | null { return TagIcon; } - - getIconColor(_subject: Subject): IconColor { - return IconColor.GREEN; - } } export const releaseHandler = new ReleaseHandler(); diff --git a/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.test.ts b/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.test.ts index 7d1937f56..b6fd92706 100644 --- a/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.test.ts +++ b/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.test.ts @@ -2,9 +2,9 @@ import { createSubjectMock } from '../../../__mocks__/notifications-mocks'; import { repositoryDependabotAlertsThreadHandler } from './repositoryDependabotAlertsThread'; describe('renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.ts', () => { - it('getIcon', () => { + it('iconType', () => { expect( - repositoryDependabotAlertsThreadHandler.getIcon( + repositoryDependabotAlertsThreadHandler.iconType( createSubjectMock({ type: 'RepositoryDependabotAlertsThread', }), diff --git a/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.ts b/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.ts index 98b386b63..cd06771be 100644 --- a/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.ts +++ b/src/renderer/utils/notifications/handlers/repositoryDependabotAlertsThread.ts @@ -9,7 +9,7 @@ import { DefaultHandler } from './default'; class RepositoryDependabotAlertsThreadHandler extends DefaultHandler { readonly type = 'RepositoryDependabotAlertsThread'; - getIcon(_subject: Subject): FC | null { + iconType(_subject: Subject): FC | null { return AlertIcon; } } diff --git a/src/renderer/utils/notifications/handlers/repositoryInvitation.test.ts b/src/renderer/utils/notifications/handlers/repositoryInvitation.test.ts index 30d52ee2b..69062a277 100644 --- a/src/renderer/utils/notifications/handlers/repositoryInvitation.test.ts +++ b/src/renderer/utils/notifications/handlers/repositoryInvitation.test.ts @@ -2,9 +2,9 @@ import { createSubjectMock } from '../../../__mocks__/notifications-mocks'; import { repositoryInvitationHandler } from './repositoryInvitation'; describe('renderer/utils/notifications/handlers/repositoryInvitation.ts', () => { - it('getIcon', () => { + it('iconType', () => { expect( - repositoryInvitationHandler.getIcon( + repositoryInvitationHandler.iconType( createSubjectMock({ type: 'RepositoryInvitation', }), diff --git a/src/renderer/utils/notifications/handlers/repositoryInvitation.ts b/src/renderer/utils/notifications/handlers/repositoryInvitation.ts index 35f46bd73..38bf30470 100644 --- a/src/renderer/utils/notifications/handlers/repositoryInvitation.ts +++ b/src/renderer/utils/notifications/handlers/repositoryInvitation.ts @@ -8,7 +8,7 @@ import { DefaultHandler } from './default'; class RepositoryInvitationHandler extends DefaultHandler { readonly type = 'RepositoryInvitation'; - getIcon(_subject: Subject): FC | null { + iconType(_subject: Subject): FC | null { return MailIcon; } } diff --git a/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.test.ts b/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.test.ts index d1a88b395..f6055ed3d 100644 --- a/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.test.ts +++ b/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.test.ts @@ -2,9 +2,9 @@ import { createSubjectMock } from '../../../__mocks__/notifications-mocks'; import { repositoryVulnerabilityAlertHandler } from './repositoryVulnerabilityAlert'; describe('renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.ts', () => { - it('getIcon', () => { + it('iconType', () => { expect( - repositoryVulnerabilityAlertHandler.getIcon( + repositoryVulnerabilityAlertHandler.iconType( createSubjectMock({ type: 'RepositoryVulnerabilityAlert', }), diff --git a/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.ts b/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.ts index fe679ca82..f3b8e77e7 100644 --- a/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.ts +++ b/src/renderer/utils/notifications/handlers/repositoryVulnerabilityAlert.ts @@ -8,7 +8,7 @@ import { DefaultHandler } from './default'; class RepositoryVulnerabilityAlertHandler extends DefaultHandler { readonly type = 'RepositoryVulnerabilityAlert'; - getIcon(_subject: Subject): FC | null { + iconType(_subject: Subject): FC | null { return AlertIcon; } } diff --git a/src/renderer/utils/notifications/handlers/types.ts b/src/renderer/utils/notifications/handlers/types.ts index 436d548e2..686e3ef50 100644 --- a/src/renderer/utils/notifications/handlers/types.ts +++ b/src/renderer/utils/notifications/handlers/types.ts @@ -21,9 +21,28 @@ export interface NotificationTypeHandler { settings: SettingsState, ): Promise; - /** Return an icon component for this notification type. */ - getIcon(subject: Subject): FC | null; + /** + * Return the icon component for this notification type. + */ + iconType(subject: Subject): FC | null; - /** Return a color for this notification type. */ - getIconColor(subject: Subject): string | undefined; + /** + * Return the icon color for this notification type. + */ + iconColor(subject: Subject): string | undefined; + + /** + * Return the formatted notification type for this notification. + */ + formattedNotificationType(notification: Notification): string; + + /** + * Return the formatted notification number for this notification. + */ + formattedNotificationNumber(notification: Notification): string; + + /** + * Return the formatted notification title for this notification. + */ + formattedNotificationTitle(notification: Notification): string; } diff --git a/src/renderer/utils/notifications/handlers/workflowRun.test.ts b/src/renderer/utils/notifications/handlers/workflowRun.test.ts index 000d4ccb1..5d843cf1d 100644 --- a/src/renderer/utils/notifications/handlers/workflowRun.test.ts +++ b/src/renderer/utils/notifications/handlers/workflowRun.test.ts @@ -52,9 +52,9 @@ describe('renderer/utils/notifications/handlers/workflowRun.ts', () => { }); }); - it('getIcon', () => { + it('iconType', () => { expect( - workflowRunHandler.getIcon( + workflowRunHandler.iconType( createSubjectMock({ type: 'WorkflowRun', }), diff --git a/src/renderer/utils/notifications/handlers/workflowRun.ts b/src/renderer/utils/notifications/handlers/workflowRun.ts index ab242450f..9dfd7df2e 100644 --- a/src/renderer/utils/notifications/handlers/workflowRun.ts +++ b/src/renderer/utils/notifications/handlers/workflowRun.ts @@ -32,7 +32,7 @@ class WorkflowRunHandler extends DefaultHandler { return null; } - getIcon(_subject: Subject): FC | null { + iconType(_subject: Subject): FC | null { return RocketIcon; } } From ca25646baf593364f8685e00c1b98706a2b62203 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 19 Aug 2025 11:13:06 -0400 Subject: [PATCH 3/4] refactor: handler enhancements Signed-off-by: Adam Setch --- .../notifications/NotificationRow.tsx | 1 - src/renderer/utils/helpers.test.ts | 17 ----------------- src/renderer/utils/helpers.ts | 15 --------------- .../utils/notifications/handlers/default.ts | 2 +- .../utils/notifications/handlers/utils.test.ts | 14 +++++++++++++- .../utils/notifications/handlers/utils.ts | 15 +++++++++++++++ 6 files changed, 29 insertions(+), 35 deletions(-) diff --git a/src/renderer/components/notifications/NotificationRow.tsx b/src/renderer/components/notifications/NotificationRow.tsx index ef90c0466..dda3f6547 100644 --- a/src/renderer/components/notifications/NotificationRow.tsx +++ b/src/renderer/components/notifications/NotificationRow.tsx @@ -70,7 +70,6 @@ export const NotificationRow: FC = ({ }; const handler = createNotificationHandler(notification); - const NotificationIcon = handler.iconType(notification.subject); const iconColor = handler.iconColor(notification.subject); const notificationType = handler.formattedNotificationType(notification); diff --git a/src/renderer/utils/helpers.test.ts b/src/renderer/utils/helpers.test.ts index be06b4142..c65f93121 100644 --- a/src/renderer/utils/helpers.test.ts +++ b/src/renderer/utils/helpers.test.ts @@ -16,7 +16,6 @@ import { } from './api/__mocks__/response-mocks'; import * as apiRequests from './api/request'; import { - formatForDisplay, generateGitHubWebUrl, generateNotificationReferrerId, getChevronDetails, @@ -517,22 +516,6 @@ describe('renderer/utils/helpers.ts', () => { }); }); - describe('formatting', () => { - it('formatForDisplay', () => { - expect(formatForDisplay(null)).toBe(''); - expect(formatForDisplay([])).toBe(''); - expect(formatForDisplay(['open', 'PullRequest'])).toBe( - 'Open Pull Request', - ); - expect(formatForDisplay(['OUTDATED', 'Discussion'])).toBe( - 'Outdated Discussion', - ); - expect(formatForDisplay(['not_planned', 'Issue'])).toBe( - 'Not Planned Issue', - ); - }); - }); - describe('getChevronDetails', () => { it('should return correct chevron details', () => { expect(getChevronDetails(true, true, 'account')).toEqual({ diff --git a/src/renderer/utils/helpers.ts b/src/renderer/utils/helpers.ts index 27e932ab5..24d47c7dc 100644 --- a/src/renderer/utils/helpers.ts +++ b/src/renderer/utils/helpers.ts @@ -171,21 +171,6 @@ export async function generateGitHubWebUrl( return url.toString() as Link; } -export function formatForDisplay(text: string[]): string { - if (!text) { - return ''; - } - - return text - .join(' ') - .replace(/([a-z])([A-Z])/g, '$1 $2') // Add space between lowercase character followed by an uppercase character - .replace(/_/g, ' ') // Replace underscores with spaces - .replace(/\w+/g, (word) => { - // Convert to proper case (capitalize first letter of each word) - return word.charAt(0).toUpperCase() + word.slice(1).toLowerCase(); - }); -} - export function getChevronDetails( hasNotifications: boolean, isVisible: boolean, diff --git a/src/renderer/utils/notifications/handlers/default.ts b/src/renderer/utils/notifications/handlers/default.ts index f1634e3ee..99f66b474 100644 --- a/src/renderer/utils/notifications/handlers/default.ts +++ b/src/renderer/utils/notifications/handlers/default.ts @@ -11,8 +11,8 @@ import type { Subject, SubjectType, } from '../../../typesGitHub'; -import { formatForDisplay } from '../../helpers'; import type { NotificationTypeHandler } from './types'; +import { formatForDisplay } from './utils'; export class DefaultHandler implements NotificationTypeHandler { type?: SubjectType; diff --git a/src/renderer/utils/notifications/handlers/utils.test.ts b/src/renderer/utils/notifications/handlers/utils.test.ts index 9863cc4a5..9437f40fa 100644 --- a/src/renderer/utils/notifications/handlers/utils.test.ts +++ b/src/renderer/utils/notifications/handlers/utils.test.ts @@ -1,5 +1,5 @@ import { partialMockUser } from '../../../__mocks__/partial-mocks'; -import { getSubjectUser } from './utils'; +import { formatForDisplay, getSubjectUser } from './utils'; describe('renderer/utils/notifications/handlers/utils.ts', () => { describe('getSubjectUser', () => { @@ -33,4 +33,16 @@ describe('renderer/utils/notifications/handlers/utils.ts', () => { }); }); }); + + it('formatForDisplay', () => { + expect(formatForDisplay(null)).toBe(''); + expect(formatForDisplay([])).toBe(''); + expect(formatForDisplay(['open', 'PullRequest'])).toBe('Open Pull Request'); + expect(formatForDisplay(['OUTDATED', 'Discussion'])).toBe( + 'Outdated Discussion', + ); + expect(formatForDisplay(['not_planned', 'Issue'])).toBe( + 'Not Planned Issue', + ); + }); }); diff --git a/src/renderer/utils/notifications/handlers/utils.ts b/src/renderer/utils/notifications/handlers/utils.ts index 5bee31fe5..f6df9e44e 100644 --- a/src/renderer/utils/notifications/handlers/utils.ts +++ b/src/renderer/utils/notifications/handlers/utils.ts @@ -23,3 +23,18 @@ export function getSubjectUser(users: User[]): SubjectUser { return subjectUser; } + +export function formatForDisplay(text: string[]): string { + if (!text) { + return ''; + } + + return text + .join(' ') + .replace(/([a-z])([A-Z])/g, '$1 $2') // Add space between lowercase character followed by an uppercase character + .replace(/_/g, ' ') // Replace underscores with spaces + .replace(/\w+/g, (word) => { + // Convert to proper case (capitalize first letter of each word) + return word.charAt(0).toUpperCase() + word.slice(1).toLowerCase(); + }); +} From 41d3c5623125ff3f8aa6feadf99cb9e8390c6ccd Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 19 Aug 2025 11:22:27 -0400 Subject: [PATCH 4/4] refactor: handler enhancements Signed-off-by: Adam Setch --- .../AccountNotifications.test.tsx.snap | 4 +- .../notifications/handlers/default.test.ts | 74 +++++++++++++++++++ .../utils/notifications/handlers/utils.ts | 3 +- 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap index e1e7e0ca6..c2dbf27b6 100644 --- a/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap @@ -1100,7 +1100,7 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > { expect(defaultHandler.iconColor(subject)).toBe(expected); }); }); + + describe('formattedNotificationType', () => { + it('formats state and type with proper casing and spacing', () => { + const notification = partialMockNotification({ + title: 'Sample', + type: 'PullRequest', + state: 'open', + }); + + expect(defaultHandler.formattedNotificationType(notification)).toBe( + 'Open Pull Request', + ); + }); + + it('handles missing state (null) gracefully', () => { + const notification = partialMockNotification({ + title: 'Sample', + type: 'Issue', + state: null, + }); + + expect(defaultHandler.formattedNotificationType(notification)).toBe( + 'Issue', + ); + }); + }); + + describe('formattedNotificationNumber', () => { + it('returns formatted number when present', () => { + const notification = partialMockNotification({ + title: 'Sample', + type: 'Issue', + state: 'open', + }); + notification.subject.number = 42; + expect(defaultHandler.formattedNotificationNumber(notification)).toBe( + '#42', + ); + }); + + it('returns empty string when number absent', () => { + const notification = partialMockNotification({ + title: 'Sample', + type: 'Issue', + state: 'open', + }); + expect(defaultHandler.formattedNotificationNumber(notification)).toBe(''); + }); + }); + + describe('formattedNotificationTitle', () => { + it('appends number in brackets when present', () => { + const notification = partialMockNotification({ + title: 'Fix bug', + type: 'Issue', + state: 'open', + }); + notification.subject.number = 101; + expect(defaultHandler.formattedNotificationTitle(notification)).toBe( + 'Fix bug [#101]', + ); + }); + + it('returns title unchanged when number missing', () => { + const notification = partialMockNotification({ + title: 'Improve docs', + type: 'Issue', + state: 'open', + }); + expect(defaultHandler.formattedNotificationTitle(notification)).toBe( + 'Improve docs', + ); + }); + }); }); diff --git a/src/renderer/utils/notifications/handlers/utils.ts b/src/renderer/utils/notifications/handlers/utils.ts index f6df9e44e..cb27e3b77 100644 --- a/src/renderer/utils/notifications/handlers/utils.ts +++ b/src/renderer/utils/notifications/handlers/utils.ts @@ -36,5 +36,6 @@ export function formatForDisplay(text: string[]): string { .replace(/\w+/g, (word) => { // Convert to proper case (capitalize first letter of each word) return word.charAt(0).toUpperCase() + word.slice(1).toLowerCase(); - }); + }) + .trim(); }