Skip to content

Commit

Permalink
Integ 231 improve error handling within analytics app component (#2919)
Browse files Browse the repository at this point in the history
* Fix overflow of error messages in Note component

* Adjust empty data message

* Add HyperLink component with tests

* Adjust tests in SlugWarningDisplay component

* Add ErrorDisplay component and adjust error types

* Lint fixes

* Remove console

* Some copy changes

* Add spec to ErrorDisplay

* Fix linter

* Fix typing of HyperLink component

* Small semantic changes

* Cleaned-up variables declarations

* Handle errors other than explicit API types and fix width of note banner

* Change variable name

* Update copies

* adjust copies and add hyperlinks to contact support

* Refactor to avoid JSX elements in state as per PR review

* Add handling for other errors

* Remove console.error

* Set error to null when successfully retreiving data
  • Loading branch information
lilbitner authored Mar 28, 2023
1 parent 90084ec commit 71f9e56
Show file tree
Hide file tree
Showing 15 changed files with 376 additions and 64 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import HyperLink from './HyperLink';
import { render, screen } from '@testing-library/react';

const BODY = 'Have a great day!';
const SUBSTRING = 'great';

const { getByTestId } = screen;

describe('HyperLink component', () => {
it('can render the TextLink component in place of substring value', () => {
render(<HyperLink body={BODY} substring={SUBSTRING} />);

const textLink = getByTestId('cf-ui-text-link');

expect(textLink).toBeVisible();
expect(textLink.firstChild).toHaveTextContent(SUBSTRING);
});

it('can handle onClick of link', () => {
const mockOnClick = jest.fn();
render(<HyperLink body={BODY} substring={SUBSTRING} onClick={mockOnClick} />);

const textLink = getByTestId('cf-ui-text-link');
textLink.click();
expect(mockOnClick).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import React, { MouseEventHandler } from 'react';
import { TextLink } from '@contentful/f36-components';

interface Props {
body: string;
substring: string;
onClick?: MouseEventHandler<HTMLAnchorElement>;
hyperLinkHref?: string;
}

const HyperLink = (props: Props) => {
const { body, substring, onClick = () => {}, hyperLinkHref } = props;
const link = (
<TextLink onClick={onClick} href={hyperLinkHref} target="_blank" rel="noopener noreferer">
{substring}
</TextLink>
);

const formatLink = () => {
const bodyWithTextLink = body.split(substring).reduce((prev: any, current, i) => {
if (!i) {
return [current];
}
return prev.concat(link, current);
}, []);
return bodyWithTextLink as JSX.Element;
};

return formatLink();
};

export default HyperLink;
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { css } from 'emotion';
import tokens from '@contentful/f36-tokens';

export const styles = {
note: css({
overflow: 'hidden',
marginBottom: tokens.spacingM,
wordBreak: 'break-word',
width: '100%',
}),
noteContent: css({
textOverflow: 'ellipsis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { Api } from 'apis/api';
import { mockSdk, mockCma, validServiceKeyId } from '../../../../test/mocks';
import runReportResponseHasViews from '../../../../../lambda/public/sampleData/runReportResponseHasViews.json';
import runReportResponseNoView from '../../../../../lambda/public/sampleData/runReportResponseNoViews.json';
import { getContentTypeSpecificMsg } from '../constants/noteMessages';
import {
EMPTY_DATA_MSG,
getContentTypeSpecificMsg,
} from 'components/main-app/constants/noteMessages';
import * as useSidebarSlug from 'hooks/useSidebarSlug/useSidebarSlug';

jest.mock('@contentful/react-apps-toolkit', () => ({
Expand Down Expand Up @@ -65,7 +68,7 @@ describe('AnalyticsApp with correct content types configured', () => {

const dropdown = await findByTestId(SELECT_TEST_ID);
const warningNote = getByTestId(NOTE_TEST_ID);
const noteText = getByText('There are no page views to show for this range.');
const noteText = getByText(EMPTY_DATA_MSG);

expect(dropdown).toBeVisible();
expect(warningNote).toBeVisible();
Expand Down Expand Up @@ -105,11 +108,14 @@ describe('AnalyticsApp when content types are not configured correctly', () => {
isContentTypeWarning: true,
}));
mockApi.mockImplementation(() => runReportResponseHasViews);
const warningMessage = getContentTypeSpecificMsg('Category')
.noSlugContentMsg.replace('app configuration page.', '')
.trim();
renderAnalyticsApp();

const dropdown = queryByTestId(SELECT_TEST_ID);
const warningNote = await findByTestId(NOTE_TEST_ID);
const noteText = getByText(getContentTypeSpecificMsg('Category', true).noSlugContentMsg.trim());
const noteText = getByText(warningMessage);

expect(dropdown).toBeFalsy();
expect(warningNote).toBeVisible();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const AnalyticsApp = (props: Props) => {
try {
const reportData = await api.runReports(reportRequestParams);
setRunReportResponse(reportData);
setError(undefined);
} catch (e) {
setError(e as Error);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import ChartContent from './ChartContent';
import { render, screen } from '@testing-library/react';
import { mockSdk } from '../../../../test/mocks';
import runReportResponseHasViews from '../../../../../lambda/public/sampleData/runReportResponseHasViews.json';
import runReportResponseNoView from '../../../../../lambda/public/sampleData/runReportResponseNoViews.json';
import { EMPTY_DATA_MSG } from '../constants/noteMessages';

jest.mock('@contentful/react-apps-toolkit', () => ({
useSDK: () => mockSdk,
}));

const { getByText } = screen;

describe('ChartContent component', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { Row, RunReportResponse } from 'types';
import Note from 'components/common/Note/Note';
import LineChart from 'components/main-app/LineChart/LineChart';
import { parseDayAndMonth } from 'helpers/DateHelpers/DateHelpers';
import { DEFAULT_ERR_MSG, EMPTY_DATA_MSG } from '../constants/noteMessages';
import ErrorDisplay from 'components/main-app/ErrorDisplay/ErrorDisplay';
import { EMPTY_DATA_MSG } from '../constants/noteMessages';
import { styles } from './ChartContent.styles';

interface Props {
Expand All @@ -27,7 +28,7 @@ const ChartContent = (props: Props) => {

const renderChartContent = () => {
if (error) {
return <Note body={error?.message || DEFAULT_ERR_MSG} variant="negative" />;
return <ErrorDisplay error={error} />;
}

if (!pageViewData.rowCount) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { css } from 'emotion';
import tokens from '@contentful/f36-tokens';

export const styles = {
root: css({
marginTop: tokens.spacing2Xs,
marginRight: tokens.spacing2Xs,
}),
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useState } from 'react';
import { Box, DisplayText, Flex, Paragraph, Select } from '@contentful/f36-components';
import { DateRangeType } from 'types';
import { styles } from './ChartHeader.styles';

interface Props {
metricName: string;
Expand Down Expand Up @@ -39,7 +40,12 @@ const ChartHeader = (props: Props) => {
</DisplayText>
<Paragraph marginBottom="none">{getMetricDisplayString(metricName)}</Paragraph>
</Box>
<Select id="daterange" name="daterange" value={dateSelection} onChange={handleOnChange}>
<Select
className={styles.root}
id="daterange"
name="daterange"
value={dateSelection}
onChange={handleOnChange}>
<Select.Option value="lastDay">Last 24 hours</Select.Option>
<Select.Option value="lastWeek">Last 7 days</Select.Option>
<Select.Option value="lastMonth">Last 28 days</Select.Option>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React from 'react';
import { useSDK } from '@contentful/react-apps-toolkit';
import { SidebarExtensionSDK } from '@contentful/app-sdk';
import { DEFAULT_ERR_MSG } from 'components/main-app/constants/noteMessages';
import HyperLink from 'components/common/HyperLink/HyperLink';

interface AppConfigPageHyperLinkProps {
bodyMsg: string;
}

export const AppConfigPageHyperLink = (props: AppConfigPageHyperLinkProps) => {
const { bodyMsg } = props;
const sdk = useSDK<SidebarExtensionSDK>();
const openConfigPage = () => sdk.navigator.openAppConfig();
return <HyperLink body={bodyMsg} substring="app configuration page." onClick={openConfigPage} />;
};

export const SupportHyperLink = () => (
<HyperLink
body={DEFAULT_ERR_MSG}
substring="contact support."
hyperLinkHref="https://www.contentful.com/support/?utm_source=webapp&utm_medium=help-menu&utm_campaign=in-app-help"
/>
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import ErrorDisplay from './ErrorDisplay';
import { render, screen } from '@testing-library/react';
import { mockSdk } from '../../../../test/mocks';
import {
DEFAULT_ERR_MSG,
INVALID_ARGUMENT_MSG,
INVALID_SERVICE_ACCOUNT,
PERMISSION_DENIED_MSG,
} from '../constants/noteMessages';
import { ApiError } from 'apis/api';

jest.mock('@contentful/react-apps-toolkit', () => ({
useSDK: () => mockSdk,
}));

const { findByText, getByTestId } = screen;

describe('ErrorDisplay', () => {
it('mounts with correct msg and hyperlink when error is of type InvalidProperty', async () => {
const HYPER_LINK_COPY = 'app configuration page.';
render(
<ErrorDisplay
error={
new ApiError({
details: '',
message: '',
status: 404,
errorType: 'InvalidProperty',
})
}
/>
);

const warningMsg = await findByText(INVALID_ARGUMENT_MSG.replace(HYPER_LINK_COPY, '').trim());
const hyperLink = getByTestId('cf-ui-text-link');

expect(warningMsg).toBeVisible();
expect(hyperLink).toBeVisible();
});

it('mounts with correct msg when error is of type DisabledDataApi', async () => {
render(
<ErrorDisplay
error={
new ApiError({
details: '',
message: '',
status: 404,
errorType: 'DisabledDataApi',
})
}
/>
);

const warningMsg = await findByText(PERMISSION_DENIED_MSG);

expect(warningMsg).toBeVisible();
});

it('mounts with correct msg when error is of type FailedFetch', async () => {
const HYPER_LINK_COPY = 'contact support.';
render(
<ErrorDisplay
error={
new ApiError({
details: '',
message: '',
status: 404,
errorType: 'FailedFetch',
})
}
/>
);

const warningMsg = await findByText(DEFAULT_ERR_MSG.replace(HYPER_LINK_COPY, '').trim());
const hyperLink = getByTestId('cf-ui-text-link');

expect(warningMsg).toBeVisible();
expect(hyperLink).toBeVisible();
});

it('mounts with correct msg when error is of type InvalidServiceAccount', async () => {
const HYPER_LINK_COPY = 'app configuration page.';
render(
<ErrorDisplay
error={
new ApiError({
details: '',
message: '',
status: 404,
errorType: 'InvalidServiceAccount',
})
}
/>
);

const warningMsg = await findByText(
INVALID_SERVICE_ACCOUNT.replace(HYPER_LINK_COPY, '').trim()
);
const hyperLink = getByTestId('cf-ui-text-link');

expect(warningMsg).toBeVisible();
expect(hyperLink).toBeVisible();
});

it('mounts with correct msg when error is of ApiError class but not an Error type explicitely handled', async () => {
const INTERNAL_ERR_MSG = 'Internal Server Error';
render(
<ErrorDisplay
error={
new ApiError({
details: '',
message: INTERNAL_ERR_MSG,
status: 500,
errorType: '',
})
}
/>
);

const warningMsg = await findByText(INTERNAL_ERR_MSG);

expect(warningMsg).toBeVisible();
});

it('mounts with correct msg when error is not a specified Api Error Type ', async () => {
const ERR_MSG = 'random error';
render(<ErrorDisplay error={new Error(ERR_MSG)} />);

const warningMsg = await findByText(ERR_MSG);

expect(warningMsg).toBeVisible();
});
});
Loading

0 comments on commit 71f9e56

Please sign in to comment.