[Ingest] Edit datasource UI #64727

jen-huang · 2020-04-29T01:06:20Z

Resolves #57957

Summary

This PR adds edit data source UI by importing the existing create data source UI components. A few other cleanups were also done, including: removal of existing code, type clean up, remove delete agent config functionality from list page (will be moved to config Settings tab), and fixes of small data source validation bugs.

Screenshots

…add additional datasource hooks

…a package

elasticmachine · 2020-04-29T01:06:22Z

Pinging @elastic/ingest-management (Team:Ingest Management)

jfsiii

I haven't checked this out and run it, but I think the code is clear.

I left some comments but nothing to hold up a merge.

jfsiii · 2020-04-29T10:34:48Z

...ager/public/applications/ingest_manager/sections/agent_config/edit_datasource_page/index.tsx

+        if (agentConfigData?.item) {
+          setAgentConfig(agentConfigData.item);
+        }
+        const { data: datasourceData } = await sendGetOneDatasource(datasourceId);


I believe we can use Promise.all here and do these two requests in parallel.

jfsiii · 2020-04-29T10:41:35Z

...gest_manager/sections/agent_config/details_page/components/datasources/datasources_table.tsx

@@ -56,6 +56,7 @@ export const DatasourcesTable: React.FunctionComponent<Props> = ({
 }) => {
  const hasWriteCapabilities = useCapabilities().write;
  const addDatasourceLink = useAgentConfigLink('add-datasource', { configId: config.id });
+  const editDatasourceLink = useLink(`/configs/${config.id}/edit-datasource/{datasourceId}`);


I'd like to avoid doing this in a view and have in some other function/service. Can useAgentConfigLink be updated or copied to do this?

jfsiii · 2020-04-29T10:44:43Z

...ager/public/applications/ingest_manager/sections/agent_config/edit_datasource_page/index.tsx

+                ...restOfInput,
+                streams: streams.map(stream => {
+                  const { agent_stream, ...restOfStream } = stream;
+                  return { ...restOfStream };


Can just return the restOfStream object, right?

jfsiii · 2020-04-29T10:58:44Z

...ager/public/applications/ingest_manager/sections/agent_config/edit_datasource_page/index.tsx

+    setDatasource(newDatasource);
+
+    // eslint-disable-next-line no-console
+    console.debug('Datasource updated', newDatasource);


We don't want to ship this do we?

This is intentional for alpha/beta as this area is higher risk for issues occurring. Having these logs can help us debug customer issues faster. The same lines are set in create flow too. Note that it's .debug and not .log so isn't displayed when using the default console output level

jfsiii · 2020-04-29T11:01:21Z

...ager/public/applications/ingest_manager/sections/agent_config/edit_datasource_page/index.tsx

+  };
+
+  // Cancel url
+  const CONFIG_URL = useLink(`${AGENT_CONFIG_DETAILS_PATH}${configId}`);


I think we have a way to get this but either way, I would like to avoid views having knowledge of the route specifics and having to assemble them.

jfsiii · 2020-04-29T11:03:00Z

...ager/public/applications/ingest_manager/sections/agent_config/edit_datasource_page/index.tsx

+    }
+    const { error } = await saveDatasource();
+    if (!error) {
+      history.push(`${AGENT_CONFIG_DETAILS_PATH}${configId}`);


Same comment about creating routes in a view

paul-tavares · 2020-04-29T13:23:24Z

...gest_manager/sections/agent_config/details_page/components/datasources/datasources_table.tsx

                    icon="pencil"
-                    onClick={() => {}}
+                    href={editDatasourceLink.replace(
+                      encodeURIComponent('{datasourceId}'),


I was not able to get Edit Datasource to display when I ran this PR locally - kept getting API error about Datasource not found. The issue seems to be here - the editDatasourceLink actually does not have the {datasourceid} url encoded. I changed it locally to this and was able to get the Edit form to show up:

Suggested change

encodeURIComponent('{datasourceId}'),

'{datasourceId}',

But, given @jfsiii 's comments above, perhaps you will get the URL using the other hook (which would imply you might need to create a HOC for this EuiContextMenuItem)

This is weird, on mine it does't work without encodeURIComponent. I changed it to get the base url without {datasourceId} and just append the ID after

paul-tavares · 2020-04-29T13:26:18Z

Aside from the one issue I found (comment provided), I ran it locally and it seems to work fine on the Endpoint datasource (which does not have data to be displayed in section 2 of the UI) - 👍

jen-huang · 2020-04-29T17:42:18Z

Re: routing

I agree with the sentiments about not having views building URIs. Our routing isn't consistent, we have views that use hooks/services for onClick (history) and hrefs, and views that build the URIs manually. Ideally I would like to have all routing functionality in hooks, but it would mean that we will need to pull out some components into smaller ones as hooks can only be used at the top level of functional components. Given the existing inconsistencies and other priorities, I'll defer this tech debt for now.

jen-huang · 2020-04-29T20:33:04Z

@elasticmachine merge upstream

jen-huang · 2020-04-29T23:13:29Z

@elasticmachine merge upstream

kibanamachine · 2020-04-30T00:57:19Z

💚 Build Succeeded

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

History

💔 Build #43961 failed c7a99e8

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

* Adjust NewDatasource type to exclude stream `agent_stream` property, add additional datasource hooks * Initial pass at edit datasource UI * Clean up dupe code, fix submit button not enabled after re-selecting a package * Remove delete config functionality from list page * Show validation errors for data source name and description fields * Fix types * Add success toasts * Review fixes, clean up i18n Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* Adjust NewDatasource type to exclude stream `agent_stream` property, add additional datasource hooks * Initial pass at edit datasource UI * Clean up dupe code, fix submit button not enabled after re-selecting a package * Remove delete config functionality from list page * Show validation errors for data source name and description fields * Fix types * Add success toasts * Review fixes, clean up i18n Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* master: (42 commits) [Ingest] Allow aggent to send metadata compliant with ECS (elastic#64452) [Endpoint] Remove todos, urls to issues (elastic#64833) [Uptime] Remove hard coded value for monitor states histograms (elastic#64396) Feature/send feedback link (elastic#64845) [ML] Moving get filters capability to admin (elastic#64879) Remove edit alert button from alerts list (elastic#64643) [EPM] Handle constant_keyword type in KB index patterns and ES index templates (elastic#64876) [ML] Disable data frame anaylics clone button based on permission (elastic#64830) Dashboard url generator to preserve saved filters from destination dashboard (elastic#64767) add generic typings for SavedObjectMigrationFn (elastic#63943) Allow to define and update a defaultPath for applications (elastic#64498) [Event Log] add rel=primary to saved objects for query targets (elastic#64615) [Lens] Use a size of 5 for first string field in visualization (elastic#64726) [SIEM][Lists] Removes plugin dependencies, adds more unit tests, fixes more TypeScript types [Ingest] Edit datasource UI (elastic#64727) [Lens] Bind all time fields to the time picker (elastic#63874) [Lens] Use suggestion system in chart switcher for subtypes (elastic#64613) Improve alpha messaging (elastic#64692) [Ingest] Allow to enable monitoring of elastic agent (elastic#63598) [Metrics UI] Fix alerting when a filter query is present (elastic#64575) ...

…bana into pipeline-editor-part-mvp-2 * 'feature/ingest-node-pipelines' of github.com:elastic/kibana: (90 commits) remove unused import address review feedback [Ingest pipelines] Cleanup (elastic#64794) [Ingest] Edit datasource UI (elastic#64727) [Lens] Bind all time fields to the time picker (elastic#63874) [Lens] Use suggestion system in chart switcher for subtypes (elastic#64613) Improve alpha messaging (elastic#64692) [Ingest] Allow to enable monitoring of elastic agent (elastic#63598) [Metrics UI] Fix alerting when a filter query is present (elastic#64575) skip flaky suite (elastic#64812) (elastic#64723) [Maps] do not display EMS or kibana layer wizards when not configured (elastic#64554) [Reporting/Test] Convert functional test code to Typescript (elastic#64601) make inserting timestamp with navigate methods optional with default true (elastic#64655) [EPM] Update UI to handle package versions and updates (elastic#64689) Minimize dependencies required by our telemetry middleware (elastic#64665) [Telemetry] oss api tests (elastic#64602) [ML] Adding endpoint capability checks (elastic#64662) Update jest config for coverage (elastic#64648) [SIEM][NP] Fixes bug in ML signals promotion (elastic#64720) share single data plugin bundle (elastic#64549) ...

jen-huang added 7 commits April 28, 2020 15:16

Adjust NewDatasource type to exclude stream agent_stream property, …

570027d

…add additional datasource hooks

Initial pass at edit datasource UI

258a01b

Clean up dupe code, fix submit button not enabled after re-selecting …

95c7c51

…a package

Remove delete config functionality from list page

1dccdc9

Show validation errors for data source name and description fields

77d6e19

Merge remote-tracking branch 'upstream/master' into ingest/edit-ds

10c9e26

Fix types

67e7f16

jen-huang added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.8.0 Team:Fleet Team label for Observability Data Collection Fleet team labels Apr 29, 2020

jen-huang requested a review from a team April 29, 2020 01:06

jen-huang self-assigned this Apr 29, 2020

Add success toasts

c7a99e8

jfsiii approved these changes Apr 29, 2020

View reviewed changes

paul-tavares reviewed Apr 29, 2020

View reviewed changes

Review fixes, clean up i18n

47a5274

Merge branch 'master' into ingest/edit-ds

13d2482

Merge branch 'master' into ingest/edit-ds

f2ae8ed

jen-huang merged commit fba5128 into elastic:master Apr 30, 2020

jen-huang deleted the ingest/edit-ds branch April 30, 2020 01:03

jen-huang mentioned this pull request Apr 30, 2020

[7.x] [Ingest] Edit datasource UI (#64727) #64860

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ingest] Edit datasource UI #64727

[Ingest] Edit datasource UI #64727

jen-huang commented Apr 29, 2020 •

edited

Loading

elasticmachine commented Apr 29, 2020

jfsiii left a comment

jfsiii Apr 29, 2020

jfsiii Apr 29, 2020

jfsiii Apr 29, 2020

jfsiii Apr 29, 2020

jen-huang Apr 29, 2020

jfsiii Apr 29, 2020

jfsiii Apr 29, 2020

paul-tavares Apr 29, 2020

jen-huang Apr 29, 2020

paul-tavares commented Apr 29, 2020

jen-huang commented Apr 29, 2020

jen-huang commented Apr 29, 2020

jen-huang commented Apr 29, 2020

kibanamachine commented Apr 30, 2020

[Ingest] Edit datasource UI #64727

[Ingest] Edit datasource UI #64727

Conversation

jen-huang commented Apr 29, 2020 • edited Loading

Summary

Screenshots

elasticmachine commented Apr 29, 2020

jfsiii 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares commented Apr 29, 2020

jen-huang commented Apr 29, 2020

jen-huang commented Apr 29, 2020

jen-huang commented Apr 29, 2020

kibanamachine commented Apr 30, 2020

💚 Build Succeeded

History

jen-huang commented Apr 29, 2020 •

edited

Loading