Skip to content

Commit

Permalink
fix: notification services controller missing env vars (#4530)
Browse files Browse the repository at this point in the history
## Explanation

If platforms try to use this controller but pass invalid environment
variables, we will make unnecessary calls to fetch feature announcements
that will fail. (This mostly happens in Extension, where some
controllers are in JS and do not provide type errors).

NOTE - that a new service is being developed to deliver feature
announcements, which might allow us to not require platforms to pass in
an environment.

## References

[NOTIFY-848](https://consensyssoftware.atlassian.net/browse/NOTIFY-848)

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/notification-services-controller`

- **CHANGED**: Updated notification mocks to use realistic tx hashes
- **FIXED**: Logic to check feature announcement environment before
fetching

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
  • Loading branch information
Prithpal-Sooriya authored Jul 18, 2024
1 parent f0530bb commit 7969752
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ export function createMockNotificationEthSent(): OnChainRawNotification {
chain_id: 1,
block_number: 17485840,
block_timestamp: '2022-03-01T00:00:00Z',
tx_hash: '0x881D40237659C251811CEC9c364ef91dC08D300C',
tx_hash:
'0xb2256b183f2fb3872f99294ab55fb03e6a479b0d4aca556a3b27568b712505a6',
unread: true,
created_at: '2022-03-01T00:00:00Z',
address: '0x881D40237659C251811CEC9c364ef91dC08D300C',
Expand Down Expand Up @@ -48,7 +49,8 @@ export function createMockNotificationEthReceived(): OnChainRawNotification {
chain_id: 1,
block_number: 17485840,
block_timestamp: '2022-03-01T00:00:00Z',
tx_hash: '0x881D40237659C251811CEC9c364ef91dC08D300C',
tx_hash:
'0xb2256b183f2fb3872f99294ab55fb03e6a479b0d4aca556a3b27568b712505a6',
unread: true,
created_at: '2022-03-01T00:00:00Z',
address: '0x881D40237659C251811CEC9c364ef91dC08D300C',
Expand Down Expand Up @@ -82,7 +84,8 @@ export function createMockNotificationERC20Sent(): OnChainRawNotification {
chain_id: 1,
block_number: 17485840,
block_timestamp: '2022-03-01T00:00:00Z',
tx_hash: '0x881D40237659C251811CEC9c364ef91dC08D300C',
tx_hash:
'0xb2256b183f2fb3872f99294ab55fb03e6a479b0d4aca556a3b27568b712505a6',
unread: true,
created_at: '2022-03-01T00:00:00Z',
address: '0x881D40237659C251811CEC9c364ef91dC08D300C',
Expand Down Expand Up @@ -122,7 +125,8 @@ export function createMockNotificationERC20Received(): OnChainRawNotification {
chain_id: 1,
block_number: 17485840,
block_timestamp: '2022-03-01T00:00:00Z',
tx_hash: '0x881D40237659C251811CEC9c364ef91dC08D300C',
tx_hash:
'0xb2256b183f2fb3872f99294ab55fb03e6a479b0d4aca556a3b27568b712505a6',
unread: true,
created_at: '2022-03-01T00:00:00Z',
address: '0x881D40237659C251811CEC9c364ef91dC08D300C',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { mockFetchFeatureAnnouncementNotifications } from '../__fixtures__/mockS
import { TRIGGER_TYPES } from '../constants/notification-schema';
import { getFeatureAnnouncementNotifications } from './feature-announcements';

// Mocked type for testing, allows overwriting TS to test erroneous values
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type MockedType = any;

jest.mock('@contentful/rich-text-html-renderer', () => ({
documentToHtmlString: jest
.fn()
Expand All @@ -20,6 +24,27 @@ describe('Feature Announcement Notifications', () => {
jest.clearAllMocks();
});

it('should return an empty array if invalid environment provided', async () => {
mockFetchFeatureAnnouncementNotifications();

const assertEnvEmpty = async (
override: Partial<typeof featureAnnouncementsEnv>,
) => {
const result = await getFeatureAnnouncementNotifications({
...featureAnnouncementsEnv,
...override,
});
expect(result).toHaveLength(0);
};

await assertEnvEmpty({ accessToken: null as MockedType });
await assertEnvEmpty({ platform: null as MockedType });
await assertEnvEmpty({ spaceId: null as MockedType });
await assertEnvEmpty({ accessToken: '' });
await assertEnvEmpty({ platform: '' });
await assertEnvEmpty({ spaceId: '' });
});

it('should return an empty array if fetch fails', async () => {
const mockEndpoint = mockFetchFeatureAnnouncementNotifications({
status: 500,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,14 @@ const fetchFeatureAnnouncementNotifications = async (
export async function getFeatureAnnouncementNotifications(
env: Env,
): Promise<INotification[]> {
const rawNotifications = await fetchFeatureAnnouncementNotifications(env);
const notifications = rawNotifications.map((notification) =>
processFeatureAnnouncement(notification),
);
if (env?.accessToken && env?.spaceId && env?.platform) {
const rawNotifications = await fetchFeatureAnnouncementNotifications(env);
const notifications = rawNotifications.map((notification) =>
processFeatureAnnouncement(notification),
);

return notifications;
}

return notifications;
return [];
}

0 comments on commit 7969752

Please sign in to comment.