Skip to content

Commit

Permalink
fix: notifications first round of tests (#10668)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

This PR introduces smalls fixes on Notifications feature after the first
round of tests.

[x] - Error when enabling Notifications due embedded Snap Execution
Environment. Changed to use external URL.
[x] - Misalignment on UI components like Loading and Notifications
Header.
[x] - Bug `Cannot update a component while rendering a different
component` due unnecessary usage of navigationsOptions on Notifications
Details page.
[x] - Fix async dependency on Notification Enable toggle & Alert. 

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
  • Loading branch information
Jonathansoufer and legobeat authored Aug 16, 2024
1 parent e7098a8 commit d05aeb1
Show file tree
Hide file tree
Showing 13 changed files with 229 additions and 111 deletions.
11 changes: 7 additions & 4 deletions app/components/Nav/App/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ const VaultRecoveryFlow = () => (
);

// eslint-disable-next-line react/prop-types
const App = ({ userLoggedIn }) => {
const App = ({ userLoggedIn, isBasicFunctionalityEnabled }) => {
const animationRef = useRef(null);
const animationNameRef = useRef(null);
const opacity = useRef(new Animated.Value(1)).current;
Expand Down Expand Up @@ -811,9 +811,11 @@ const App = ({ userLoggedIn }) => {
{
///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps)
}
<View>
<SnapsExecutionWebView />
</View>
{isBasicFunctionalityEnabled && (
<View>
<SnapsExecutionWebView />
</View>
)}
{
///: END:ONLY_INCLUDE_IF
}
Expand Down Expand Up @@ -950,6 +952,7 @@ const App = ({ userLoggedIn }) => {

const mapStateToProps = (state) => ({
userLoggedIn: state.user.userLoggedIn,
isBasicFunctionalityEnabled: state.settings?.basicFunctionalityEnabled,
});

export default connect(mapStateToProps)(App);
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ exports[`NotificationsList should render correctly 1`] = `
<View
style={
{
"alignItems": "center",
"height": "100%",
"justifyContent": "center",
"position": "absolute",
"width": "100%",
"zIndex": 999,
Expand Down
2 changes: 2 additions & 0 deletions app/components/UI/Notification/List/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export const createStyles = ({ colors, typography }: Theme) =>
zIndex: 999,
width: '100%',
height: '100%',
justifyContent: 'center',
alignItems: 'center',
},
menuItemContainer: {
flexDirection: 'row',
Expand Down
29 changes: 1 addition & 28 deletions app/components/Views/Notifications/Details/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import { ScrollView } from 'react-native-gesture-handler';
import { Notification } from '../../../../util/notifications';
import { useTheme } from '../../../../util/theme';

import {
NavigationProp,
ParamListBase,
RouteProp,
} from '@react-navigation/native';
import { NavigationProp, ParamListBase } from '@react-navigation/native';
import Icon, {
IconName,
IconSize,
Expand All @@ -20,7 +16,6 @@ import { createStyles } from './styles';
import ModalField from './Fields';
import ModalHeader from './Headers';
import ModalFooter from './Footers';
import { NotificationModalDetails } from '../../../../util/notifications/notification-states/types/NotificationModalDetails';
import { toLocaleDate } from '../../../../util/date';

interface Props {
Expand Down Expand Up @@ -90,7 +85,6 @@ const NotificationsDetails = ({ route, navigation }: Props) => {
if (!state) {
return null;
}

return (
<ScrollView contentContainerStyle={styles.contentContainerWrapper}>
<View style={styles.renderContainer}>
Expand All @@ -115,24 +109,3 @@ const NotificationsDetails = ({ route, navigation }: Props) => {
};

export default NotificationsDetails;

NotificationsDetails.navigationOptions = ({
route,
navigation,
state,
}: {
route: RouteProp<{ params: { notification: Notification } }, 'params'>;
navigation: NavigationProp<Record<string, undefined>>;
state: NotificationModalDetails;
}) => {
const notification = route?.params?.notification;
if (!notification) {
navigation.goBack();
return {};
}

if (!state) {
navigation.goBack();
return {};
}
};
2 changes: 1 addition & 1 deletion app/components/Views/Notifications/Details/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,6 @@ export const createStyles = ({ colors, typography }: Theme) =>
marginTop: -16,
},
backIcon: {
marginHorizontal: 16,
marginLeft: 16,
},
});
18 changes: 10 additions & 8 deletions app/components/Views/Notifications/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,16 @@ const NotificationsView = ({
web3Notifications={announcementNotifications}
loading={isLoading}
/>
<Button
variant={ButtonVariants.Primary}
label={strings('notifications.mark_all_as_read')}
onPress={handleMarkAllAsRead}
size={ButtonSize.Lg}
style={styles.stickyButton}
disabled={loading}
/>
{!isLoading && (
<Button
variant={ButtonVariants.Primary}
label={strings('notifications.mark_all_as_read')}
onPress={handleMarkAllAsRead}
size={ButtonSize.Lg}
style={styles.stickyButton}
disabled={loading}
/>
)}
</>
) : (
<Empty
Expand Down
14 changes: 8 additions & 6 deletions app/components/Views/Settings/NotificationsSettings/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ import { NotificationsToggleTypes } from './NotificationsSettings.constants';

import { selectIsMetamaskNotificationsEnabled } from '../../../../selectors/notifications';

import { requestPushNotificationsPermission } from '../../../../util/notifications';
import {
requestPushNotificationsPermission,
asyncAlert,
} from '../../../../util/notifications';
import Routes from '../../../../constants/navigation/Routes';
import { IconName } from '../../../../component-library/components/Icons/Icon';
import ButtonIcon, {
Expand Down Expand Up @@ -83,12 +86,11 @@ const NotificationsSettings = ({ navigation, route }: Props) => {

const toggleNotificationsEnabled = async () => {
if (!isMetamaskNotificationsEnabled) {
const notificationSettings = await requestPushNotificationsPermission();
const nativeNotificationStatus = await requestPushNotificationsPermission(
asyncAlert,
);

if (
notificationSettings &&
notificationSettings.authorizationStatus >= 1
) {
if (nativeNotificationStatus) {
await enableNotifications();
}
} else {
Expand Down
5 changes: 2 additions & 3 deletions app/components/Views/Wallet/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ import {
} from '../../../reducers/collectibles';
import { getCurrentRoute } from '../../../reducers/navigation';
import { WalletViewSelectorsIDs } from '../../../../e2e/selectors/wallet/WalletView.selectors';
import { selectIsMetamaskNotificationsEnabled } from '../../../selectors/notifications';

const createStyles = ({ colors, typography }: Theme) =>
StyleSheet.create({
Expand Down Expand Up @@ -270,9 +271,7 @@ const Wallet = ({
);

const isNotificationEnabled = useSelector(
// TODO: Replace "any" with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(state: any) => state.notification?.notificationsSettings?.isEnabled,
selectIsMetamaskNotificationsEnabled,
);

const networkName = useSelector(selectNetworkName);
Expand Down
5 changes: 3 additions & 2 deletions app/core/NotificationManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
NotificationTransactionTypes,
isNotificationsFeatureEnabled,
requestPushNotificationsPermission,
asyncAlert,
} from '../util/notifications';
import { safeToChecksumAddress } from '../util/address';
import ReviewManager from './ReviewManager';
Expand Down Expand Up @@ -253,7 +254,7 @@ class NotificationManager {

Device.isIos() &&
setTimeout(() => {
requestPushNotificationsPermission();
requestPushNotificationsPermission(asyncAlert);
}, 5000);

// Prompt review
Expand Down Expand Up @@ -487,7 +488,7 @@ export default {
return instance?.gotIncomingTransaction(lastBlock);
},
requestPushNotificationsPermission() {
return instance?.requestPushNotificationsPermission();
return instance?.requestPushNotificationsPermission(asyncAlert);
},
showSimpleNotification(data) {
return instance?.showSimpleNotification(data);
Expand Down
114 changes: 114 additions & 0 deletions app/util/notifications/pushPermissions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import notifee, {
NotificationSettings,
AuthorizationStatus,
IOSNotificationSetting,
IOSNotificationSettings,
AndroidNotificationSettings,
AndroidNotificationSetting,
} from '@notifee/react-native';

import { strings } from '../../../locales/i18n';
import { requestPushNotificationsPermission } from './pushPermissions';

jest.mock('@notifee/react-native', () => ({
getNotificationSettings: jest.fn(),
AuthorizationStatus: {
AUTHORIZED: 1,
DENIED: 2,
},
IOSNotificationSetting: {
ENABLED: 1,
DISABLED: 2,
},
AndroidNotificationSetting: {
ENABLED: 1,
DISABLED: 2,
},
}));

jest.mock('../Logger', () => ({
error: jest.fn(),
}));

jest.mock('../../../locales/i18n', () => ({
strings: jest.fn(),
}));

jest.mock('./pushPermissions', () => ({
...jest.requireActual('./pushPermissions'),
AsyncAlert: jest.fn(),
}));

describe('requestPushNotificationsPermission', () => {
afterEach(() => {
jest.clearAllMocks();
});

const mockIOSSettings: IOSNotificationSettings = {
authorizationStatus: AuthorizationStatus.AUTHORIZED,
alert: IOSNotificationSetting.ENABLED,
badge: IOSNotificationSetting.ENABLED,
sound: IOSNotificationSetting.ENABLED,
carPlay: IOSNotificationSetting.DISABLED,
criticalAlert: IOSNotificationSetting.DISABLED,
inAppNotificationSettings: IOSNotificationSetting.DISABLED,
lockScreen: IOSNotificationSetting.ENABLED,
notificationCenter: IOSNotificationSetting.ENABLED,
showPreviews: 1,
announcement: 1,
};

const mockAndroidSettings: AndroidNotificationSettings = {
alarm: AndroidNotificationSetting.ENABLED,
};

it('should return notification settings if already authorized', async () => {
const mockSettings: NotificationSettings = {
authorizationStatus: AuthorizationStatus.AUTHORIZED,
ios: mockIOSSettings,
android: mockAndroidSettings,
web: {},
};

(notifee.getNotificationSettings as jest.Mock).mockResolvedValue(
mockSettings,
);

const result = await requestPushNotificationsPermission();

expect(notifee.getNotificationSettings).toHaveBeenCalledTimes(1);
expect(result).toBe(mockSettings);
});

it('should prompt the user with AsyncAlert and simulate a click', async () => {
const mockSettings: NotificationSettings = {
authorizationStatus: AuthorizationStatus.DENIED,
ios: mockIOSSettings,
android: mockAndroidSettings,
web: {},
};

const updatedSettings: NotificationSettings = {
...mockSettings,
authorizationStatus: AuthorizationStatus.AUTHORIZED,
};

(notifee.getNotificationSettings as jest.Mock)
.mockResolvedValueOnce(mockSettings)
.mockResolvedValueOnce(updatedSettings);
(strings as jest.Mock).mockImplementation((key: string) => key);

const mockAsyncAlert = jest.fn().mockResolvedValue(true);

const result = await requestPushNotificationsPermission(mockAsyncAlert);

expect(mockAsyncAlert).toHaveBeenCalledWith(
'notifications.prompt_title',
'notifications.prompt_desc',
);
expect(notifee.getNotificationSettings).toHaveBeenCalledTimes(2);
expect(strings).toHaveBeenCalledWith('notifications.prompt_title');
expect(strings).toHaveBeenCalledWith('notifications.prompt_desc');
expect(result).toBe(updatedSettings);
});
});
Loading

0 comments on commit d05aeb1

Please sign in to comment.