Skip to content

Commit

Permalink
fix notification address settings api spam
Browse files Browse the repository at this point in the history
  • Loading branch information
Prithpal-Sooriya committed Jul 18, 2024
1 parent f8f0dae commit 5f76720
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 78 deletions.
77 changes: 75 additions & 2 deletions ui/hooks/metamask-notifications/useSwitchNotifications.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useState, useCallback } from 'react';
import { useDispatch } from 'react-redux';
import { useState, useCallback, useMemo, useEffect } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import log from 'loglevel';
import {
setFeatureAnnouncementsEnabled,
Expand All @@ -8,6 +8,7 @@ import {
updateOnChainTriggersByAccount,
hideLoadingIndication,
} from '../../store/actions';
import { getIsUpdatingMetamaskNotificationsAccount } from '../../selectors/metamask-notifications/metamask-notifications';

export function useSwitchFeatureAnnouncementsChange(): {
onChange: (state: boolean) => Promise<void>;
Expand Down Expand Up @@ -114,3 +115,75 @@ export function useSwitchAccountNotificationsChange(): {
error,
};
}

function useRefetchAccountSettings() {
const dispatch = useDispatch();

const getAccountSettings = useCallback(async (accounts: string[]) => {
try {
const result = (await dispatch(
checkAccountsPresence(accounts),
)) as unknown as UseSwitchAccountNotificationsData;

return result;
} catch {
return {};
}
}, []);

return getAccountSettings;
}

/**
* Account Settings Hook.
* Gets initial loading states, and returns enable/disable account states.
* Also exposes an update() method so each switch can be manually updated.
*
* @param accounts - the accounts we are checking to see if notifications are enabled/disabled
* @returns props for settings page
*/
export function useAccountSettingsProps(accounts: string[]) {
const accountsBeingUpdated = useSelector(
getIsUpdatingMetamaskNotificationsAccount,
);
const fetchAccountSettings = useRefetchAccountSettings();
const [data, setData] = useState<UseSwitchAccountNotificationsData>({});
const [loading, setLoading] = useState<boolean>(false);
const [error, setError] = useState<string | null>(null);

// Memoize the accounts array to avoid unnecessary re-fetching
const jsonAccounts = useMemo(() => JSON.stringify(accounts), [accounts]);

const update = useCallback(async (addresses: string[]) => {
try {
setLoading(true);
setError(null);
const res = await fetchAccountSettings(addresses);
setData(res);
} catch {
setError('Failed to get account settings');
} finally {
setLoading(false);
}
}, []);

// Effect - async get if accounts are enabled/disabled
useEffect(() => {
try {
const memoAccounts: string[] = JSON.parse(jsonAccounts);
update(memoAccounts);
} catch {
setError('Failed to get account settings');
} finally {
setLoading(false);
}
}, [jsonAccounts, fetchAccountSettings]);

return {
data,
initialLoading: loading,
error,
accountsBeingUpdated,
update,
};
}
100 changes: 53 additions & 47 deletions ui/pages/notifications-settings/notifications-settings-per-account.tsx
Original file line number Diff line number Diff line change
@@ -1,89 +1,95 @@
import React, { useEffect, useState, useCallback, useContext } from 'react';
import React, { useState, useCallback, useContext } from 'react';
import { MetaMetricsContext } from '../../contexts/metametrics';
import {
MetaMetricsEventCategory,
MetaMetricsEventName,
} from '../../../shared/constants/metametrics';
import { useMetamaskNotificationsContext } from '../../contexts/metamask-notifications/metamask-notifications';
import {
useSwitchAccountNotifications,
useSwitchAccountNotificationsChange,
type UseSwitchAccountNotificationsData,
} from '../../hooks/metamask-notifications/useSwitchNotifications';
import { useSwitchAccountNotificationsChange } from '../../hooks/metamask-notifications/useSwitchNotifications';
import {
NotificationsSettingsBox,
NotificationsSettingsAccount,
} from '../../components/multichain';
import { useListNotifications } from '../../hooks/metamask-notifications/useNotifications';

type NotificationsSettingsPerAccountProps = {
address: string;
name: string;
disabled: boolean;
loading: boolean;

isEnabled: boolean;
isLoading?: boolean;
disabledSwitch?: boolean;
refetchAccountSettings: () => Promise<void>;
};

function useUpdateAccountSetting(
address: string,
refetchAccountSettings: () => Promise<void>,
) {
const { onChange: switchAccountNotifications, error } =
useSwitchAccountNotificationsChange();
const { listNotifications: refetch } = useListNotifications();

// Local states
const [loading, setLoading] = useState(false);

const toggleAccount = useCallback(
async (state: boolean) => {
setLoading(true);
try {
await switchAccountNotifications([address], state);
await refetchAccountSettings();
refetch();
} catch {
// Do nothing (we don't need to propagate this)
}
setLoading(false);
},
[address, refetch, refetchAccountSettings, switchAccountNotifications],
);

return { toggleAccount, loading, error };
}

export const NotificationsSettingsPerAccount = ({
address,
name,
disabled,
loading,
isEnabled,
isLoading,
disabledSwitch,
refetchAccountSettings,
}: NotificationsSettingsPerAccountProps) => {
const { listNotifications } = useMetamaskNotificationsContext();
const trackEvent = useContext(MetaMetricsContext);

// Hooks
const {
onChange: onChangeAccountNotifications,
error: errorAccountNotificationsChange,
} = useSwitchAccountNotificationsChange();
const { switchAccountNotifications, error: errorSwitchAccountNotifications } =
useSwitchAccountNotifications();

const [data, setData] = useState<
UseSwitchAccountNotificationsData | undefined
>(undefined);
const [isLoading, setIsLoading] = useState<boolean>(false);

useEffect(() => {
const fetchData = async () => {
const fetchedData = await switchAccountNotifications([address]);
setData(fetchedData || {});
};
fetchData();
}, [address, switchAccountNotifications]);

useEffect(() => {
setIsLoading(loading);
}, [loading]);
toggleAccount,
loading: isUpdatingAccount,
error: accountError,
} = useUpdateAccountSetting(address, refetchAccountSettings);

const error =
errorAccountNotificationsChange || errorSwitchAccountNotifications;
const loading = isLoading || isUpdatingAccount;
const error = accountError;

const handleToggleAccountNotifications = useCallback(async () => {
const originalValue = data?.[address];
await onChangeAccountNotifications([address], !originalValue);
trackEvent({
category: MetaMetricsEventCategory.NotificationSettings,
event: originalValue
event: isEnabled
? MetaMetricsEventName.DisablingAccountNotifications
: MetaMetricsEventName.EnablingAccountNotifications,
properties: {
address,
},
});
const fetchedData = await switchAccountNotifications([address]);
setData(fetchedData || {});
listNotifications();
}, [address, data, onChangeAccountNotifications]);
await toggleAccount(!isEnabled);
}, [address, isEnabled, toggleAccount, trackEvent]);

return (
<>
<NotificationsSettingsBox
value={data?.[address] ?? false}
value={isEnabled ?? false}
onToggle={handleToggleAccountNotifications}
key={address}
disabled={disabled}
loading={isLoading || !data}
disabled={disabledSwitch}
loading={loading}
error={error}
>
<NotificationsSettingsAccount address={address} name={name} />
Expand Down
55 changes: 26 additions & 29 deletions ui/pages/notifications-settings/notifications-settings.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useState } from 'react';
import React, { useMemo, useState } from 'react';
import { useSelector } from 'react-redux';
import { useHistory } from 'react-router-dom';
import type { InternalAccount } from '@metamask/keyring-api';
Expand Down Expand Up @@ -26,9 +26,9 @@ import { Content, Header } from '../../components/multichain/pages/page';
import {
selectIsMetamaskNotificationsEnabled,
getIsUpdatingMetamaskNotifications,
getIsUpdatingMetamaskNotificationsAccount,
} from '../../selectors/metamask-notifications/metamask-notifications';
import { getInternalAccounts } from '../../selectors';
import { useAccountSettingsProps } from '../../hooks/metamask-notifications/useSwitchNotifications';
import { NotificationsSettingsAllowNotifications } from './notifications-settings-allow-notifications';
import { NotificationsSettingsTypes } from './notifications-settings-types';
import { NotificationsSettingsPerAccount } from './notifications-settings-per-account';
Expand Down Expand Up @@ -56,36 +56,23 @@ export default function NotificationsSettings() {
const isUpdatingMetamaskNotifications = useSelector(
getIsUpdatingMetamaskNotifications,
);
const isUpdatingMetamaskNotificationsAccount = useSelector(
getIsUpdatingMetamaskNotificationsAccount,
);
const accounts: AccountType[] = useSelector(getInternalAccounts);

// States
const [loadingAllowNotifications, setLoadingAllowNotifications] =
useState<boolean>(isUpdatingMetamaskNotifications);
const [updatingAccountList, setUpdatingAccountList] = useState<string[]>([]);
const [updatingAccount, setUpdatingAccount] = useState<boolean>(false);

useEffect(() => {
if (updatingAccountList.length > 0) {
setUpdatingAccount(true);
} else {
setUpdatingAccount(false);
}
}, [updatingAccountList]);

useEffect(() => {
if (isUpdatingMetamaskNotifications) {
setLoadingAllowNotifications(isUpdatingMetamaskNotifications);
}
}, [isUpdatingMetamaskNotifications]);
const accountAddresses = useMemo(
() => accounts.map((a) => a.address),
[accounts],
);

useEffect(() => {
if (isUpdatingMetamaskNotificationsAccount) {
setUpdatingAccountList(isUpdatingMetamaskNotificationsAccount);
}
}, [isUpdatingMetamaskNotificationsAccount]);
// Account Settings
const accountSettingsProps = useAccountSettingsProps(accountAddresses);
const updatingAccounts = accountSettingsProps.accountsBeingUpdated.length > 0;
const refetchAccountSettings = async () => {
await accountSettingsProps.update(accountAddresses);
};

return (
<NotificationsPage>
Expand All @@ -108,7 +95,7 @@ export default function NotificationsSettings() {
loading={loadingAllowNotifications}
setLoading={setLoadingAllowNotifications}
data-testid="notifications-settings-allow-notifications"
disabled={updatingAccount}
disabled={updatingAccounts}
/>
<Box
borderColor={BorderColor.borderMuted}
Expand All @@ -120,7 +107,7 @@ export default function NotificationsSettings() {
<>
{/* Notifications settings per types */}
<NotificationsSettingsTypes
disabled={loadingAllowNotifications || updatingAccount}
disabled={loadingAllowNotifications || updatingAccounts}
/>

{/* Notifications settings per account */}
Expand Down Expand Up @@ -160,8 +147,18 @@ export default function NotificationsSettings() {
key={account.id}
address={account.address}
name={account.metadata.name}
disabled={updatingAccountList.length > 0}
loading={updatingAccountList.includes(account.address)}
disabledSwitch={
accountSettingsProps.initialLoading || updatingAccounts
}
isLoading={accountSettingsProps.accountsBeingUpdated.includes(
account.address,
)}
isEnabled={
accountSettingsProps.data?.[
account.address.toLowerCase()
] ?? false
}
refetchAccountSettings={refetchAccountSettings}
/>
))}
</Box>
Expand Down

0 comments on commit 5f76720

Please sign in to comment.