Skip to content

Commit

Permalink
bugfix: Do not show stale errors when a user rejoins a call (#2549)
Browse files Browse the repository at this point in the history
  • Loading branch information
prprabhu-ms authored Nov 23, 2022
1 parent 78c04a3 commit a1fd00b
Show file tree
Hide file tree
Showing 14 changed files with 111 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Ignore errors from previous call when surfacing errors to composite UI",
"packageName": "@azure/communication-react",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Add a prop to ErrorBar component to ignore old errors",
"packageName": "@azure/communication-react",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Ignore errors from previous call when surfacing errors to composite UI",
"packageName": "@azure/communication-react",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Add a prop to ErrorBar component to ignore old errors",
"packageName": "@azure/communication-react",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -2088,6 +2088,7 @@ export const ErrorBar: (props: ErrorBarProps) => JSX.Element;
// @public
export interface ErrorBarProps extends IMessageBarProps {
activeErrorMessages: ActiveErrorMessage[];
ignorePremountErrors?: boolean;
strings?: ErrorBarStrings;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1602,6 +1602,7 @@ export const ErrorBar: (props: ErrorBarProps) => JSX.Element;
// @public
export interface ErrorBarProps extends IMessageBarProps {
activeErrorMessages: ActiveErrorMessage[];
ignorePremountErrors?: boolean;
strings?: ErrorBarStrings;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ export const ErrorBar: (props: ErrorBarProps) => JSX.Element;
// @public
export interface ErrorBarProps extends IMessageBarProps {
activeErrorMessages: ActiveErrorMessage[];
ignorePremountErrors?: boolean;
strings?: ErrorBarStrings;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ export const ErrorBar: (props: ErrorBarProps) => JSX.Element;
// @public
export interface ErrorBarProps extends IMessageBarProps {
activeErrorMessages: ActiveErrorMessage[];
ignorePremountErrors?: boolean;
strings?: ErrorBarStrings;
}

Expand Down
48 changes: 45 additions & 3 deletions packages/react-components/src/components/ErrorBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import React from 'react';
import { act } from 'react-dom/test-utils';
import { initializeIcons, MessageBar } from '@fluentui/react';
import { ActiveErrorMessage, ErrorBar } from './ErrorBar';
import { ActiveErrorMessage, ErrorBar, ErrorBarProps } from './ErrorBar';
import Enzyme, { ReactWrapper, mount } from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';

Expand Down Expand Up @@ -110,6 +110,11 @@ describe('ErrorBar dismissal for errors without timestamp', () => {
});

describe('ErrorBar dismissal with multiple errors', () => {
beforeAll(() => {
Enzyme.configure({ adapter: new Adapter() });
initializeIcons();
});

it('clearing an error with multiple errors leaves other errors untouched', () => {
const root = mountErrorBarWithDefaults();
setActiveErrors(root, [{ type: 'accessDenied', timestamp: new Date(Date.now()) }, { type: 'muteGeneric' }]);
Expand All @@ -119,10 +124,47 @@ describe('ErrorBar dismissal with multiple errors', () => {
});
});

const mountErrorBarWithDefaults = (): ReactWrapper => {
describe('ErrorBar handling of errors from previous call or chat', () => {
beforeAll(() => {
Enzyme.configure({ adapter: new Adapter() });
initializeIcons();
});

it('shows all old errors by default', () => {
const oldErrors: ActiveErrorMessage[] = [
// Make sure old error is in the past.
{ type: 'accessDenied', timestamp: new Date(Date.now() - 10) },
{ type: 'muteGeneric' }
];
const root = mountErrorBarWithDefaults();
setActiveErrors(root, oldErrors);
expect(messageBarCount(root)).toBe(2);
});

it('does not show old errors with timestamp when ignorePremountErrors is set', () => {
// Make sure old error is in the past.
const oldErrors: ActiveErrorMessage[] = [{ type: 'accessDenied', timestamp: new Date(Date.now() - 10) }];
const root = mountErrorBarWithDefaults({ ignorePremountErrors: true });
setActiveErrors(root, oldErrors);
expect(messageBarCount(root)).toBe(0);
});

it('shows old errors without timestamp when ignorePremountErrors is set', () => {
const oldErrors: ActiveErrorMessage[] = [{ type: 'muteGeneric' }];
const root = mountErrorBarWithDefaults({ ignorePremountErrors: true });
setActiveErrors(root, oldErrors);
expect(messageBarCount(root)).toBe(1);
});
});

const mountErrorBarWithDefaults = (props?: Partial<ErrorBarProps>): ReactWrapper => {
const mergedProps: ErrorBarProps = {
activeErrorMessages: [],
...(props ?? {})
};
let root;
act(() => {
root = mount(<ErrorBar activeErrorMessages={[]} />);
root = mount(<ErrorBar {...mergedProps} />);
});
return root;
};
Expand Down
23 changes: 21 additions & 2 deletions packages/react-components/src/components/ErrorBar.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import React, { useEffect, useState } from 'react';
import React, { useEffect, useRef, useState } from 'react';
import { IMessageBarProps, MessageBar, Stack } from '@fluentui/react';
import { useLocale } from '../localization';
import {
Expand Down Expand Up @@ -31,6 +31,17 @@ export interface ErrorBarProps extends IMessageBarProps {
* Currently active errors.
*/
activeErrorMessages: ActiveErrorMessage[];

/**
* If set, errors with {@link ActiveErrorMessage.timestamp} older than the time this component is mounted
* are not shown.
*
* This is useful when using the {@link ErrorBar} with a stateful client that handles more than one call
* or chat thread. Set this prop to ignore errors from previous call or chat.
*
* @defaultValue false
*/
ignorePremountErrors?: boolean;
}

/**
Expand Down Expand Up @@ -234,6 +245,10 @@ export const ErrorBar = (props: ErrorBarProps): JSX.Element => {
const localeStrings = useLocale().strings.errorBar;
const strings = props.strings ?? localeStrings;

// Timestamp for when this comopnent is first mounted.
// Never updated through the lifecycle of this component.
const mountTimestamp = useRef(new Date(Date.now()));

const [dismissedErrors, setDismissedErrors] = useState<DismissedError[]>([]);

// dropDismissalsForInactiveErrors only returns a new object if `dismissedErrors` actually changes.
Expand All @@ -243,7 +258,11 @@ export const ErrorBar = (props: ErrorBarProps): JSX.Element => {
[props.activeErrorMessages, dismissedErrors]
);

const toShow = errorsToShow(props.activeErrorMessages, dismissedErrors);
const toShow = errorsToShow(
props.activeErrorMessages,
dismissedErrors,
props.ignorePremountErrors ? mountTimestamp.current : undefined
);

return (
<Stack data-ui-id="error-bar-stack">
Expand Down
8 changes: 7 additions & 1 deletion packages/react-components/src/components/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,20 @@ export const dropDismissalsForInactiveErrors = (
*/
export const errorsToShow = (
activeErrorMessages: ActiveErrorMessage[],
dismissedErrors: DismissedError[]
dismissedErrors: DismissedError[],
mountTimestamp?: Date
): ActiveErrorMessage[] => {
const dismissed: Map<ErrorType, DismissedError> = new Map();
for (const error of dismissedErrors) {
dismissed.set(error.type, error);
}

return activeErrorMessages.filter((error) => {
if (mountTimestamp && error.timestamp && mountTimestamp > error.timestamp) {
// Error has a timestamp and it is older than when the component was mounted.
return false;
}

const dismissal = dismissed.get(error.type);
if (!dismissal) {
// This error was never dismissed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ export const CallPage = (props: CallPageProps): JSX.Element => {
return (
<CallArrangement
complianceBannerProps={{ ...complianceBannerProps, strings }}
errorBarProps={options?.errorBar !== false && { ...errorBarProps }}
// Ignore errors from before current call. This avoids old errors from showing up when a user re-joins a call.
errorBarProps={options?.errorBar !== false && { ...errorBarProps, ignorePremountErrors: true }}
mutedNotificationProps={mutedNotificationProps}
callControlProps={{
callInvitationURL: callInvitationURL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ export const HoldPage = (props: HoldPageProps): JSX.Element => {
return (
<CallArrangement
complianceBannerProps={{ strings }}
errorBarProps={props.options?.errorBar !== false && { ...errorBarProps }}
// Ignore errors from before current call. This avoids old errors from showing up when a user re-joins a call.
errorBarProps={props.options?.errorBar !== false && { ...errorBarProps, ignorePremountErrors: true }}
callControlProps={{
options: callControlOptions,
increaseFlyoutItemSize: props.mobileView
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ export const LobbyPage = (props: LobbyPageProps): JSX.Element => {
return (
<CallArrangement
complianceBannerProps={{ strings }}
errorBarProps={props.options?.errorBar !== false && { ...errorBarProps }}
// Ignore errors from before current call. This avoids old errors from showing up when a user re-joins a call.
errorBarProps={props.options?.errorBar !== false && { ...errorBarProps, ignorePremountErrors: true }}
callControlProps={{
options: callControlOptions,
increaseFlyoutItemSize: props.mobileView
Expand Down

0 comments on commit a1fd00b

Please sign in to comment.