[Enterprise Search] Migrate shared Indexing Status component by scottybollinger · Pull Request #84571 · elastic/kibana

scottybollinger · 2020-11-30T18:35:55Z

Summary

Migrate shared IndexingStatus components used by both App Search and Workplace Search.

The IndexingStatusFetcher component is a WIP, as there is no way to test it in the browser with out the components that use it. I have migrated the way I think it will work and will wait to backport this to 7.x until this has been verified once the Workplace Search Schema component, which depends on this, has been migrated.

Finally, the IndexingStatusPopover component is not being migrated as a part of this PR because it is only used in App Search and cannot be tested.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

This is needed to animate the loading progress bar in the Enterprise Search schema views

This is a straight copy/paste with only linting changes and tests added

This is a copy/paste with linting changes and tests added. Also changed out the Link component to our EuiLinkTo component for internal routing

This is a straight copy/paste with only linting changes and tests added

This is a copy/paste with some modifications. The http/axios code has been removed in favor of the HTTPLogic in Kibana. This is a WIP that I am merging to master until I can get it working in the UI. Without either Schema component in the UIs for App Search or Workplace Search this is only a POC. Will not backport until this is verified working and have written tests.

yakhinvadim · 2020-11-30T19:56:26Z

package.json

    "react-intl": "^2.8.0",
    "react-is": "^16.8.0",
    "react-moment-proptypes": "^1.7.0",
+    "react-motion": "^0.5.2",


I don't think we should bring this library to Kibana. The latest version was released 3 years ago. The library seems to be abandoned: chenglou/react-motion#569

This reverts commit 92db929.

x-pack/plugins/enterprise_search/public/applications/shared/indexing_status/constants.ts

cee-chen · 2020-11-30T21:09:35Z

...ins/enterprise_search/public/applications/shared/indexing_status/indexing_status_fetcher.tsx

+  }, []);
+
+  return children(indexingStatus.percentageComplete, indexingStatus.numDocumentsWithErrors);
+};


Do we plan on writing a test for this file?

Also, just curious - not a blocking request for this PR of course (unless people really like the idea), but at some point do we think it would be worth it to rewrite this file as a Kea logic file instead of as a render prop, and have this be an actions.fetchIndexingStatus action that updates values.percentageComplete and values.numDocumentsWithErrors?

Doing so has the advantage of not requiring useRef for pollingInterval, for example.

Yeah, I've been toying with doing that but wondered if I had the time. I think I'll just go ahead and do it. I want to get this merged in so I can test it in Kibana though. Everything but this file has tests and my changes will be a fast-follower to this PR

Sweet, sounds good - thanks Scotty!

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

cee-chen

Sorry for jumping around so scatteredly today, these are my last set of comments. They're not blockers though so if you would strongly rather tackle CSS classes later, just LMK

cee-chen · 2020-11-30T22:10:51Z

...gins/enterprise_search/public/applications/shared/indexing_status/indexing_status_errors.tsx

+
+export const IndexingStatusErrors: React.FC<IIndexingStatusErrorsProps> = ({ viewLinkPath }) => (
+  <EuiCallOut
+    className="c-stui-indexing-status-errors"


Are we using these CSS classes? If not, let's lose them for now and re-add them as needed and w/ correct casing

Suggested change

className="c-stui-indexing-status-errors"

cee-chen · 2020-11-30T22:13:38Z

x-pack/plugins/enterprise_search/public/applications/shared/indexing_status/indexing_status.tsx

+  return (
+    <IndexingStatusFetcher {...props}>
+      {(percentageComplete, numDocumentsWithErrors) => (
+        <div className="c-stui-indexing-status-wrapper">


Same question here re: CSS classes

Suggested change

<div className="c-stui-indexing-status-wrapper">

<div>

cee-chen · 2020-11-30T22:16:16Z

x-pack/plugins/enterprise_search/public/applications/shared/indexing_status/indexing_status.tsx

+    <IndexingStatusFetcher {...props}>
+      {(percentageComplete, numDocumentsWithErrors) => (
+        <div className="c-stui-indexing-status-wrapper">
+          {percentageComplete < 100 && statusPanel(percentageComplete)}


Very minor comment, but I don't see the advantage of pulling out statusPanel into a separate const since it's not repeated or anything - would suggest just leaving it inline

Suggested change

{percentageComplete < 100 && statusPanel(percentageComplete)}

{percentageComplete < 100 && (

<EuiPanel paddingSize="l" hasShadow>

<IndexingStatusContent percentageComplete={percentComplete} />

</EuiPanel>

)}

Also, do we just show nothing if percentageComplete is 100 with no errors? Like, we don't even show a completed bar?

- Remove stui classes - Inline status

cee-chen

Thanks Scotty, sorry for the many minor comments!

scottybollinger · 2020-12-01T18:44:41Z

Thanks Scotty, sorry for the many minor comments!

Thanks for the review! I'm going to ping you on the other PR as well, the one that adds the logic for IndexingStatus and tests. Hope to have that this week but definitely by next week. Will backport this along with that one, once completed.

kibanamachine · 2020-12-01T20:00:05Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: db239d7

Metrics [docs]

✅ unchanged

History

💚 Build #90885 succeeded beb9589
💚 Build #90872 succeeded e34f038
💚 Build #90836 succeeded 730ac38

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

…#84571) * Add react-motion package This is needed to animate the loading progress bar in the Enterprise Search schema views * Add shared interface * Migrate IndexingStatusContent component This is a straight copy/paste with only linting changes and tests added * Migrate IndexingStatusErrors component This is a copy/paste with linting changes and tests added. Also changed out the Link component to our EuiLinkTo component for internal routing * Migrate IndexingStatus component This is a straight copy/paste with only linting changes and tests added * Migrate IndexingStatusFetcher component This is a copy/paste with some modifications. The http/axios code has been removed in favor of the HTTPLogic in Kibana. This is a WIP that I am merging to master until I can get it working in the UI. Without either Schema component in the UIs for App Search or Workplace Search this is only a POC. Will not backport until this is verified working and have written tests. * Add i18n * Revert "Add react-motion package" This reverts commit 92db929. * Remove motion dependency * Update copy Co-authored-by: Constance <constancecchen@users.noreply.github.com> * Refactor per code review - Remove stui classes - Inline status Co-authored-by: Constance <constancecchen@users.noreply.github.com>

…#84706) * Add react-motion package This is needed to animate the loading progress bar in the Enterprise Search schema views * Add shared interface * Migrate IndexingStatusContent component This is a straight copy/paste with only linting changes and tests added * Migrate IndexingStatusErrors component This is a copy/paste with linting changes and tests added. Also changed out the Link component to our EuiLinkTo component for internal routing * Migrate IndexingStatus component This is a straight copy/paste with only linting changes and tests added * Migrate IndexingStatusFetcher component This is a copy/paste with some modifications. The http/axios code has been removed in favor of the HTTPLogic in Kibana. This is a WIP that I am merging to master until I can get it working in the UI. Without either Schema component in the UIs for App Search or Workplace Search this is only a POC. Will not backport until this is verified working and have written tests. * Add i18n * Revert "Add react-motion package" This reverts commit 92db929. * Remove motion dependency * Update copy Co-authored-by: Constance <constancecchen@users.noreply.github.com> * Refactor per code review - Remove stui classes - Inline status Co-authored-by: Constance <constancecchen@users.noreply.github.com> Co-authored-by: Constance <constancecchen@users.noreply.github.com>

* master: [Lens] Show color in flyout instead of auto (elastic#84532) [Lens] Use index pattern through service instead of reading saved object (elastic#84432) Make it possible to use Kibana anonymous authentication provider with ES anonymous access. (elastic#84074) TelemetryCollectionManager: Use X-Pack strategy as an OSS overwrite (elastic#84477) migrate away from rest_total_hits_as_int (elastic#84508) [Input Control] Custom renderer (elastic#84423) Attempt to more granularly separate App Search vs Workplace Search vs shared GitHub notifications (elastic#84713) [Security Solutino][Case] Case connector alert UI (elastic#82405) [Maps] Support runtime fields in tooltips (elastic#84377) [CCR] Fix row actions in follower index and auto-follow pattern tables (elastic#84433) [Enterprise Search] Migrate shared Indexing Status component (elastic#84571) [maps] remove fields from index-pattern test artifacts (elastic#84379) Add routes for use in Sources Schema (elastic#84579) Changes UI links for drilldowns (elastic#83971) endpoint telemetry cloned endpoint tests (elastic#81498) [Fleet] Handler api key creation errors when Fleet Admin is invalid (elastic#84576)

* master: (72 commits) Make alert status fetching more resilient (elastic#84676) [APM] Refactor hooks and context (elastic#84615) Added word break styles to the texts in the item details card. (elastic#84654) [Search] Disable "send to background" when auto-refresh is enabled (elastic#84106) Add readme for new palette service (elastic#84512) Make all providers to preserve original URL when session expires. (elastic#84229) [Lens] Show color in flyout instead of auto (elastic#84532) [Lens] Use index pattern through service instead of reading saved object (elastic#84432) Make it possible to use Kibana anonymous authentication provider with ES anonymous access. (elastic#84074) TelemetryCollectionManager: Use X-Pack strategy as an OSS overwrite (elastic#84477) migrate away from rest_total_hits_as_int (elastic#84508) [Input Control] Custom renderer (elastic#84423) Attempt to more granularly separate App Search vs Workplace Search vs shared GitHub notifications (elastic#84713) [Security Solutino][Case] Case connector alert UI (elastic#82405) [Maps] Support runtime fields in tooltips (elastic#84377) [CCR] Fix row actions in follower index and auto-follow pattern tables (elastic#84433) [Enterprise Search] Migrate shared Indexing Status component (elastic#84571) [maps] remove fields from index-pattern test artifacts (elastic#84379) Add routes for use in Sources Schema (elastic#84579) Changes UI links for drilldowns (elastic#83971) ...

scottybollinger added 7 commits November 30, 2020 11:12

Add react-motion package

92db929

This is needed to animate the loading progress bar in the Enterprise Search schema views

Add shared interface

d435cf4

Migrate IndexingStatusContent component

6822405

This is a straight copy/paste with only linting changes and tests added

Migrate IndexingStatusErrors component

569eb59

This is a copy/paste with linting changes and tests added. Also changed out the Link component to our EuiLinkTo component for internal routing

Migrate IndexingStatus component

7661303

This is a straight copy/paste with only linting changes and tests added

Add i18n

730ac38

scottybollinger added Feature:Plugins v8.0.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Nov 30, 2020

scottybollinger requested a review from a team November 30, 2020 18:35

yakhinvadim reviewed Nov 30, 2020

View reviewed changes

scottybollinger added 2 commits November 30, 2020 14:28

Revert "Add react-motion package"

e0411e3

This reverts commit 92db929.

Remove motion dependency

e34f038

cee-chen reviewed Nov 30, 2020

View reviewed changes

x-pack/plugins/enterprise_search/public/applications/shared/indexing_status/constants.ts Outdated Show resolved Hide resolved

cee-chen reviewed Nov 30, 2020

View reviewed changes

Update copy

beb9589

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

cee-chen reviewed Nov 30, 2020

View reviewed changes

Refactor per code review

db239d7

- Remove stui classes - Inline status

cee-chen approved these changes Dec 1, 2020

View reviewed changes

scottybollinger merged commit ae9df69 into elastic:master Dec 1, 2020

scottybollinger deleted the scottybollinger/shared-indexing-status branch December 1, 2020 20:26

scottybollinger mentioned this pull request Dec 1, 2020

[7.x] [Enterprise Search] Migrate shared Indexing Status component (#84571) #84706

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enterprise Search] Migrate shared Indexing Status component#84571

[Enterprise Search] Migrate shared Indexing Status component#84571
scottybollinger merged 11 commits intoelastic:masterfrom
scottybollinger:scottybollinger/shared-indexing-status

scottybollinger commented Nov 30, 2020

Uh oh!

yakhinvadim Nov 30, 2020 •

edited

Loading

Uh oh!

Uh oh!

cee-chen Nov 30, 2020 •

edited

Loading

Uh oh!

scottybollinger Nov 30, 2020

Uh oh!

cee-chen Nov 30, 2020

Uh oh!

cee-chen left a comment

Uh oh!

cee-chen Nov 30, 2020

Uh oh!

cee-chen Nov 30, 2020

Uh oh!

cee-chen Nov 30, 2020

Uh oh!

cee-chen left a comment

Uh oh!

scottybollinger commented Dec 1, 2020

Uh oh!

kibanamachine commented Dec 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-          {percentageComplete < 100 && statusPanel(percentageComplete)}
+          {percentageComplete < 100 && (
+            <EuiPanel paddingSize="l" hasShadow>
+              <IndexingStatusContent percentageComplete={percentComplete} />
+            </EuiPanel>
+          )}

Conversation

scottybollinger commented Nov 30, 2020

Summary

Checklist

Uh oh!

yakhinvadim Nov 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cee-chen Nov 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottybollinger Nov 30, 2020

Choose a reason for hiding this comment

Uh oh!

cee-chen Nov 30, 2020

Choose a reason for hiding this comment

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

cee-chen Nov 30, 2020

Choose a reason for hiding this comment

Uh oh!

cee-chen Nov 30, 2020

Choose a reason for hiding this comment

Uh oh!

cee-chen Nov 30, 2020

Choose a reason for hiding this comment

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

scottybollinger commented Dec 1, 2020

Uh oh!

kibanamachine commented Dec 1, 2020

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yakhinvadim Nov 30, 2020 •

edited

Loading

cee-chen Nov 30, 2020 •

edited

Loading