Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: breaking selector due to missing controller state #12375

Merged
merged 8 commits into from
Nov 25, 2024
11 changes: 7 additions & 4 deletions app/selectors/notifications/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,21 @@ type AuthenticationState =
type UserStorageState = UserStorageController.UserStorageControllerState;

const selectAuthenticationControllerState = (state: RootState) =>
state?.engine?.backgroundState?.AuthenticationController;
state?.engine?.backgroundState?.AuthenticationController ??
AuthenticationController.defaultState;

const selectUserStorageControllerState = (state: RootState) =>
state?.engine?.backgroundState?.UserStorageController;
state?.engine?.backgroundState?.UserStorageController ??
UserStorageController.defaultState;
Prithpal-Sooriya marked this conversation as resolved.
Show resolved Hide resolved

const selectNotificationServicesControllerState = (state: RootState) =>
state?.engine?.backgroundState?.NotificationServicesController;
state?.engine?.backgroundState?.NotificationServicesController ??
NotificationServicesController.defaultState;

export const selectIsProfileSyncingEnabled = createSelector(
selectUserStorageControllerState,
(userStorageControllerState: UserStorageState) =>
userStorageControllerState.isProfileSyncingEnabled,
userStorageControllerState?.isProfileSyncingEnabled,
Prithpal-Sooriya marked this conversation as resolved.
Show resolved Hide resolved
);
export const selectIsProfileSyncingUpdateLoading = createSelector(
selectUserStorageControllerState,
Expand Down
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 Actions from '../../../actions/notification/helpers';
import useCreateSession from './useCreateSession';

jest.mock('../constants', () => {

Check failure on line 14 in app/util/notifications/hooks/useCreateSession.test.tsx

View workflow job for this annotation

GitHub Actions / scripts (lint)

Unexpected block statement surrounding arrow body; parenthesize the returned value and move it immediately after the `=>`
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;
}
Prithpal-Sooriya marked this conversation as resolved.
Show resolved Hide resolved

// 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 @@
createMockNotificationEthSent,
} from '../../../components/UI/Notification/__mocks__/mock_notifications';

jest.mock('../constants', () => {

Check failure on line 27 in app/util/notifications/hooks/useNotifications.test.tsx

View workflow job for this annotation

GitHub Actions / scripts (lint)

Unexpected block statement surrounding arrow body; parenthesize the returned value and move it immediately after the `=>`
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 initialRootState from '../../../util/test/initial-root-state';
import { useProfileSyncing } from './useProfileSyncing';

jest.mock('../constants', () => {

Check failure on line 14 in app/util/notifications/hooks/useProfileSyncing.test.tsx

View workflow job for this annotation

GitHub Actions / scripts (lint)

Unexpected block statement surrounding arrow body; parenthesize the returned value and move it immediately after the `=>`
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 { KeyringTypes } from '@metamask/keyring-controller';
import Engine from '../../../core/Engine';

jest.mock('../constants', () => {

Check failure on line 23 in app/util/notifications/hooks/useSwitchNotifications.test.tsx

View workflow job for this annotation

GitHub Actions / scripts (lint)

Unexpected block statement surrounding arrow body; parenthesize the returned value and move it immediately after the `=>`
return {
...jest.requireActual('../constants'),
isNotificationsFeatureEnabled: () => true,
};
});

jest.mock('../../../core/Engine', () => ({
context: {
NotificationServicesController: {
Expand Down Expand Up @@ -170,15 +177,13 @@
.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 @@
[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 @@
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]);
Prithpal-Sooriya marked this conversation as resolved.
Show resolved Hide resolved

return { updateAndfetchAccountSettings };
}
Loading