Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Implement push notification toggle in device detail #9308

Merged
merged 19 commits into from
Sep 27, 2022
Merged
Prev Previous commit
Next Next commit
Implement push notification toggle in device detail
Germain Souquet committed Sep 22, 2022
commit 1fa1c75a3c26c7d8a21399d68691b3f4591c1b2b
49 changes: 34 additions & 15 deletions src/components/views/settings/devices/DeviceDetails.tsx
Original file line number Diff line number Diff line change
@@ -14,7 +14,9 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React from 'react';
import React, { useState, useEffect, useContext } from 'react';
import { IPusher } from 'matrix-js-sdk/src/@types/PushRules';
import { PUSHER_ENABLED } from 'matrix-js-sdk/src/@types/event';

import { formatDate } from '../../../../DateUtils';
import { _t } from '../../../../languageHandler';
@@ -24,13 +26,16 @@ import ToggleSwitch from '../../elements/ToggleSwitch';
import { DeviceDetailHeading } from './DeviceDetailHeading';
import { DeviceVerificationStatusCard } from './DeviceVerificationStatusCard';
import { DeviceWithVerification } from './types';
import MatrixClientContext from '../../../../contexts/MatrixClientContext';

interface Props {
device: DeviceWithVerification;
pusher: IPusher | null;
isSigningOut: boolean;
onVerifyDevice?: () => void;
onSignOutDevice: () => void;
saveDeviceName: (deviceName: string) => Promise<void>;
setPusherEnabled: (deviceId: string, enabled: boolean) => Promise<void>;
}

interface MetadataTable {
@@ -40,11 +45,21 @@ interface MetadataTable {

const DeviceDetails: React.FC<Props> = ({
device,
pusher,
isSigningOut,
onVerifyDevice,
onSignOutDevice,
saveDeviceName,
setPusherEnabled,
}) => {
const [supportMSC3881, setSupportsMSC3881] = useState<boolean>(false);
const matrixClient = useContext(MatrixClientContext);
useEffect(() => {
matrixClient.doesServerSupportUnstableFeature("org.matrix.msc3881").then(isEnabled => {
setSupportsMSC3881(isEnabled);
});
}, [matrixClient]);

const metadata: MetadataTable[] = [
{
values: [
@@ -94,20 +109,24 @@ const DeviceDetails: React.FC<Props> = ({
</table>,
) }
</section>
<section className='mx_DeviceDetails_section mx_DeviceDetails_pushNotifications'>
<ToggleSwitch
checked={true}
disabled={true}
onChange={() => {}}
aria-label={_t("Toggle push notifications on this session.")}
/>
<p className='mx_DeviceDetails_sectionHeading'>
{ _t('Push notifications') }
<small className='mx_DeviceDetails_sectionSubheading'>
{ _t('Receive push notifications on this session.') }
</small>
</p>
</section>
{ pusher && (
<section className='mx_DeviceDetails_section mx_DeviceDetails_pushNotifications'>
<ToggleSwitch
// For backwards compatibility, if `enabled` is missing
// default to `true`
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
checked={pusher?.[PUSHER_ENABLED.name] ?? true}
disabled={!supportMSC3881}
onChange={(checked) => setPusherEnabled(device.device_id, checked)}
aria-label={_t("Toggle push notifications on this session.")}
/>
<p className='mx_DeviceDetails_sectionHeading'>
{ _t('Push notifications') }
<small className='mx_DeviceDetails_sectionSubheading'>
{ _t('Receive push notifications on this session.') }
</small>
</p>
</section>
) }
<section className='mx_DeviceDetails_section'>
<AccessibleButton
onClick={onSignOutDevice}
18 changes: 18 additions & 0 deletions src/components/views/settings/devices/FilteredDeviceList.tsx
Original file line number Diff line number Diff line change
@@ -15,6 +15,8 @@ limitations under the License.
*/

import React, { ForwardedRef, forwardRef } from 'react';
import { IPusher } from 'matrix-js-sdk/src/@types/PushRules';
import { PUSHER_DEVICE_ID } from 'matrix-js-sdk/src/@types/event';

import { _t } from '../../../../languageHandler';
import AccessibleButton from '../../elements/AccessibleButton';
@@ -36,6 +38,7 @@ import { DevicesState } from './useOwnDevices';

interface Props {
devices: DevicesDictionary;
pushers: IPusher[];
expandedDeviceIds: DeviceWithVerification['device_id'][];
signingOutDeviceIds: DeviceWithVerification['device_id'][];
filter?: DeviceSecurityVariation;
@@ -44,6 +47,7 @@ interface Props {
onSignOutDevices: (deviceIds: DeviceWithVerification['device_id'][]) => void;
saveDeviceName: DevicesState['saveDeviceName'];
onRequestDeviceVerification?: (deviceId: DeviceWithVerification['device_id']) => void;
setPusherEnabled: (deviceId: string, enabled: boolean) => Promise<void>;
}

// devices without timestamp metadata should be sorted last
@@ -135,20 +139,24 @@ const NoResults: React.FC<NoResultsProps> = ({ filter, clearFilter }) =>

const DeviceListItem: React.FC<{
device: DeviceWithVerification;
pusher: IPusher | null;
isExpanded: boolean;
isSigningOut: boolean;
onDeviceExpandToggle: () => void;
onSignOutDevice: () => void;
saveDeviceName: (deviceName: string) => Promise<void>;
onRequestDeviceVerification?: () => void;
setPusherEnabled: (deviceId: string, enabled: boolean) => Promise<void>;
}> = ({
device,
pusher,
isExpanded,
isSigningOut,
onDeviceExpandToggle,
onSignOutDevice,
saveDeviceName,
onRequestDeviceVerification,
setPusherEnabled,
}) => <li className='mx_FilteredDeviceList_listItem'>
<DeviceTile
device={device}
@@ -162,10 +170,12 @@ const DeviceListItem: React.FC<{
isExpanded &&
<DeviceDetails
device={device}
pusher={pusher}
isSigningOut={isSigningOut}
onVerifyDevice={onRequestDeviceVerification}
onSignOutDevice={onSignOutDevice}
saveDeviceName={saveDeviceName}
setPusherEnabled={setPusherEnabled}
/>
}
</li>;
@@ -177,6 +187,7 @@ const DeviceListItem: React.FC<{
export const FilteredDeviceList =
forwardRef(({
devices,
pushers,
filter,
expandedDeviceIds,
signingOutDeviceIds,
@@ -185,9 +196,14 @@ export const FilteredDeviceList =
saveDeviceName,
onSignOutDevices,
onRequestDeviceVerification,
setPusherEnabled,
}: Props, ref: ForwardedRef<HTMLDivElement>) => {
const sortedDevices = getFilteredSortedDevices(devices, filter);

function getPusherForDevice(device: DeviceWithVerification): IPusher | null {
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
return pushers.find(pusher => pusher[PUSHER_DEVICE_ID.name] === device.device_id);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function getPusherForDevice(device: DeviceWithVerification): IPusher | null {
return pushers.find(pusher => pusher[PUSHER_DEVICE_ID.name] === device.device_id);
}
const getPusherForDevice = (device: DeviceWithVerification): IPusher | null => {
return pushers.find(pusher => pusher[PUSHER_DEVICE_ID.name] === device.device_id);
};

I'm paranoid about scope changing randomly on us

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the rationale behind this change.
This function, overall declared will be scoped to the function it's declared in. Using const will scope it to the block it's declared in...
It's the whole concept of closures and sounds more like a stylistic preference


const options: FilterDropdownOption<DeviceFilterKey>[] = [
{ id: ALL_FILTER_ID, label: _t('All') },
{
@@ -236,6 +252,7 @@ export const FilteredDeviceList =
{ sortedDevices.map((device) => <DeviceListItem
key={device.device_id}
device={device}
pusher={getPusherForDevice(device)}
isExpanded={expandedDeviceIds.includes(device.device_id)}
isSigningOut={signingOutDeviceIds.includes(device.device_id)}
onDeviceExpandToggle={() => onDeviceExpandToggle(device.device_id)}
@@ -246,6 +263,7 @@ export const FilteredDeviceList =
? () => onRequestDeviceVerification(device.device_id)
: undefined
}
setPusherEnabled={setPusherEnabled}
/>,
) }
</ol>
27 changes: 26 additions & 1 deletion src/components/views/settings/devices/useOwnDevices.ts
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@ limitations under the License.
*/

import { useCallback, useContext, useEffect, useState } from "react";
import { IMyDevice, MatrixClient } from "matrix-js-sdk/src/matrix";
import { IMyDevice, IPusher, MatrixClient, PUSHER_DEVICE_ID, PUSHER_ENABLED } from "matrix-js-sdk/src/matrix";
import { CrossSigningInfo } from "matrix-js-sdk/src/crypto/CrossSigning";
import { VerificationRequest } from "matrix-js-sdk/src/crypto/verification/request/VerificationRequest";
import { MatrixError } from "matrix-js-sdk/src/http-api";
@@ -76,12 +76,14 @@ export enum OwnDevicesError {
}
export type DevicesState = {
devices: DevicesDictionary;
pushers: IPusher[];
currentDeviceId: string;
isLoadingDeviceList: boolean;
// not provided when current session cannot request verification
requestDeviceVerification?: (deviceId: DeviceWithVerification['device_id']) => Promise<VerificationRequest>;
refreshDevices: () => Promise<void>;
saveDeviceName: (deviceId: DeviceWithVerification['device_id'], deviceName: string) => Promise<void>;
setPusherEnabled: (deviceId: DeviceWithVerification['device_id'], enabled: boolean) => Promise<void>;
error?: OwnDevicesError;
};
export const useOwnDevices = (): DevicesState => {
@@ -91,6 +93,7 @@ export const useOwnDevices = (): DevicesState => {
const userId = matrixClient.getUserId();

const [devices, setDevices] = useState<DevicesState['devices']>({});
const [pushers, setPushers] = useState<DevicesState['pushers']>([]);
const [isLoadingDeviceList, setIsLoadingDeviceList] = useState(true);

const [error, setError] = useState<OwnDevicesError>();
@@ -105,6 +108,10 @@ export const useOwnDevices = (): DevicesState => {
}
const devices = await fetchDevicesWithVerification(matrixClient, userId);
setDevices(devices);

const { pushers } = await matrixClient.getPushers();
setPushers(pushers);

setIsLoadingDeviceList(false);
} catch (error) {
if ((error as MatrixError).httpStatus == 404) {
@@ -154,13 +161,31 @@ export const useOwnDevices = (): DevicesState => {
}
}, [matrixClient, devices, refreshDevices]);

const setPusherEnabled = useCallback(
async (deviceId: DeviceWithVerification['device_id'], enabled: boolean): Promise<void> => {
const pusher = pushers.find(pusher => pusher[PUSHER_DEVICE_ID.name] === deviceId);
try {
await matrixClient.setPusher({
...pusher,
[PUSHER_ENABLED.name]: enabled,
});
await refreshDevices();
} catch (error) {
logger.error("Error setting session display name", error);
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
throw new Error(_t("Failed to set display name"));
}
}, [matrixClient, pushers, refreshDevices],
);

return {
devices,
pushers,
currentDeviceId,
isLoadingDeviceList,
error,
requestDeviceVerification,
refreshDevices,
saveDeviceName,
setPusherEnabled,
};
};
Original file line number Diff line number Diff line change
@@ -87,11 +87,13 @@ const useSignOut = (
const SessionManagerTab: React.FC = () => {
const {
devices,
pushers,
currentDeviceId,
isLoadingDeviceList,
requestDeviceVerification,
refreshDevices,
saveDeviceName,
setPusherEnabled,
} = useOwnDevices();
const [filter, setFilter] = useState<DeviceSecurityVariation>();
const [expandedDeviceIds, setExpandedDeviceIds] = useState<DeviceWithVerification['device_id'][]>([]);
@@ -186,6 +188,7 @@ const SessionManagerTab: React.FC = () => {
>
<FilteredDeviceList
devices={otherDevices}
pushers={pushers}
filter={filter}
expandedDeviceIds={expandedDeviceIds}
signingOutDeviceIds={signingOutDeviceIds}
@@ -194,6 +197,7 @@ const SessionManagerTab: React.FC = () => {
onRequestDeviceVerification={requestDeviceVerification ? onTriggerDeviceVerification : undefined}
onSignOutDevices={onSignOutOtherDevices}
saveDeviceName={saveDeviceName}
setPusherEnabled={setPusherEnabled}
ref={filteredDeviceListRef}
/>
</SettingsSubsection>
Original file line number Diff line number Diff line change
@@ -19,6 +19,8 @@ import { fireEvent, render } from '@testing-library/react';
import { act } from 'react-dom/test-utils';

import CurrentDeviceSection from '../../../../../src/components/views/settings/devices/CurrentDeviceSection';
import MatrixClientContext from '../../../../../src/contexts/MatrixClientContext';
import { getMockClientWithEventEmitter } from '../../../../test-utils';

describe('<CurrentDeviceSection />', () => {
const deviceId = 'alices_device';
@@ -40,8 +42,15 @@ describe('<CurrentDeviceSection />', () => {
isLoading: false,
isSigningOut: false,
};
const getComponent = (props = {}): React.ReactElement =>
(<CurrentDeviceSection {...defaultProps} {...props} />);

const mockClient = getMockClientWithEventEmitter({
doesServerSupportUnstableFeature: jest.fn().mockReturnValue(Promise.resolve(true)),
});

const getComponent = (props = {}) =>
(<MatrixClientContext.Provider value={mockClient}>
<CurrentDeviceSection {...defaultProps} {...props} />
</MatrixClientContext.Provider>);

it('renders spinner while device is loading', () => {
const { container } = render(getComponent({ device: undefined, isLoading: true }));
15 changes: 14 additions & 1 deletion test/components/views/settings/devices/DeviceDetails-test.tsx
Original file line number Diff line number Diff line change
@@ -18,6 +18,8 @@ import React from 'react';
import { render } from '@testing-library/react';

import DeviceDetails from '../../../../../src/components/views/settings/devices/DeviceDetails';
import MatrixClientContext from '../../../../../src/contexts/MatrixClientContext';
import { getMockClientWithEventEmitter } from '../../../../test-utils';

describe('<DeviceDetails />', () => {
const baseDevice = {
@@ -26,12 +28,23 @@ describe('<DeviceDetails />', () => {
};
const defaultProps = {
device: baseDevice,
pusher: null,
isSigningOut: false,
isLoading: false,
onSignOutDevice: jest.fn(),
saveDeviceName: jest.fn(),
setPusherEnabled: jest.fn(),
};
const getComponent = (props = {}) => <DeviceDetails {...defaultProps} {...props} />;

const mockClient = getMockClientWithEventEmitter({
doesServerSupportUnstableFeature: jest.fn().mockReturnValue(Promise.resolve(true)),
});

const getComponent = (props = {}) =>
(<MatrixClientContext.Provider value={mockClient}>
<DeviceDetails {...defaultProps} {...props} />
</MatrixClientContext.Provider>);

// 14.03.2022 16:15
const now = 1647270879403;
jest.useFakeTimers();
Original file line number Diff line number Diff line change
@@ -19,7 +19,8 @@ import { act, fireEvent, render } from '@testing-library/react';

import { FilteredDeviceList } from '../../../../../src/components/views/settings/devices/FilteredDeviceList';
import { DeviceSecurityVariation } from '../../../../../src/components/views/settings/devices/types';
import { flushPromises, mockPlatformPeg } from '../../../../test-utils';
import { flushPromises, getMockClientWithEventEmitter, mockPlatformPeg } from '../../../../test-utils';
import MatrixClientContext from '../../../../../src/contexts/MatrixClientContext';

mockPlatformPeg();

@@ -45,6 +46,7 @@ describe('<FilteredDeviceList />', () => {
onDeviceExpandToggle: jest.fn(),
onSignOutDevices: jest.fn(),
saveDeviceName: jest.fn(),
setPusherEnabled: jest.fn(),
expandedDeviceIds: [],
signingOutDeviceIds: [],
devices: {
@@ -54,9 +56,17 @@ describe('<FilteredDeviceList />', () => {
[hundredDaysOld.device_id]: hundredDaysOld,
[hundredDaysOldUnverified.device_id]: hundredDaysOldUnverified,
},
pushers: [],
};

const mockClient = getMockClientWithEventEmitter({
doesServerSupportUnstableFeature: jest.fn().mockReturnValue(Promise.resolve(true)),
});

const getComponent = (props = {}) =>
(<FilteredDeviceList {...defaultProps} {...props} />);
(<MatrixClientContext.Provider value={mockClient}>
<FilteredDeviceList {...defaultProps} {...props} />
</MatrixClientContext.Provider>);

it('renders devices in correct order', () => {
const { container } = render(getComponent());
Loading