Skip to content

Commit 2a3a2eb

Browse files
chore: Notification services eslint cleanup (#7485)
## Explanation This PR addresses and resolves several ESLint rule violations across three files within `packages/notification-services-controller`, allowing for the removal of their corresponding suppressions from `eslint-suppressions.json`. * **Current state and why change?** ESLint suppressions were present in `eslint-suppressions.json` for rules such as `@typescript-eslint/explicit-function-return-type`, `@typescript-eslint/naming-convention`, `@typescript-eslint/prefer-nullish-coalescing`, and `id-length`. These suppressions indicated areas where the code did not conform to established linting standards. The goal is to improve code quality and maintainability by fixing these violations and removing the suppressions. * **What is the solution your changes offer and how does it work?** * **`feature-announcements.ts`**: * Explicit return types were added to `getFeatureAnnouncementUrl` and `findIncludedItem`. * `prefer-nullish-coalescing` was applied by replacing `||` with `??`. * `id-length` violations were resolved by renaming short, non-descriptive variables (`r`, `i`, `n`) to more explicit names (`response`, `entry`/`asset`, `item`/`rawNotification`). * `@typescript-eslint/naming-convention` suppressions were added with explanations for `Entry` and `Asset` properties within the `ContentfulResult` type, as their casing is dictated by the external Contentful API response structure. * **`notification-config-cache.ts`**: * The private field `#TTL` was renamed to `#ttl` to conform to camelCase naming conventions for private class members. * **`perp-notifications.test.ts`**: * An explicit return type was added to the `arrangeMocks` test helper function. * All corresponding entries for these files were removed from `eslint-suppressions.json`. * **Are there any changes whose purpose might not be obvious?** The `eslint-disable-next-line @typescript-eslint/naming-convention` comments in `feature-announcements.ts` for `Entry` and `Asset` properties are intentional. These properties directly reflect the structure of responses from the Contentful API, and renaming them would break compatibility or require complex mapping. ## References * Fixes ASSETS-2100 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them --- <a href="https://cursor.com/background-agent?bcId=bc-f2af14fa-6392-4945-9c74-67168ae9e6e0"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-cursor-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-cursor-light.svg"><img alt="Open in Cursor" src="https://cursor.com/open-in-cursor.svg"></picture></a>&nbsp;<a href="https://cursor.com/agents?id=bc-f2af14fa-6392-4945-9c74-67168ae9e6e0"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-web-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-web-light.svg"><img alt="Open in Web" src="https://cursor.com/open-in-web.svg"></picture></a> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Fixes ESLint violations in feature announcements, notification config cache, and perps test, and removes their suppressions. > > - **Notification Services Controller**: > - `services/feature-announcements.ts`: > - Add explicit return types to `getFeatureAnnouncementUrl` and `findIncludedItem`. > - Replace `||` with `??` and improve variable names (`response`, `entry`/`asset`, `item`, `rawNotification`). > - Add targeted `@typescript-eslint/naming-convention` disables for `includes.Entry` and `includes.Asset` to match Contentful schema. > - Minor refactors: safer optional access, typed mappings, and consistent `createdAt` source. > - `services/notification-config-cache.ts`: > - Rename private field `#TTL` to `#ttl` and update references. > - `services/perp-notifications.test.ts`: > - Add explicit return type to `arrangeMocks` helper. > - **ESLint suppressions**: > - Remove entries for `feature-announcements.ts`, `notification-config-cache.ts`, and `perp-notifications.test.ts` from `eslint-suppressions.json`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e6f1d91. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Cursor Agent <[email protected]>
1 parent 32dd105 commit 2a3a2eb

File tree

4 files changed

+43
-45
lines changed

4 files changed

+43
-45
lines changed

eslint-suppressions.json

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,30 +1575,6 @@
15751575
"count": 1
15761576
}
15771577
},
1578-
"packages/notification-services-controller/src/NotificationServicesController/services/feature-announcements.ts": {
1579-
"@typescript-eslint/explicit-function-return-type": {
1580-
"count": 2
1581-
},
1582-
"@typescript-eslint/naming-convention": {
1583-
"count": 2
1584-
},
1585-
"@typescript-eslint/prefer-nullish-coalescing": {
1586-
"count": 2
1587-
},
1588-
"id-length": {
1589-
"count": 3
1590-
}
1591-
},
1592-
"packages/notification-services-controller/src/NotificationServicesController/services/notification-config-cache.ts": {
1593-
"@typescript-eslint/naming-convention": {
1594-
"count": 1
1595-
}
1596-
},
1597-
"packages/notification-services-controller/src/NotificationServicesController/services/perp-notifications.test.ts": {
1598-
"@typescript-eslint/explicit-function-return-type": {
1599-
"count": 1
1600-
}
1601-
},
16021578
"packages/notification-services-controller/src/NotificationServicesController/services/perp-notifications.ts": {
16031579
"@typescript-eslint/explicit-function-return-type": {
16041580
"count": 1

packages/notification-services-controller/src/NotificationServicesController/services/feature-announcements.ts

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,25 @@ type Env = {
3737
*/
3838
export type ContentfulResult = {
3939
includes?: {
40+
// Property names match Contentful API response structure
41+
// eslint-disable-next-line @typescript-eslint/naming-convention
4042
Entry?: Entry[];
43+
// eslint-disable-next-line @typescript-eslint/naming-convention
4144
Asset?: Asset[];
4245
};
4346
items?: TypeFeatureAnnouncement[];
4447
};
4548

46-
export const getFeatureAnnouncementUrl = (env: Env, previewToken?: string) => {
49+
export const getFeatureAnnouncementUrl = (
50+
env: Env,
51+
previewToken?: string,
52+
): string => {
4753
const domain = previewToken ? PREVIEW_DOMAIN : DEFAULT_DOMAIN;
4854
const replacedUrl = FEATURE_ANNOUNCEMENT_URL.replace(
4955
DEFAULT_SPACE_ID,
5056
env.spaceId,
5157
)
52-
.replace(DEFAULT_ACCESS_TOKEN, previewToken || env.accessToken)
58+
.replace(DEFAULT_ACCESS_TOKEN, previewToken ?? env.accessToken)
5359
.replace(DEFAULT_CLIENT_ID, env.platform)
5460
.replace(DEFAULT_DOMAIN, domain);
5561
return encodeURI(replacedUrl);
@@ -62,14 +68,22 @@ const fetchFeatureAnnouncementNotifications = async (
6268
const url = getFeatureAnnouncementUrl(env, previewToken);
6369

6470
const data = await fetch(url)
65-
.then((r) => r.json())
71+
.then((response) => response.json())
6672
.catch(() => null);
6773

6874
if (!data) {
6975
return [];
7076
}
7177

72-
const findIncludedItem = (sysId: string) => {
78+
const findIncludedItem = (
79+
sysId: string,
80+
):
81+
| ImageFields['fields']
82+
| TypeExtensionLinkFields['fields']
83+
| TypePortfolioLinkFields['fields']
84+
| TypeMobileLinkFields['fields']
85+
| TypeExternalLinkFields['fields']
86+
| null => {
7387
const typedData: EntryCollection<
7488
| ImageFields
7589
| TypeExtensionLinkFields
@@ -78,15 +92,19 @@ const fetchFeatureAnnouncementNotifications = async (
7892
| TypeExternalLinkFields
7993
> = data;
8094
const item =
81-
typedData?.includes?.Entry?.find((i: Entry) => i?.sys?.id === sysId) ||
82-
typedData?.includes?.Asset?.find((i: Asset) => i?.sys?.id === sysId);
95+
typedData?.includes?.Entry?.find(
96+
(entry: Entry) => entry?.sys?.id === sysId,
97+
) ??
98+
typedData?.includes?.Asset?.find(
99+
(asset: Asset) => asset?.sys?.id === sysId,
100+
);
83101
return item ? item?.fields : null;
84102
};
85103

86104
const contentfulNotifications = data?.items ?? [];
87105
const rawNotifications: FeatureAnnouncementRawNotification[] =
88-
contentfulNotifications.map((n: TypeFeatureAnnouncement) => {
89-
const { fields } = n;
106+
contentfulNotifications.map((item: TypeFeatureAnnouncement) => {
107+
const { fields } = item;
90108
const imageFields = fields.image
91109
? (findIncludedItem(fields.image.sys.id) as ImageFields['fields'])
92110
: undefined;
@@ -114,7 +132,7 @@ const fetchFeatureAnnouncementNotifications = async (
114132

115133
const notification: FeatureAnnouncementRawNotification = {
116134
type: TRIGGER_TYPES.FEATURES_ANNOUNCEMENT,
117-
createdAt: new Date(n.sys.createdAt).toString(),
135+
createdAt: new Date(item.sys.createdAt).toString(),
118136
data: {
119137
id: fields.id,
120138
category: fields.category,
@@ -163,15 +181,17 @@ const fetchFeatureAnnouncementNotifications = async (
163181
},
164182
} as const;
165183

166-
const filteredRawNotifications = rawNotifications.filter((n) => {
167-
const minVersion = n.data?.[versionKeys[env.platform].min];
168-
const maxVersion = n.data?.[versionKeys[env.platform].max];
169-
return isVersionInBounds({
170-
currentVersion: env.platformVersion,
171-
minVersion,
172-
maxVersion,
173-
});
174-
});
184+
const filteredRawNotifications = rawNotifications.filter(
185+
(rawNotification) => {
186+
const minVersion = rawNotification.data?.[versionKeys[env.platform].min];
187+
const maxVersion = rawNotification.data?.[versionKeys[env.platform].max];
188+
return isVersionInBounds({
189+
currentVersion: env.platformVersion,
190+
minVersion,
191+
maxVersion,
192+
});
193+
},
194+
);
175195

176196
return filteredRawNotifications;
177197
};

packages/notification-services-controller/src/NotificationServicesController/services/notification-config-cache.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ export const NotificationConfigCacheTTL = 1000 * 60; // 60 seconds
88
export class OnChainNotificationsCache {
99
#cache: NotificationConfigCache | null = null;
1010

11-
readonly #TTL = NotificationConfigCacheTTL;
11+
readonly #ttl = NotificationConfigCacheTTL;
1212

1313
#isExpired(): boolean {
14-
return !this.#cache || Date.now() - this.#cache.timestamp > this.#TTL;
14+
return !this.#cache || Date.now() - this.#cache.timestamp > this.#ttl;
1515
}
1616

1717
#hasAllAddresses(addresses: string[]): boolean {

packages/notification-services-controller/src/NotificationServicesController/services/perp-notifications.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ describe('Perps Service - createPerpOrderNotification', () => {
1414
jest.clearAllMocks();
1515
});
1616

17-
const arrangeMocks = () => {
17+
const arrangeMocks = (): {
18+
consoleErrorSpy: jest.SpyInstance<void, Parameters<typeof console.error>>;
19+
} => {
1820
const consoleErrorSpy = jest
1921
.spyOn(console, 'error')
2022
.mockImplementation(jest.fn());

0 commit comments

Comments
 (0)