[Upgrade Assistant] New Kibana deprecations page

[alisonelizabeth]

This PR updates the Kibana deprecations page in Upgrade Assistant to align with the new design prototype.

How to test

There should be a number of config deprecations that display out of the box. To add a deprecation that supports the "Quick resolve" functionality, follow these steps:

1. Create a PDF report (you can easily do this from Dashboard using the sample data)
2. Issue the following curl command as outlined in [Reporting] Add deprecation notice to the upgrade assistant #104303:

curl --request PUT \
  --url 'http://localhost:9200/.reporting-*/_settings' \
  --header 'Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==' \
  --header 'Content-Type: application/json' \
  --data '{
	"settings": {
		"index.lifecycle.name": null
	}
}'

Deferred work

There are some features that the core team is looking into supporting within the Kibana deprecations service. If/when these are implemented, I will create a follow-up PR for UA.

1. Markdown support for manual steps
2. Requiring the "deprecationType" field when registered a deprecation. This is currently optional, and UA defaults to "uncategorized" when not specified.
3. Requiring a deprecation has a title ([Core deprecations] Add title field to deprecation info #109840). This will allow us to remove the hard-coded title, "XX is using a deprecated feature". Added support via 6773636

Screenshots

Overview of changes

// Quick resolve flyout

// Manual steps only flyout

// Deprecation that has been resolved

// Deprecation with failure

[elasticmachine]

Pinging @elastic/kibana-stack-management (Team:Stack Management)

[alisonelizabeth]

/cc @cuff-links I've also added an accessibility test in x-pack/test/accessibility/apps/upgrade_assistant.ts if you'd like to take a look.

Flaky test runner: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1861/

Direct link to UA test runs: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1861/testReport/X-Pack%20Accessibility%20Tests/x-pack_test_accessibility_apps_upgrade_assistant%C2%B7ts/

[cjcenizal]

Tested locally, code looks great! Had a couple suggestions but nothing blocking.

[cjcenizal]

Do you think it'd make sense to add an assertion before clicking the refresh button, too? To verify that it had an effect?

expect(deprecationService.getAllDeprecations).toHaveBeenCalledTimes(1);
await actions.table.clickRefreshButton();
expect(deprecationService.getAllDeprecations).toHaveBeenCalledTimes(2);

[alisonelizabeth]

👍 good catch

[cjcenizal]

Nit: I'd find this easier to read if we grouped these lines into 2 groups with comments to describe them.

      // Show only critical deprecations
      await actions.searchBar.clickCriticalFilterButton();
      const { rows: criticalRows } = table.getMetaData('kibanaDeprecationsTable');
      expect(criticalRows.length).toEqual(criticalDeprecations.length);

      // Show all deprecations
      await actions.searchBar.clickCriticalFilterButton();
      const { rows: allRows } = table.getMetaData('kibanaDeprecationsTable');
      expect(allRows.length).toEqual(kibanaDeprecationsMockResponse.length);

[cjcenizal]

What do you think of extracting this UI-manipulation into a testBed action? This will simplify the test significantly, making it easier for the reader focus on what it's testing rather than how it's executing the test. If we do that we could probably remove the non-null assertion too unless I'm missing something.

    const filterByConfigType = async () => {
      await actions.searchBar.clickTypeFilterDropdownAt(0);

      // We need to read the document "body" as the filter dropdown options are added there and not inside
      // the component DOM tree.
      const configTypeFilterButton: HTMLButtonElement | null = document.body.querySelector(
        '.euiFilterSelect__items .euiFilterSelectItem'
      );

      await act(async () => {
        configTypeFilterButton!.click();
      });

      component.update();
    };

[cjcenizal]

It might be helpful to explain in the comment that we expect the first item in the list to be the "Config" option.

[cjcenizal]

Can we add a test for the loading state too?

[alisonelizabeth]

This is a bit tricky to test. I verified the loading state manually. I'm going to defer for now, and may need to reach out to Seb for the best way to handle.

[cjcenizal]

These are awesome. I'll write my error-handling tests in a separate file, too.

[cjcenizal]

Not sure why but when I have just "Config" and "Uncategorized" rows, the order remains the same regardless of which direction I sort on this column.

[alisonelizabeth]

Good catch! Fixed.

[alisonelizabeth]

The implementation will also be improved once #111070 is addressed.

[cjcenizal]

Can we use a switch/case instead?

[cjcenizal]

Nice trick!

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 2eeffaf
• Storybooks Preview

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
upgradeAssistant	133	138	+5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
upgradeAssistant	206.5KB	208.7KB	+2.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
upgradeAssistant	27.2KB	27.2KB	+8.0B

History

• 💚 Build #150364 succeeded 6773636
• 💚 Build #150323 succeeded 4c5dd0e
• 💚 Build #150177 succeeded bb7ac78
• 💔 Build #150025 failed 4634f90
• 💔 Build #149930 failed 4a7d189

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[dborodyansky]

Looks great. Tested locally but without encountering Quick Resolve action, for which screenshots look good. Assuming that firing Quick Resolve button transitions to isLoading button state.

[cjcenizal]

Do you think it might make sense to add setLoadKibanaDeprecations functions to http_requests.ts that encapsulate this logic?

I understand the internal implementation would be inconsistent because for the other functions we're stubbing responses from the server, and in this case we'd use the deprecation service mock. But I think we could add a comment to explain why, and from the consumer's perspective the interface would be consistent, so maybe it's a nice DX improvement. Totally up to you.

[alisonelizabeth]

Thanks for the suggestion! I think that could be helpful. I'll look into this separately, as I think the overview tests could benefit from this as well.