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

Live location share - handle insufficient permissions in location sharing (PSG-610) #9047

Merged
merged 2 commits into from
Jul 13, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/components/views/dialogs/QuestionDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { IDialogProps } from "./IDialogProps";
import BaseDialog from "./BaseDialog";
import DialogButtons from "../elements/DialogButtons";

interface IProps extends IDialogProps {
export interface IQuestionDialogProps extends IDialogProps {
title?: string;
description?: React.ReactNode;
extraButtons?: React.ReactNode;
Expand All @@ -39,8 +39,8 @@ interface IProps extends IDialogProps {
cancelButton?: React.ReactNode;
}

export default class QuestionDialog extends React.Component<IProps> {
public static defaultProps: Partial<IProps> = {
export default class QuestionDialog extends React.Component<IQuestionDialogProps> {
public static defaultProps: Partial<IQuestionDialogProps> = {
title: "",
description: "",
extraButtons: null,
Expand Down
41 changes: 36 additions & 5 deletions src/components/views/location/shareLocation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ limitations under the License.
*/

import { MatrixClient } from "matrix-js-sdk/src/client";
import { MatrixError } from "matrix-js-sdk/src/http-api";
import { makeLocationContent, makeBeaconInfoContent } from "matrix-js-sdk/src/content-helpers";
import { logger } from "matrix-js-sdk/src/logger";
import { IEventRelation } from "matrix-js-sdk/src/models/event";
Expand All @@ -23,7 +24,7 @@ import { THREAD_RELATION_TYPE } from "matrix-js-sdk/src/models/thread";

import { _t } from "../../../languageHandler";
import Modal from "../../../Modal";
import QuestionDialog from "../dialogs/QuestionDialog";
import QuestionDialog, { IQuestionDialogProps } from "../dialogs/QuestionDialog";
import SdkConfig from "../../../SdkConfig";
import { OwnBeaconStore } from "../../../stores/OwnBeaconStore";

Expand All @@ -44,12 +45,32 @@ const DEFAULT_LIVE_DURATION = 300000;

export type ShareLocationFn = (props: LocationShareProps) => Promise<void>;

const handleShareError = (error: Error, openMenu: () => void, shareType: LocationShareType) => {
const getPermissionsErrorParams = (shareType: LocationShareType): {
errorMessage: string;
modalParams: IQuestionDialogProps;
} => {
const errorMessage = shareType === LocationShareType.Live ?
"Insufficient permissions to start sharing your live location" :
"Insufficient permissions to send your location";
Copy link
Member

Choose a reason for hiding this comment

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

According to our new code style ternaries should look like this:

Suggested change
const errorMessage = shareType === LocationShareType.Live ?
"Insufficient permissions to start sharing your live location" :
"Insufficient permissions to send your location";
const errorMessage = shareType === LocationShareType.Live
? "Insufficient permissions to start sharing your live location"
: "Insufficient permissions to send your location";


const modalParams = {
title: _t("You don't have permission to share locations"),
description: _t("You need to have the right permissions in order to share locations in this room."),
button: _t("OK"),
hasCancelButton: false,
onFinished: () => {}, // NOOP
};
return { modalParams, errorMessage };
};

const getDefaultErrorParams = (shareType: LocationShareType, openMenu: () => void): {
errorMessage: string;
modalParams: IQuestionDialogProps;
} => {
const errorMessage = shareType === LocationShareType.Live ?
"We couldn't start sharing your live location" :
"We couldn't send your location";
logger.error(errorMessage, error);
const params = {
const modalParams = {
title: _t("We couldn't send your location"),
description: _t("%(brand)s could not send your location. Please try again later.", {
brand: SdkConfig.get().brand,
Expand All @@ -62,7 +83,17 @@ const handleShareError = (error: Error, openMenu: () => void, shareType: Locatio
}
},
};
Modal.createDialog(QuestionDialog, params);
return { modalParams, errorMessage };
};

const handleShareError = (error: Error, openMenu: () => void, shareType: LocationShareType): void => {
const { modalParams, errorMessage } = (error as MatrixError).errcode === 'M_FORBIDDEN' ?
getPermissionsErrorParams(shareType) :
getDefaultErrorParams(shareType, openMenu);
Comment on lines +90 to +92
Copy link
Member

Choose a reason for hiding this comment

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

Same here with the ternary


logger.error(errorMessage, error);

Modal.createDialog(QuestionDialog, modalParams);
};

export const shareLiveLocation = (
Expand Down
2 changes: 2 additions & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -2206,6 +2206,8 @@
"Failed to fetch your location. Please try again later.": "Failed to fetch your location. Please try again later.",
"Timed out trying to fetch your location. Please try again later.": "Timed out trying to fetch your location. Please try again later.",
"Unknown error fetching location. Please try again later.": "Unknown error fetching location. Please try again later.",
"You don't have permission to share locations": "You don't have permission to share locations",
"You need to have the right permissions in order to share locations in this room.": "You need to have the right permissions in order to share locations in this room.",
"We couldn't send your location": "We couldn't send your location",
"%(brand)s could not send your location. Please try again later.": "%(brand)s could not send your location. Please try again later.",
"%(displayName)s's live location": "%(displayName)s's live location",
Expand Down
39 changes: 37 additions & 2 deletions test/components/views/location/LocationShareMenu-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import Modal from '../../../../src/Modal';
import { DEFAULT_DURATION_MS } from '../../../../src/components/views/location/LiveDurationDropdown';
import { OwnBeaconStore } from '../../../../src/stores/OwnBeaconStore';
import { SettingLevel } from '../../../../src/settings/SettingLevel';
import QuestionDialog from '../../../../src/components/views/dialogs/QuestionDialog';

jest.useFakeTimers();

Expand Down Expand Up @@ -417,7 +418,7 @@ describe('<LocationShareMenu />', () => {
}));
});

it('opens error dialog when beacon creation fails ', async () => {
it('opens error dialog when beacon creation fails', async () => {
// stub logger to keep console clean from expected error
const logSpy = jest.spyOn(logger, 'error').mockReturnValue(undefined);
const error = new Error('oh no');
Expand All @@ -438,7 +439,41 @@ describe('<LocationShareMenu />', () => {
await flushPromisesWithFakeTimers();

expect(logSpy).toHaveBeenCalledWith("We couldn't start sharing your live location", error);
expect(mocked(Modal).createDialog).toHaveBeenCalled();
expect(mocked(Modal).createDialog).toHaveBeenCalledWith(QuestionDialog, expect.objectContaining({
button: 'Try again',
description: 'Element could not send your location. Please try again later.',
title: `We couldn't send your location`,
cancelButton: 'Cancel',
}));
});

it('opens error dialog when beacon creation fails with permission error', async () => {
// stub logger to keep console clean from expected error
const logSpy = jest.spyOn(logger, 'error').mockReturnValue(undefined);
const error = { errcode: 'M_FORBIDDEN' } as unknown as Error;
mockClient.unstable_createLiveBeacon.mockRejectedValue(error);
const component = getComponent();

// advance to location picker
setShareType(component, LocationShareType.Live);
setLocation(component);

act(() => {
getSubmitButton(component).at(0).simulate('click');
component.setProps({});
});

await flushPromisesWithFakeTimers();
await flushPromisesWithFakeTimers();
await flushPromisesWithFakeTimers();

expect(logSpy).toHaveBeenCalledWith("Insufficient permissions to start sharing your live location", error);
expect(mocked(Modal).createDialog).toHaveBeenCalledWith(QuestionDialog, expect.objectContaining({
button: 'OK',
description: 'You need to have the right permissions in order to share locations in this room.',
title: `You don't have permission to share locations`,
hasCancelButton: false,
}));
});
});
});
Expand Down