[APM] Adds 7.x migration message to 'no services' view #30811
Conversation
jasonrhodes
requested review from
formgeist,
sorenlouv and
makwarth
and removed request for
formgeist
x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/apm/public/components/app/ServiceOverview/NoServicesMessage.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/apm/public/components/app/ServiceOverview/NoServicesMessage.tsx
Show resolved
Hide resolved
| 'You may also have old data that needs to be migrated.' | ||
| })}{' '} | ||
| {/* TODO: GET REAL URL FOR KIBANA MIGRATION ASSISTANT */} | ||
| <KibanaLink pathname="/app/kibana" hash="/management"> |
There was a problem hiding this comment.
I think this is the url: /app/kibana#/management/elasticsearch/upgrade_assistant.
Unfortunately we cannot link to the "Indices" tab: #29845
There was a problem hiding this comment.
Hm I saw that in the management section but it says 8.0 Upgrade Assistant even when I'm on the 7.0 branch ... I'm not sure who to confirm this with
There was a problem hiding this comment.
@tylersmalley would be the one to ask.
There was a problem hiding this comment.
Confirmed with the operations team that this is the correct URL. Will remove that comment and push.
x-pack/plugins/apm/public/components/app/ServiceOverview/NoServicesMessage.tsx
Show resolved
Hide resolved
|
Made some review tweaks and added a test for the new component. Will check with @tylersmalley about the migration assistant link. |
💚 Build Succeeded |
| describe('No Services Message', () => { | ||
| it('should render correctly when historical data is found', () => { | ||
| const wrapper = shallow(<NoServicesMessage historicalDataFound={true} />); | ||
| expect(wrapper).toMatchSnapshot(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| expect(wrapper).toMatchSnapshot(); | ||
| }); | ||
|
|
||
| it('should render correctly when NO historical data is found', () => { |
There was a problem hiding this comment.
What about should show link to Getting Started guide, when no historical data is found
| import { NoServicesMessage } from '../NoServicesMessage'; | ||
|
|
||
| describe('No Services Message', () => { | ||
| it('should render correctly when historical data is found', () => { |
There was a problem hiding this comment.
What about: should show "no services"-message when historical data was found
| import React from 'react'; | ||
| import { NoServicesMessage } from '../NoServicesMessage'; | ||
|
|
||
| describe('No Services Message', () => { |
There was a problem hiding this comment.
are these spaces intentional or is it supposed to be the component name (NoServicesMessage)?
|
retest |
|
There were comments throughout, but just a follow-up |
|
retest |
LeeDr
added
Team:APM
|
Pinging @elastic/apm-ui |
💚 Build Succeeded |
jasonrhodes
deleted the
7.0-migration-messaging/apm-30274-master-cherry
branch
* Adds 7.x migration message to 'no services message' * Small tweaks and adds a test * Removed comment after confirming link url with operations team * Updates test descriptions to be more descriptive per review
* Adds 7.x migration message to 'no services message' * Small tweaks and adds a test * Removed comment after confirming link url with operations team * Updates test descriptions to be more descriptive per review
Closes #30274 (need to move APM server check/copy update from that ticket into separate ticket)
Summary
Adds a new message to the APM services list when no data is found, encouraging users who are upgrading from pre-7.x versions to upgrade APM server and possibly migrate data.
NOTE: Still need to track down the "Kibana Migration Assistant" link path....