MM-59416 Don't request notification permissions when we already have them (#27629)

* MM-59416 Only request notification permissions from the browser once per session

* MM-59416 Don't request notification permissions when we already have them

* Address feedback

Co-authored-by: Daniel Espino García <larkox@gmail.com>

* Fix bad merge

---------

Co-authored-by: Daniel Espino García <larkox@gmail.com>
Этот коммит содержится в:
Harrison Healey
2024-07-15 12:50:25 -04:00
коммит произвёл GitHub
родитель d44c3d5d45
Коммит d3dac41cda
2 изменённых файлов: 175 добавлений и 62 удалений

Просмотреть файл

@@ -21,6 +21,15 @@ describe('Notifications.showNotification', () => {
let store: ReturnType<typeof configureStore>;
beforeEach(() => {
// Re-initialize window.Notification so that tests can modify it as needed. By default, everything exists,
// we've never requested permissions before, and any request for permissions will be denied.
window.Notification = jest.fn(() => ({
close: jest.fn(),
}));
window.Notification.requestPermission = jest.fn(() => Promise.resolve('denied'));
window.Notification.permission = 'default';
// Reset and re-import utils/notifications for every test to reset requestedNotificationPermission
jest.resetModules();
Notifications = require('utils/notifications');
@@ -28,45 +37,47 @@ describe('Notifications.showNotification', () => {
});
it('should throw an exception if Notification is not defined on window', async () => {
delete window.Notification;
await expect(store.dispatch(Notifications.showNotification())).rejects.toThrow('Notification not supported');
});
it('should throw an exception if Notification.requestPermission is not defined', async () => {
window.Notification = {};
window.Notification = jest.fn();
await expect(store.dispatch(Notifications.showNotification())).rejects.toThrow('Notification.requestPermission not supported');
});
it('should throw an exception if Notification.requestPermission is not a function', async () => {
window.Notification = {
requestPermission: true,
};
await expect(store.dispatch(Notifications.showNotification())).rejects.toThrow('Notification.requestPermission not supported');
});
it('should request permissions, promise style, if not previously requested, do nothing', async () => {
window.Notification = {
requestPermission: () => Promise.resolve('denied'),
permission: 'denied',
};
await expect(store.dispatch(Notifications.showNotification())).resolves.toBeTruthy();
});
it('should request permissions, callback style, if not previously requested, do nothing', async () => {
window.Notification = {
requestPermission: (callback: NotificationPermissionCallback) => {
if (callback) {
callback('denied');
}
},
permission: 'denied',
};
await expect(store.dispatch(Notifications.showNotification())).resolves.toBeTruthy();
});
it('should request permissions, promise style, if not previously requested, handling success', async () => {
window.Notification = jest.fn();
window.Notification.requestPermission = () => Promise.resolve('granted');
window.Notification.permission = 'denied';
window.Notification.requestPermission = true;
await expect(store.dispatch(Notifications.showNotification())).rejects.toThrow('Notification.requestPermission not supported');
expect(window.Notification).not.toHaveBeenCalled();
});
it('should request permissions, promise style, if not previously requested and not show a notification when permission is denied', async () => {
window.Notification.requestPermission.mockResolvedValue('denied');
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_denied',
});
expect(window.Notification).not.toHaveBeenCalled();
});
it('should request permissions, callback style, if not previously requested and not show a notification when permission is denied', async () => {
window.Notification.requestPermission = (callback: NotificationPermissionCallback) => callback?.('denied');
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_denied',
});
expect(window.Notification).not.toHaveBeenCalled();
});
it('should request permissions, promise style, if not previously requested and show notification when permission is granted', async () => {
window.Notification.requestPermission.mockResolvedValue('granted');
const n = {};
window.Notification.mockReturnValueOnce(n);
@@ -76,8 +87,10 @@ describe('Notifications.showNotification', () => {
requireInteraction: true,
silent: false,
title: '',
}))).resolves.toBeTruthy();
await expect(window.Notification.mock.calls.length).toBe(1);
}))).resolves.toMatchObject({
status: 'success',
});
expect(window.Notification.mock.calls.length).toBe(1);
const call = window.Notification.mock.calls[0];
expect(call[1]).toEqual({
body: 'body',
@@ -88,25 +101,22 @@ describe('Notifications.showNotification', () => {
});
});
it('should request permissions, callback style, if not previously requested, handling success', async () => {
window.Notification = jest.fn();
it('should request permissions, callback style, if not previously requested and show notification when permission is granted', async () => {
window.Notification.requestPermission = (callback: NotificationPermissionCallback) => {
if (callback) {
callback('granted');
}
};
window.Notification.permission = 'denied';
const n = {};
window.Notification.mockReturnValueOnce(n);
await expect(store.dispatch(Notifications.showNotification({
body: 'body',
requireInteraction: true,
silent: false,
title: '',
}))).resolves.toBeTruthy();
await expect(window.Notification.mock.calls.length).toBe(1);
}))).resolves.toMatchObject({
status: 'success',
});
expect(window.Notification.mock.calls.length).toBe(1);
const call = window.Notification.mock.calls[0];
expect(call[1]).toEqual({
body: 'body',
@@ -118,15 +128,116 @@ describe('Notifications.showNotification', () => {
});
it('should do nothing if permissions previously requested but not granted', async () => {
window.Notification = {
requestPermission: () => Promise.resolve('denied'),
permission: 'denied',
};
window.Notification.requestPermission.mockResolvedValue('denied');
// Call one to deny and mark as already requested, do nothing, throw nothing
await expect(store.dispatch(Notifications.showNotification())).resolves.toBeTruthy();
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_denied',
});
// Try again
await expect(store.dispatch(Notifications.showNotification())).resolves.toBeTruthy();
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_previously_denied',
});
});
it('should only request permissions once if permissions are granted in response to that request', async () => {
window.Notification.requestPermission.mockResolvedValue('granted');
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'success',
});
expect(window.Notification).toHaveBeenCalledTimes(1);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(1);
window.Notification.permission = 'granted';
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'success',
});
expect(window.Notification).toHaveBeenCalledTimes(2);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(1);
});
it('should only request permissions once if request gets denied', async () => {
window.Notification.requestPermission.mockResolvedValue('denied');
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_denied',
});
expect(window.Notification).toHaveBeenCalledTimes(0);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(1);
window.Notification.permission = 'denied';
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_previously_denied',
});
expect(window.Notification).toHaveBeenCalledTimes(0);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(1);
});
it('should only request permissions once if request gets ignored', async () => {
window.Notification.requestPermission.mockResolvedValue('default');
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_denied',
});
expect(window.Notification).toHaveBeenCalledTimes(0);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(1);
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_previously_denied',
});
expect(window.Notification).toHaveBeenCalledTimes(0);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(1);
});
it('should not request permission if it was granted during a previous session', async () => {
window.Notification.permission = 'granted';
// Reload utils/notifications to set requestedNotificationPermission to true based on Notification.permission
jest.resetModules();
Notifications = require('utils/notifications');
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'success',
});
expect(window.Notification).toHaveBeenCalledTimes(1);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(0);
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'success',
});
expect(window.Notification).toHaveBeenCalledTimes(2);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(0);
});
it('should not request permission if it was denied during a previous session', async () => {
window.Notification.permission = 'denied';
// Reload utils/notifications to set requestedNotificationPermission to true based on Notification.permission
jest.resetModules();
Notifications = require('utils/notifications');
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_previously_denied',
});
expect(window.Notification).toHaveBeenCalledTimes(0);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(0);
await expect(store.dispatch(Notifications.showNotification())).resolves.toMatchObject({
status: 'not_sent',
reason: 'notifications_permission_previously_denied',
});
expect(window.Notification).toHaveBeenCalledTimes(0);
expect(window.Notification.requestPermission).toHaveBeenCalledTimes(0);
});
});

Просмотреть файл

@@ -14,7 +14,7 @@ export type NotificationResult = {
data?: string;
}
let requestedNotificationPermission = false;
let requestedNotificationPermission = Boolean('Notification' in window && Notification.permission !== 'default');
// showNotification displays a platform notification with the configured parameters.
//
@@ -60,24 +60,26 @@ export function showNotification(
throw new Error('Notification.requestPermission not supported');
}
if (Notification.permission !== 'granted' && requestedNotificationPermission) {
// User didn't allow notifications
return {status: 'not_sent', reason: 'notifications_permission_previously_denied', data: Notification.permission, callback: () => {}};
}
if (Notification.permission !== 'granted') {
if (requestedNotificationPermission) {
// User didn't allow notifications
return {status: 'not_sent', reason: 'notifications_permission_previously_denied', data: Notification.permission, callback: () => {}};
}
requestedNotificationPermission = true;
requestedNotificationPermission = true;
let permission = await Notification.requestPermission();
if (typeof permission === 'undefined') {
// Handle browsers that don't support the promise-based syntax.
permission = await new Promise((resolve) => {
Notification.requestPermission(resolve);
});
}
let permission = await Notification.requestPermission();
if (typeof permission === 'undefined') {
// Handle browsers that don't support the promise-based syntax.
permission = await new Promise((resolve) => {
Notification.requestPermission(resolve);
});
}
if (permission !== 'granted') {
// User has denied notification for the site
return {status: 'not_sent', reason: 'notifications_permission_denied', data: permission, callback: () => {}};
if (permission !== 'granted') {
// User has denied notification for the site
return {status: 'not_sent', reason: 'notifications_permission_denied', data: permission, callback: () => {}};
}
}
const notification = new Notification(title, {