[Resolver] model `location.search` in redux #76140

oatkiller · 2020-08-27T19:16:28Z

Read location.search from the redux store instead of a hook so that the entire view has a single (synchronized) source of truth.

Still works:

Still works in the test app as well:

Checklist

This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

oatkiller · 2020-08-27T19:17:20Z

x-pack/plugins/security_solution/public/resolver/models/location_search.ts

+ * The legacy `crumbEvent` and `crumbId` parameters.
+ * @deprecated
+ */
+export function breadcrumbParameters(


This file will have the logic for getting values out of (and constructing new) URLSearchParams

oatkiller · 2020-08-27T19:18:06Z

x-pack/plugins/security_solution/public/resolver/store/data/action.ts

@@ -75,7 +75,15 @@ interface AppReceivedNewExternalProperties {
     * the `_id` of an ES document. This defines the origin of the Resolver graph.
     */
    databaseDocumentID?: string;
+    /**


This action isn't specific to data at all so I'm going to move it to the top level Resolver actions file.

oatkiller · 2020-08-27T19:18:27Z

x-pack/plugins/security_solution/public/resolver/store/selectors.ts

+ * The legacy `crumbEvent` and `crumbId` parameters.
+ * @deprecated
+ */
+export const breadcrumbParameters = composeSelectors(


use this to get the query string parameters used by the panel

oatkiller · 2020-08-27T19:19:22Z

x-pack/plugins/security_solution/public/resolver/store/ui/selectors.ts

-/**
- * id of the currently "selected" tree node
- */
-export const selectedNode = createSelector(


I'm guessing this is another line-endings thing.

oatkiller · 2020-08-27T19:19:53Z

x-pack/plugins/security_solution/public/resolver/store/ui/selectors.ts

+ * The legacy `crumbEvent` and `crumbId` parameters.
+ * @deprecated
+ */
+export const breadcrumbParameters = createSelector(


This function was added. the rest of the changes (except for import statements) are white space.

oatkiller · 2020-08-27T19:20:33Z

x-pack/plugins/security_solution/public/resolver/view/panels/index.tsx

@@ -39,7 +39,9 @@ const PanelContent = memo(function PanelContent() {

  const { timestamp } = useContext(SideEffectContext);

-  const { pushToQueryParams, queryParams } = useResolverQueryParams();
+  const queryParams = useSelector(selectors.breadcrumbParameters);


use the selector to get the panel query params now

oatkiller · 2020-08-27T19:21:00Z

x-pack/plugins/security_solution/public/resolver/view/panels/index.tsx

-  const { pushToQueryParams, queryParams } = useResolverQueryParams();
+  const queryParams = useSelector(selectors.breadcrumbParameters);
+
+  const pushToQueryParams = useReplaceBreadcrumbParameters();


renamed useResolverQueryParams to this. it no longer returns the query params.

oatkiller · 2020-08-27T19:22:16Z

x-pack/plugins/security_solution/public/resolver/view/use_state_syncing_actions.ts

  useLayoutEffect(() => {
    dispatch({
      type: 'appReceivedNewExternalProperties',
-      payload: { databaseDocumentID, resolverComponentInstanceID },
+      payload: { databaseDocumentID, resolverComponentInstanceID, locationSearch },


dispatch this action any time the location.search changes.

Having this action fire whenever location.search changes seems sort of overkill, why not just when the relevant params change?

The app will need to read the entire query string. For example, we might need to show a link that causes our panel to open. In that case we'll want the link to include all query string values (not just ones relevant to this Resolver instance.)

kqualters-elastic

Since I had more or less the same logic just in a few different places, LGTM 👍 looks great to me!

Begin reading `location.search` via redux so that all parts of the app share a single source of truth about it

… file

oatkiller · 2020-08-27T20:12:34Z

x-pack/plugins/security_solution/public/resolver/view/panels/panel_content_error.tsx

+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { i18n } from '@kbn/i18n';


Seems like this file probably had DOS line endings. Hide whitespace so you can see what really changed.

kibanamachine · 2020-08-27T22:06:31Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 8f40cd1

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
securitySolution	1926	+1	1925

async chunks size

id	value	diff	baseline
securitySolution	9.5MB	+1.7KB	9.5MB

distributable file count

id	value	diff	baseline
total	53140	+1	53139

History

💔 Build #71016 failed ca58e92

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

Read location.search from the redux store instead of a hook so that the entire view has a single (synchronized) source of truth. Also, no longer pass `pushToQueryParams` function to various components.

…s-for-710 * 'master' of github.com:elastic/kibana: (43 commits) [APM] Chart units don't update when toggling the chart legends (elastic#74931) [ILM] Add support for frozen phase in UI (elastic#75968) Hides advanced json for count metric (elastic#74636) add client-side feature usage API (elastic#75486) [Maps] add drilldown support map embeddable (elastic#75598) [Enterprise Search] Request handler refactors/enhancements + update existing routes to use shared handler (elastic#76106) [Resolver] model `location.search` in redux (elastic#76140) [APM] Prevent imports of public in server code (elastic#75979) fix eslint issue skip flaky suite (elastic#76223) [APM] Transaction duration anomaly alerting integration (elastic#75719) [Transforms] Avoid using "Are you sure" (elastic#75932) [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (elastic#76145) [plugin-helpers] improve 3rd party KP plugin support (elastic#75019) [docs/getting-started] link to yarn v1 specifically (elastic#76169) [Security_Solution][Resolver] Resolver loading and error state (elastic#75600) Fixes App Search documentation links (elastic#76133) Fix alerts unable to create / update when the name has trailing whitepace(s) (elastic#76079) [Resolver] Fix useSelector usage (elastic#76129) [Enterprise Search] Migrate util and components from ent-search (elastic#76051) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/data_tier_allocation/node_allocation.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx # x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts # x-pack/plugins/index_lifecycle_management/public/application/services/policies/warm_phase.ts

kibanamachine · 2020-09-01T12:58:44Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

Read location.search from the redux store instead of a hook so that the entire view has a single (synchronized) source of truth. Also, no longer pass `pushToQueryParams` function to various components. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

oatkiller requested review from a team as code owners August 27, 2020 19:16

oatkiller force-pushed the use-url-to-determine-what-panel-to-show branch from 6f44894 to 62a06bb Compare August 27, 2020 19:16

oatkiller commented Aug 27, 2020

View reviewed changes

oatkiller requested review from kqualters-elastic and michaelolo24 August 27, 2020 19:22

oatkiller added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Aug 27, 2020

kqualters-elastic approved these changes Aug 27, 2020

View reviewed changes

oatkiller added 3 commits August 27, 2020 15:54

[Resolver] model location.search in redux

302aa15

Begin reading `location.search` via redux so that all parts of the app share a single source of truth about it

Move appReceivedExternal... action from data to the top level actions…

ff0b3fb

… file

cleanup test

94d171d

michaelolo24 approved these changes Aug 27, 2020

View reviewed changes

dont pass pushToQueryParams to components

aee1ee4

oatkiller force-pushed the use-url-to-determine-what-panel-to-show branch from ca58e92 to aee1ee4 Compare August 27, 2020 20:09

oops

952346d

oatkiller commented Aug 27, 2020

View reviewed changes

remove unused type

8f40cd1

oatkiller merged commit 3fc3f58 into elastic:master Aug 28, 2020

oatkiller deleted the use-url-to-determine-what-panel-to-show branch August 28, 2020 12:53

oatkiller mentioned this pull request Aug 28, 2020

[7.x] [Resolver] model location.search in redux (#76140) #76228

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 1, 2020

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Resolver] model `location.search` in redux #76140

[Resolver] model `location.search` in redux #76140

oatkiller commented Aug 27, 2020 •

edited

Loading

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

kqualters-elastic Aug 27, 2020

oatkiller Aug 27, 2020

kqualters-elastic left a comment

oatkiller Aug 27, 2020

kibanamachine commented Aug 27, 2020

kibanamachine commented Sep 1, 2020

[Resolver] model location.search in redux #76140

[Resolver] model location.search in redux #76140

Conversation

oatkiller commented Aug 27, 2020 • edited Loading

Checklist

For maintainers

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

kqualters-elastic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Aug 27, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

async chunks size

distributable file count

History

kibanamachine commented Sep 1, 2020

[Resolver] model `location.search` in redux #76140

[Resolver] model `location.search` in redux #76140

oatkiller commented Aug 27, 2020 •

edited

Loading