Skip to content

Commit

Permalink
fix: ensure notification hooks are not called when flag is disabled
Browse files Browse the repository at this point in the history
  • Loading branch information
Prithpal-Sooriya committed Nov 22, 2024
1 parent df8e3b9 commit 01a1859
Show file tree
Hide file tree
Showing 9 changed files with 119 additions and 25 deletions.
7 changes: 7 additions & 0 deletions app/util/notifications/hooks/useCreateSession.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ import * as Selectors from '../../../selectors/notifications';
import * as Actions from '../../../actions/notification/helpers';
import useCreateSession from './useCreateSession';

jest.mock('../constants', () => {
return {
...jest.requireActual('../constants'),
isNotificationsFeatureEnabled: () => true,
};
});

function arrangeStore() {
const store = createMockStore()(initialRootState);

Expand Down
5 changes: 5 additions & 0 deletions app/util/notifications/hooks/useCreateSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
disableProfileSyncing,
signIn,
} from '../../../actions/notification/helpers';
import { isNotificationsFeatureEnabled } from '../constants';

/**
* Custom hook to manage the creation of a session based on the user's authentication status,
Expand All @@ -31,6 +32,10 @@ function useCreateSession(): UseCreateSessionReturn {
const [error, setError] = useState<string | undefined>(undefined);

const createSession = useCallback(async () => {
if (!isNotificationsFeatureEnabled()) {
return;
}

// If the user is already signed in, no need to create a new session
if (isSignedIn) {
return;
Expand Down
7 changes: 7 additions & 0 deletions app/util/notifications/hooks/useNotifications.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ import {
createMockNotificationEthSent,
} from '../../../components/UI/Notification/__mocks__/mock_notifications';

jest.mock('../constants', () => {
return {
...jest.requireActual('../constants'),
isNotificationsFeatureEnabled: () => true,
};
});

function arrangeStore() {
const store = createMockStore()(initialRootState);

Expand Down
40 changes: 36 additions & 4 deletions app/util/notifications/hooks/useNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import {
} from '../../../actions/notification/helpers';
import { getNotificationsList } from '../../../selectors/notifications';
import { usePushNotifications } from './usePushNotifications';
import { isNotificationsFeatureEnabled } from '../constants';

/**
* Custom hook to fetch and update the list of notifications.
* Manages loading and error states internally.
Expand All @@ -33,6 +35,10 @@ export function useListNotifications(): ListNotificationsReturn {
const [error, setError] = useState<string>();

const listNotifications = useCallback(async () => {
if (!isNotificationsFeatureEnabled()) {
return;
}

setLoading(true);
setError(undefined);
try {
Expand Down Expand Up @@ -68,6 +74,10 @@ export function useCreateNotifications(): CreateNotificationsReturn {
const [error, setError] = useState<string>();

const createNotifications = useCallback(async (accounts: string[]) => {
if (!isNotificationsFeatureEnabled()) {
return;
}

setLoading(true);
setError(undefined);
try {
Expand Down Expand Up @@ -106,12 +116,19 @@ export function useEnableNotifications(): EnableNotificationsReturn {
const [error, setError] = useState<string>();
const { switchPushNotifications } = usePushNotifications();
const enableNotifications = useCallback(async () => {
if (!isNotificationsFeatureEnabled()) {
return;
}

setLoading(true);
setError(undefined);
try {
const errorEnablingNotifications = await enableNotificationServices();
const errorEnablingPushNotifications = await switchPushNotifications(true);
const errorMessage = errorEnablingNotifications || errorEnablingPushNotifications;
const errorEnablingPushNotifications = await switchPushNotifications(
true,
);
const errorMessage =
errorEnablingNotifications || errorEnablingPushNotifications;

if (errorMessage) {
setError(getErrorMessage(errorMessage));
Expand Down Expand Up @@ -143,12 +160,19 @@ export function useDisableNotifications(): DisableNotificationsReturn {
const [error, setError] = useState<string>();
const { switchPushNotifications } = usePushNotifications();
const disableNotifications = useCallback(async () => {
if (!isNotificationsFeatureEnabled()) {
return;
}

setLoading(true);
setError(undefined);
try {
const errorDisablingNotifications = await disableNotificationServices();
const errorDisablingPushNotifications = await switchPushNotifications(false);
const errorMessage = errorDisablingNotifications || errorDisablingPushNotifications;
const errorDisablingPushNotifications = await switchPushNotifications(
false,
);
const errorMessage =
errorDisablingNotifications || errorDisablingPushNotifications;

if (errorMessage) {
setError(getErrorMessage(errorMessage));
Expand Down Expand Up @@ -182,6 +206,10 @@ export function useMarkNotificationAsRead(): MarkNotificationAsReadReturn {

const markNotificationAsRead = useCallback(
async (notifications: MarkAsReadNotificationsParam) => {
if (!isNotificationsFeatureEnabled()) {
return;
}

setLoading(true);
setError(undefined);
try {
Expand Down Expand Up @@ -221,6 +249,10 @@ export function useDeleteNotificationsStorageKey(): deleteNotificationsStorageKe
const [error, setError] = useState<string>();

const deleteNotificationsStorageKey = useCallback(async () => {
if (!isNotificationsFeatureEnabled()) {
return;
}

setLoading(true);
setError(undefined);
try {
Expand Down
7 changes: 7 additions & 0 deletions app/util/notifications/hooks/useProfileSyncing.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ import * as Actions from '../../../actions/notification/helpers';
import initialRootState from '../../../util/test/initial-root-state';
import { useProfileSyncing } from './useProfileSyncing';

jest.mock('../constants', () => {
return {
...jest.requireActual('../constants'),
isNotificationsFeatureEnabled: () => true,
};
});

function arrangeStore() {
const store = createMockStore()(initialRootState);

Expand Down
9 changes: 9 additions & 0 deletions app/util/notifications/hooks/useProfileSyncing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
disableProfileSyncing as disableProfileSyncingAction,
enableProfileSyncing as enableProfileSyncingAction,
} from '../../../actions/notification/helpers';
import { isNotificationsFeatureEnabled } from '../constants';

/**
* Custom hook to enable profile syncing. This hook handles the process of signing in
Expand All @@ -17,6 +18,10 @@ export function useProfileSyncing(): ProfileSyncingReturn {
const [error, setError] = useState<string>();

const enableProfileSyncing = useCallback(async () => {
if (!isNotificationsFeatureEnabled()) {
return;
}

setLoading(true);
setError(undefined);
try {
Expand All @@ -36,6 +41,10 @@ export function useProfileSyncing(): ProfileSyncingReturn {
}, []);

const disableProfileSyncing = useCallback(async () => {
if (!isNotificationsFeatureEnabled()) {
return;
}

setLoading(true);
setError(undefined);
try {
Expand Down
12 changes: 9 additions & 3 deletions app/util/notifications/hooks/usePushNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from '../../../actions/notification/helpers';
import { mmStorage } from '../settings';
import { UserStorage } from '@metamask/notification-services-controller/dist/NotificationServicesController/types/user-storage/index.cjs';

import { isNotificationsFeatureEnabled } from '../constants';

export function usePushNotifications() {
const [loading, setLoading] = useState<boolean>(false);
Expand All @@ -18,6 +18,10 @@ export function usePushNotifications() {

const switchPushNotifications = useCallback(
async (state: boolean) => {
if (!isNotificationsFeatureEnabled()) {
return;
}

resetStates();
setLoading(true);
let errorMessage: string | undefined;
Expand All @@ -26,7 +30,10 @@ export function usePushNotifications() {
const userStorage: UserStorage = mmStorage.getLocal('pnUserStorage');
if (state) {
const fcmToken = mmStorage.getLocal('metaMaskFcmToken');
errorMessage = await enablePushNotifications(userStorage, fcmToken?.data);
errorMessage = await enablePushNotifications(
userStorage,
fcmToken?.data,
);
} else {
errorMessage = await disablePushNotifications(userStorage);
}
Expand All @@ -43,7 +50,6 @@ export function usePushNotifications() {
[resetStates],
);


return {
switchPushNotifications,
loading,
Expand Down
28 changes: 19 additions & 9 deletions app/util/notifications/hooks/useSwitchNotifications.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ import { Hex } from '@metamask/utils';
import { KeyringTypes } from '@metamask/keyring-controller';
import Engine from '../../../core/Engine';

jest.mock('../constants', () => {
return {
...jest.requireActual('../constants'),
isNotificationsFeatureEnabled: () => true,
};
});

jest.mock('../../../core/Engine', () => ({
context: {
NotificationServicesController: {
Expand Down Expand Up @@ -170,15 +177,13 @@ describe('useAccountSettingsProps', () => {
.spyOn(Selectors, 'selectIsUpdatingMetamaskNotificationsAccount')
.mockReturnValue([MOCK_ACCOUNTS[0].address]);

const isMetamaskNotificationsEnabled = jest
.spyOn(Selectors,
'selectIsMetamaskNotificationsEnabled',
)
const isMetamaskNotificationsEnabled = jest
.spyOn(Selectors, 'selectIsMetamaskNotificationsEnabled')
.mockReturnValue(true);

return {
selectIsUpdatingMetamaskNotificationsAccount,
isMetamaskNotificationsEnabled
isMetamaskNotificationsEnabled,
};
}

Expand All @@ -189,9 +194,12 @@ describe('useAccountSettingsProps', () => {
[MOCK_ACCOUNTS[1].address]: false,
});

Engine.context.NotificationServicesController.checkAccountsPresence = mockCheckAccountsPresence;
Engine.context.NotificationServicesController.checkAccountsPresence =
mockCheckAccountsPresence;

mockSelectors.selectIsUpdatingMetamaskNotificationsAccount.mockReturnValue([]);
mockSelectors.selectIsUpdatingMetamaskNotificationsAccount.mockReturnValue(
[],
);
mockSelectors.isMetamaskNotificationsEnabled.mockReturnValue(true);

const { hook, store } = arrangeHook(MOCK_ACCOUNTS);
Expand All @@ -200,13 +208,15 @@ describe('useAccountSettingsProps', () => {
await hook.result.current.updateAndfetchAccountSettings();
});

expect(mockCheckAccountsPresence).toHaveBeenCalledWith(MOCK_ACCOUNTS.map(account => account.address));
expect(mockCheckAccountsPresence).toHaveBeenCalledWith(
MOCK_ACCOUNTS.map((account) => account.address),
);

expect(store.dispatch).toHaveBeenCalledWith(
updateAccountState({
[MOCK_ACCOUNTS[0].address]: true,
[MOCK_ACCOUNTS[1].address]: false,
})
}),
);
});
});
29 changes: 20 additions & 9 deletions app/util/notifications/hooks/useSwitchNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { useDispatch } from 'react-redux';
import { updateAccountState } from '../../../core/redux/slices/notifications';
import { Account } from '../../../components/hooks/useAccounts/useAccounts.types';
import Logger from '../../../util/Logger';
import { isNotificationsFeatureEnabled } from '../constants';

export function useSwitchNotifications() {
const [loading, setLoading] = useState<boolean>(false);
Expand All @@ -22,6 +23,10 @@ export function useSwitchNotifications() {

const switchFeatureAnnouncements = useCallback(
async (state: boolean) => {
if (!isNotificationsFeatureEnabled()) {
return;
}

resetStates();
setLoading(true);

Expand Down Expand Up @@ -87,16 +92,22 @@ export function useAccountSettingsProps(accounts: Account[]) {
const dispatch = useDispatch();

// Memoize the accounts array to avoid unnecessary re-fetching
const memoAccounts = useMemo(() => accounts.map((account) => account.address),[accounts]);
const memoAccounts = useMemo(
() => accounts.map((account) => account.address),
[accounts],
);
const updateAndfetchAccountSettings = useCallback(async () => {
try {
const result = await Engine.context.NotificationServicesController.checkAccountsPresence(memoAccounts);
dispatch(updateAccountState(result));
return result;
} catch (err) {
Logger.log('Failed to get account settings:', err);
}
}, [dispatch, memoAccounts]);
try {
const result =
await Engine.context.NotificationServicesController.checkAccountsPresence(
memoAccounts,
);
dispatch(updateAccountState(result));
return result;
} catch (err) {
Logger.log('Failed to get account settings:', err);
}
}, [dispatch, memoAccounts]);

return { updateAndfetchAccountSettings };
}

0 comments on commit 01a1859

Please sign in to comment.