Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APM] Adds 7.x migration message to 'no services' view #30811

Merged
merged 4 commits into from
Feb 13, 2019
Merged

[APM] Adds 7.x migration message to 'no services' view #30811

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
07e12dd
Adds 7.x migration message to 'no services message'
jasonrhodes Feb 12, 2019
2fc31b4
Small tweaks and adds a test
jasonrhodes Feb 12, 2019
80d81f6
Removed comment after confirming link url with operations team
jasonrhodes Feb 12, 2019
7617ac4
Updates test descriptions to be more descriptive per review
jasonrhodes Feb 12, 2019
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
79 changes: 79 additions & 0 deletions x-pack/plugins/apm/public/components/app/ServiceOverview/NoServicesMessage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { EuiEmptyPrompt } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { KibanaLink } from '../../shared/Links/KibanaLink';
import { SetupInstructionsLink } from '../../shared/Links/SetupInstructionsLink';

interface Props {
// any data submitted from APM agents found (not just in the given time range)
historicalDataFound: boolean;
}

export function NoServicesMessage({ historicalDataFound }: Props) {
if (historicalDataFound) {
return (
<EuiEmptyPrompt
title={
<div>
{i18n.translate('xpack.apm.servicesTable.notFoundLabel', {
defaultMessage: 'No services found'
})}
</div>
}
titleSize="s"
/>
);
} else {
jasonrhodes marked this conversation as resolved.
Show resolved Hide resolved
return (
<EuiEmptyPrompt
title={
<div>
{i18n.translate('xpack.apm.servicesTable.noServicesLabel', {
defaultMessage: `Looks like you don't have any APM services installed. Let's add some!`
})}
</div>
}
titleSize="s"
body={
<React.Fragment>
<p>
sorenlouv marked this conversation as resolved.
Show resolved Hide resolved
{i18n.translate(
'xpack.apm.servicesTable.7xUpgradeServerMessage',
{
defaultMessage: `Upgrading from a pre-7.x version? Make sure you've also upgraded
your APM server instance(s) to at least 7.0.`
}
)}
</p>
<p>
{i18n.translate('xpack.apm.servicesTable.7xOldDataMessage', {
defaultMessage:
'You may also have old data that needs to be migrated.'
})}{' '}
<KibanaLink
pathname="/app/kibana"
hash="/management/elasticsearch/upgrade_assistant"
>
{i18n.translate(
'xpack.apm.servicesTable.MigrationAssistantLink',
{
defaultMessage:
'Learn more by visiting the Kibana Migration Assistant'
}
)}
</KibanaLink>
.
</p>
</React.Fragment>
}
actions={<SetupInstructionsLink buttonFill={true} />}
/>
);
}
}
21 changes: 21 additions & 0 deletions x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/NoServicesMessage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { shallow } from 'enzyme';
import React from 'react';
import { NoServicesMessage } from '../NoServicesMessage';

describe('No Services Message', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

are these spaces intentional or is it supposed to be the component name (NoServicesMessage)?

it('should render correctly when historical data is found', () => {
Copy link
Member

Choose a reason for hiding this comment

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

What about: should show "no services"-message when historical data was found

const wrapper = shallow(<NoServicesMessage historicalDataFound={true} />);
expect(wrapper).toMatchSnapshot();
Copy link
Member

Choose a reason for hiding this comment

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

This is fine for now, but I think it would be great with a discussion about how we use snapshot.
I think we over-use them a bit. In this case, if the output changes the slightest the test will fail - which causes noise, and it's easy to overlook if something significant actually changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we probably need to write up a clear guide on how we want to do testing and what kinds of testing we want to do in what situations, so that we can refer to it as we develop.

});

it('should render correctly when NO historical data is found', () => {
Copy link
Member

Choose a reason for hiding this comment

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

What about should show link to Getting Started guide, when no historical data is found

const wrapper = shallow(<NoServicesMessage historicalDataFound={false} />);
expect(wrapper).toMatchSnapshot();
});
});
49 changes: 49 additions & 0 deletions ...lic/components/app/ServiceOverview/__test__/__snapshots__/NoServicesMessage.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`No Services Message should render correctly when NO historical data is found 1`] = `
<EuiEmptyPrompt
actions={
<SetupInstructionsLink
buttonFill={true}
/>
}
body={
<React.Fragment>
<p>
Upgrading from a pre-7.x version? Make sure you've also upgraded
your APM server instance(s) to at least 7.0.
</p>
<p>
You may also have old data that needs to be migrated.

<Connect(UnconnectedKibanaLink)
hash="/management/elasticsearch/upgrade_assistant"
pathname="/app/kibana"
>
Learn more by visiting the Kibana Migration Assistant
</Connect(UnconnectedKibanaLink)>
.
</p>
</React.Fragment>
}
iconColor="subdued"
title={
<div>
Looks like you don't have any APM services installed. Let's add some!
</div>
}
titleSize="s"
/>
`;

exports[`No Services Message should render correctly when historical data is found 1`] = `
<EuiEmptyPrompt
iconColor="subdued"
title={
<div>
No services found
</div>
}
titleSize="s"
/>
`;
14 changes: 4 additions & 10 deletions ...public/components/app/ServiceOverview/__test__/__snapshots__/ServiceOverview.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ exports[`Service Overview -> View should render when historical data is found 1`
exports[`Service Overview -> View should render when historical data is found 2`] = `
Object {
"items": Array [],
"noItemsMessage": <EmptyMessage
heading="No services were found"
subheading={null}
"noItemsMessage": <NoServicesMessage
historicalDataFound={true}
/>,
}
`;
Expand All @@ -29,13 +28,8 @@ exports[`Service Overview -> View should render when historical data is not foun
exports[`Service Overview -> View should render when historical data is not found 2`] = `
Object {
"items": Array [],
"noItemsMessage": <EmptyMessage
heading="Looks like you don't have any services with APM installed. Let's add some!"
subheading={
<SetupInstructionsLink
buttonFill={true}
/>
}
"noItemsMessage": <NoServicesMessage
historicalDataFound={false}
/>,
}
`;
29 changes: 7 additions & 22 deletions x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { i18n } from '@kbn/i18n';
import React, { Component } from 'react';
import { RRRRenderResponse } from 'react-redux-request';
import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams';
import { IServiceListItem } from 'x-pack/plugins/apm/server/lib/services/get_services';
import { loadAgentStatus } from '../../../services/rest/apm/status_check';
import { ServiceListRequest } from '../../../store/reactReduxRequest/serviceList';
import { EmptyMessage } from '../../shared/EmptyMessage';
import { SetupInstructionsLink } from '../../shared/Links/SetupInstructionsLink';
import { NoServicesMessage } from './NoServicesMessage';
import { ServiceList } from './ServiceList';

interface Props {
Expand All @@ -21,6 +19,7 @@ interface Props {
}

interface State {
// any data submitted from APM agents found (not just in the given time range)
historicalDataFound: boolean;
}

Expand All @@ -38,24 +37,6 @@ export class ServiceOverview extends Component<Props, State> {

public render() {
const { urlParams } = this.props;
const { historicalDataFound } = this.state;

const noItemsMessage = (
<EmptyMessage
heading={
historicalDataFound
? i18n.translate('xpack.apm.servicesTable.notFoundLabel', {
defaultMessage: 'No services were found'
})
: i18n.translate('xpack.apm.servicesTable.noServicesLabel', {
defaultMessage: `Looks like you don't have any services with APM installed. Let's add some!`
})
}
subheading={
!historicalDataFound ? <SetupInstructionsLink buttonFill /> : null
}
/>
);

// Render method here uses this.props.serviceList instead of received "data" from RRR
// to make it easier to test -- mapStateToProps uses the RRR selector so the data
Expand All @@ -67,7 +48,11 @@ export class ServiceOverview extends Component<Props, State> {
render={() => (
<ServiceList
items={this.props.serviceList.data}
noItemsMessage={noItemsMessage}
noItemsMessage={
<NoServicesMessage
historicalDataFound={this.state.historicalDataFound}
/>
}
/>
)}
/>
Expand Down