[App Search] Added all Document related routes and logic #83324

JasonStoltz · 2020-11-12T18:39:54Z

Summary

Adds all logic and routes for Document and Document Detail views

Checklist

Delete any items that are not applicable to this PR.

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

For maintainers

This was checked for breaking API changes and was labeled appropriately

JasonStoltz · 2020-11-12T18:47:50Z

...terprise_search/public/applications/app_search/components/documents/document_detail_logic.ts

+          const { http } = HttpLogic.values;
+          await http.delete(`/api/app_search/engines/${engineName}/documents/${documentId}`);
+          // Note: This used to use a server generated message, but no i18n so we'll do it here
+          setQueuedSuccessMessage(DELETE_SUCCESS);


This used to use a server generated message, but that is not i18n compliant so I moved the message inline here.

Ooo, great call! I actually have a note buried somewhere in my backlog to figure out what to do with i18n and errors/messages coming in from the server 😬 For easy stuff like this I'm a huge +1 to pulling them out like this. For the more complex wildcard stuff, we'll have to ask the Kibana team for guidance I think

JasonStoltz · 2020-11-12T18:48:35Z

...terprise_search/public/applications/app_search/components/documents/document_detail_logic.ts

+          await http.delete(`/api/app_search/engines/${engineName}/documents/${documentId}`);
+          // Note: This used to use a server generated message, but no i18n so we'll do it here
+          setQueuedSuccessMessage(DELETE_SUCCESS);
+          // TODO Handle routing after success


The old code used a history.push to navigate back to the document list view after a successful delete.

I wonder if that is the way we should continue doing it or not.

You can use KibanaLogic.values.navigateToUrl() to replace history.push() - it works correctly with Kibana's SPA history (doesn't rerender top-level app which resets Kea state) and also automatically prefixes our app route for us.

I'll try to write up a cheat sheet/reference list of conversions like that at some point soon for our migration QOL

I will re-add this when I have the UI wired up with 404 handling. It will be easier to test.

JasonStoltz · 2020-11-12T18:49:07Z

...terprise_search/public/applications/app_search/components/documents/document_detail_logic.ts

+      try {
+        const { http } = HttpLogic.values;
+        const response = await http.get(
+          `/api/app_search/engines/${engineName}/documents/${documentId}`


Future note to myself. We'll need to handle 404s somehow. It's not handled here.

cee-chen

Awesome stuff! Just a bunch of small nits and a quick note about my grand future plans for DocumentLogic and the creation modal (no change requested there, just a heads up).

x-pack/plugins/enterprise_search/server/routes/app_search/documents.test.ts

cee-chen · 2020-11-12T21:14:54Z

x-pack/plugins/enterprise_search/public/applications/app_search/components/documents/types.ts

+
+export interface FieldDetails {
+  name: string;
+  value: string | string[];


[not a change request, just me thinking out loud] Someday in a very distant future it might be nice to clean that up a bit more in a server transform - e.g. by doing isArray(value) ? value : [value] we know for sure we're always dealing w/ arrays

💯 Yeah that would be ideal

I made a simple fix for this at the Logic level for now. This could be refactored later, but I think it's worth at least this small amount of effort because it will greatly simplify things downstream in the UI: 772e801

Oh awesome, love it! I have a very slight preference to storing / testing normalizeFields in a utils file to separate out the array logic from getDocumentDetails test (in theory a unit test is easier to follow if it handles 1 thing at a time), but it's a very slight preference and I'm fine with merging as-is.

I reverted it. We actually need to know if it's an array or not on the front-end.

a40fcb9

Ahh no worries, I was hoping something like that wouldn't come up, but alas. Some day

...ins/enterprise_search/public/applications/app_search/components/documents/documents_logic.ts

...terprise_search/public/applications/app_search/components/documents/document_detail_logic.ts

cee-chen · 2020-11-12T21:31:16Z

...terprise_search/public/applications/app_search/components/documents/document_detail_logic.ts

+          const { http } = HttpLogic.values;
+          await http.delete(`/api/app_search/engines/${engineName}/documents/${documentId}`);
+          // Note: This used to use a server generated message, but no i18n so we'll do it here
+          setQueuedSuccessMessage(DELETE_SUCCESS);


Ooo, great call! I actually have a note buried somewhere in my backlog to figure out what to do with i18n and errors/messages coming in from the server 😬 For easy stuff like this I'm a huge +1 to pulling them out like this. For the more complex wildcard stuff, we'll have to ask the Kibana team for guidance I think

...se_search/public/applications/app_search/components/documents/documents_detail_logic.test.ts

Co-authored-by: Constance <[email protected]>

apmmachine · 2020-11-16T15:39:26Z

💚 Build Succeeded

pipeline
Commit: e4e4cd0

JasonStoltz · 2020-11-16T16:14:49Z

@constancecchen I made some changes, take a look!

cee-chen · 2020-11-16T18:12:09Z

I think the only semi-blocking comment that I want to figure out before merging is #83324 (comment) - I'm pushing back against exporting our interfaces because I'm worried it may encourage overriding types / using as SomeLogicValues as a way to override incorrectly typed values in source code. Kea should be able to handle correctly type checking all values from us going forward, including in tests, so I'm confused why we need to re-define our mock values there.

This reverts commit 772e801.

JasonStoltz · 2020-11-16T20:58:31Z

Good points. Updated.

kibanamachine · 2020-11-16T22:37:00Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 505a9d3

Metrics [docs]

Distributable file count

id	before	after	diff
`default`	42835	42836	+1

History

💚 Build #88055 succeeded e4e4cd0
💔 Build #88029 failed f492e2e
💚 Build #87706 succeeded c371287

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

…3548)

* master: (51 commits) [ML] Persisted URL state for the Data frame analytics jobs and models pages (elastic#83439) adds xpack.security.authc.selector.enabled setting (elastic#83551) skip flaky suite (elastic#77279) [ML] Improve support for script and aggregation fields in anomaly detection jobs (elastic#81923) [Workplace Search] Migrate SourcesLogic from ent-search (elastic#83544) [ML] Add UI test for feature importance features (elastic#82677) [Maps] Improve icons for all layer types (elastic#83503) Replace experimental badge with Beta (elastic#83468) [Fleet][EPM] Unified install and archive (elastic#83384) Move src/legacy/server/keystore to src/cli (elastic#83483) Used SO for saving the API key IDs that should be deleted (elastic#82211) [Uptime] Mock implementation to account for math flakiness test (elastic#83535) [Workplace Search] Enable check for org context based on URL (elastic#83487) [App Search] Added all Document related routes and logic (elastic#83324) [Alerting UI] Fix console error when setting connector params (elastic#83333) [Discover] Allow custom name for fields via index pattern field management (elastic#70039) [Uptime] Fix monitor list down histogram (elastic#83411) remove headers timeout hack, rely on nodejs timeouts (elastic#83419) [ML] Update console autocomplete for ML data frame evaluate API (elastic#83151) [Lens] Color in dimension trigger (elastic#76871) ...

JasonStoltz added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Nov 12, 2020

JasonStoltz force-pushed the documents branch from ecf42b1 to d91ca4f Compare November 12, 2020 18:45

JasonStoltz commented Nov 12, 2020

View reviewed changes

Added all document logic and routes

2100770

JasonStoltz force-pushed the documents branch from d91ca4f to 2100770 Compare November 12, 2020 18:51

JasonStoltz marked this pull request as ready for review November 12, 2020 18:51

JasonStoltz requested a review from a team November 12, 2020 18:51

Added a index.ts

c371287

cee-chen reviewed Nov 12, 2020

View reviewed changes

Apply suggestions from code review

f492e2e

Co-authored-by: Constance <[email protected]>

JasonStoltz added 3 commits November 16, 2020 10:57

Fixed suggestion error

9d35baa

Fixed test file name

273a24b

Normalize FieldDetails

772e801

JasonStoltz requested a review from cee-chen November 16, 2020 16:14

JasonStoltz added 2 commits November 16, 2020 11:28

Fixed a type error

d86bea2

Merge branch 'master' into documents

e4e4cd0

JasonStoltz added 2 commits November 16, 2020 15:51

Revert "Normalize FieldDetails"

a40fcb9

This reverts commit 772e801.

Remove kea exports

505a9d3

cee-chen approved these changes Nov 16, 2020

View reviewed changes

JasonStoltz merged commit 9b5605f into elastic:master Nov 17, 2020

JasonStoltz deleted the documents branch November 17, 2020 13:09

JasonStoltz mentioned this pull request Nov 17, 2020

[7.x] [App Search] Added all Document related routes and logic (#83324) #83548

Merged

JasonStoltz added a commit to JasonStoltz/kibana that referenced this pull request Nov 17, 2020

[App Search] Added all Document related routes and logic (elastic#83324)

1c3b410

JasonStoltz added a commit that referenced this pull request Nov 17, 2020

[App Search] Added all Document related routes and logic (#83324) (#8…

88206b2

…3548)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[App Search] Added all Document related routes and logic #83324

[App Search] Added all Document related routes and logic #83324

JasonStoltz commented Nov 12, 2020

JasonStoltz Nov 12, 2020

cee-chen Nov 12, 2020

JasonStoltz Nov 12, 2020

cee-chen Nov 12, 2020 •

edited

Loading

JasonStoltz Nov 16, 2020

JasonStoltz Nov 12, 2020

cee-chen left a comment

cee-chen Nov 12, 2020

JasonStoltz Nov 16, 2020

JasonStoltz Nov 16, 2020

cee-chen Nov 16, 2020

JasonStoltz Nov 16, 2020

cee-chen Nov 16, 2020

cee-chen Nov 12, 2020

apmmachine commented Nov 16, 2020 •

edited

Loading

JasonStoltz commented Nov 16, 2020

cee-chen commented Nov 16, 2020 •

edited

Loading

JasonStoltz commented Nov 16, 2020

kibanamachine commented Nov 16, 2020

[App Search] Added all Document related routes and logic #83324

[App Search] Added all Document related routes and logic #83324

Conversation

JasonStoltz commented Nov 12, 2020

Summary

Checklist

For maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen Nov 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apmmachine commented Nov 16, 2020 • edited Loading

💚 Build Succeeded

JasonStoltz commented Nov 16, 2020

cee-chen commented Nov 16, 2020 • edited Loading

JasonStoltz commented Nov 16, 2020

kibanamachine commented Nov 16, 2020

💚 Build Succeeded

Metrics [docs]

Distributable file count

History

cee-chen Nov 12, 2020 •

edited

Loading

apmmachine commented Nov 16, 2020 •

edited

Loading

cee-chen commented Nov 16, 2020 •

edited

Loading