[App Search] API logs: Add log detail flyout #96162

cee-chen · 2021-04-02T19:51:57Z

Summary

Final PR for API logs. This adds the Detail flyout (previously a modal in the standalone UI). The UI within the flyout remains mostly the same as before.

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

cee-chen · 2021-04-02T19:55:39Z

...nterprise_search/public/applications/app_search/components/api_logs/api_log/api_log_logic.ts

+  closeFlyout(): void;
+}
+
+export const ApiLogLogic = kea<MakeLogicType<ApiLogValues, ApiLogActions>>({


Maybe a little extra to split this out from the ApiLogsLogic, but I'm a fan of modularity, and arguably ApiLogsLogic is complicated enough w/ the extra polling business logic to be kept as focused on polling as possible

cee-chen · 2021-04-02T19:56:56Z

...erprise_search/public/applications/app_search/components/api_logs/api_log/api_log_flyout.tsx

+export const ApiLogHeading: React.FC = ({ children }) => (
+  <EuiTitle size="xs">
+    <h3>{children}</h3>
+  </EuiTitle>
+);


This is also probably a little extra but I figured if Davey ever wants to change the size of all the titles at once this would make it easier? lol.

That being said I can get rid of this if people aren't a fan, just let me know

Wouldn't be my style but doesn't offend me

cee-chen · 2021-04-02T19:59:33Z

...search/public/applications/app_search/components/api_logs/components/api_logs_table.test.tsx

    expect(wrapper.find(EuiButtonEmpty)).toHaveLength(3);
    wrapper.find('[data-test-subj="ApiLogsTableDetailsButton"]').first().simulate('click');
-    // TODO: API log details flyout
+    expect(actions.openFlyout).toHaveBeenCalled();


Follow-up to #96008 (comment) - @JasonStoltz feel free to yell if you'd like this to be in its own test block and not in the main renders test 🤔

cee-chen · 2021-04-02T20:03:57Z

x-pack/plugins/enterprise_search/public/applications/app_search/components/api_logs/utils.ts

+export const safeJsonParseAndStringify = (json: string) => {
+  try {
+    return JSON.stringify(JSON.parse(json), null, 2);
+  } catch (e) {
+    return json;
+  }
+};


FYI I migrated this entirely as-is from App Search, I'm not a super huge fan of the name but I can't really think of a better one? Feel free to shout if you have any ideas 🤷

x-pack/plugins/enterprise_search/public/applications/app_search/components/api_logs/utils.ts

cee-chen · 2021-04-02T20:09:52Z

...ck/plugins/enterprise_search/public/applications/app_search/components/api_logs/api_logs.tsx

          <ApiLogsTable hasPagination />
+          <ApiLogFlyout />


I debated putting ApiLogFlyout within ApiLogsTable but opted not to in the end. I don't feel super strongly either way, but I landed on this side of the fence because:

I like our <Table /> components just being self contained modular tables/components, and it definitely simplifies tests for the table component

It's nice having an 'overview' of what's on the page from the view itself

That being said in the end I don't think it super matters either way so happy to be convinced otherwise

I agree it should be separate from ApiLogsTable

…h/components/api_logs/utils.ts

byronhulcher

Looking great @constancecchen

byronhulcher · 2021-04-05T14:25:15Z

...erprise_search/public/applications/app_search/components/api_logs/api_log/api_log_flyout.tsx

+export const ApiLogHeading: React.FC = ({ children }) => (
+  <EuiTitle size="xs">
+    <h3>{children}</h3>
+  </EuiTitle>
+);


Wouldn't be my style but doesn't offend me

byronhulcher · 2021-04-05T14:30:54Z

...ck/plugins/enterprise_search/public/applications/app_search/components/api_logs/api_logs.tsx

          <ApiLogsTable hasPagination />
+          <ApiLogFlyout />


I agree it should be separate from ApiLogsTable

x-pack/plugins/enterprise_search/public/applications/app_search/components/api_logs/utils.ts

Co-authored-by: Byron Hulcher <byronhulcher@gmail.com>

cee-chen · 2021-04-05T16:06:19Z

@elasticmachine merge upstream

cee-chen · 2021-04-05T16:11:51Z

Thanks a ton for the great feedback Byron! 🙌 Enabling auto-merge for now but feel free to yell @JasonStoltz if you see any issues or change requests, or as always I can follow up on any later comments in a follow-up PR

cee-chen · 2021-04-05T16:39:06Z

Oh snap, I didn't see the suggestion had a name change - fixing

cee-chen · 2021-04-05T18:42:32Z

@elasticmachine merge upstream

kibanamachine · 2021-04-05T20:47:56Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`enterpriseSearch`	1315	1318	+3

Async chunks

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

id	before	after	diff
`enterpriseSearch`	1.9MB	1.9MB	+5.4KB

History

💔 Build #117288 failed 3ca74f6
💔 Build #117275 failed 9571b4c
💚 Build #117101 succeeded b04020a

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

* Set up helper for showing JSON request/response bodies * Set up mock API log obj for tests to use * Add ApiLogLogic file for flyout handling * Add ApiLogFlyout component * Update views to load flyout * Update table to open flyout * Update x-pack/plugins/enterprise_search/public/applications/app_search/components/api_logs/utils.ts * PR feedback: comments Co-authored-by: Byron Hulcher <byronhulcher@gmail.com> Co-authored-by: Byron Hulcher <byronhulcher@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-04-05T20:50:45Z

💚 Backport successful

✅ 7.x / #96258

This backport PR will be merged automatically after passing CI.

* Set up helper for showing JSON request/response bodies * Set up mock API log obj for tests to use * Add ApiLogLogic file for flyout handling * Add ApiLogFlyout component * Update views to load flyout * Update table to open flyout * Update x-pack/plugins/enterprise_search/public/applications/app_search/components/api_logs/utils.ts * PR feedback: comments Co-authored-by: Byron Hulcher <byronhulcher@gmail.com> Co-authored-by: Byron Hulcher <byronhulcher@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Constance <constancecchen@users.noreply.github.com> Co-authored-by: Byron Hulcher <byronhulcher@gmail.com>

…-nav * 'master' of github.com:elastic/kibana: (106 commits) [Lens] don't use eui variables for zindex (elastic#96117) Remove /src/legacy (elastic#95510) skip flaky suite (elastic#95899) [Dashboard] Fix Lens and TSVB chart tooltip positioning relative to global headers (elastic#94247) fixes a skipped management x-pack test (elastic#96178) [App Search] API logs: Add log detail flyout (elastic#96162) [tech-debt] Remove defunct opacity parameters from EUI shadow functions (elastic#96191) Add Input Controls project configuration (elastic#96238) [file upload] document file upload privileges and provide actionable UI when failures occur (elastic#95883) Revert "TS Incremental build exclude test files (elastic#95610)" (elastic#96223) [App Search] Added Sample Response section to Result Settings (elastic#95971) [Maps] Safe-erase text-field (elastic#94873) [RAC][Alert Triage][TGrid] Update the Alerts Table (TGrid) API to implement `renderCellValue` (elastic#96098) [Maps] Enable all zoom levels for all users (elastic#96093) Use plugin version in its publicPath (elastic#95945) [Enterprise Search] Expose core.chrome.setIsVisible for use in Workplace Search (elastic#95984) [Workplace Search] Add sub nav and fix rendering bugs in Personal dashboard (elastic#96100) [OBS]home page is showing incorrect value of APM throughput (tpm) (elastic#95991) [Observability] Exploratory View initial skeleton (elastic#94426) [KQL] Fixed styles of KQL textarea for the K8 theme (elastic#96190) ... # Conflicts: # x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/restore_snapshot.helpers.ts

cee-chen added 6 commits April 2, 2021 12:11

Set up helper for showing JSON request/response bodies

d8cf8af

Set up mock API log obj for tests to use

a57fdb6

Add ApiLogLogic file for flyout handling

bee1976

Add ApiLogFlyout component

803c8fb

Update views to load flyout

4e42f93

Update table to open flyout

930df38

cee-chen added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Apr 2, 2021

cee-chen requested a review from a team April 2, 2021 19:51

cee-chen commented Apr 2, 2021

View reviewed changes

Update x-pack/plugins/enterprise_search/public/applications/app_searc…

b04020a

…h/components/api_logs/utils.ts

byronhulcher self-requested a review April 5, 2021 14:26

JasonStoltz assigned JasonStoltz and unassigned JasonStoltz Apr 5, 2021

byronhulcher approved these changes Apr 5, 2021

View reviewed changes

PR feedback: comments

24fc7cf

Co-authored-by: Byron Hulcher <byronhulcher@gmail.com>

Merge branch 'master' into api-logs-4

9571b4c

cee-chen enabled auto-merge (squash) April 5, 2021 16:11

PR feedback: naming/comments

3ca74f6

Merge branch 'master' into api-logs-4

8d8723c

cee-chen merged commit 56d9f3d into elastic:master Apr 5, 2021

cee-chen deleted the api-logs-4 branch April 5, 2021 20:48

kibanamachine mentioned this pull request Apr 5, 2021

[7.x] [App Search] API logs: Add log detail flyout (#96162) #96258

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[App Search] API logs: Add log detail flyout #96162

[App Search] API logs: Add log detail flyout #96162

cee-chen commented Apr 2, 2021

cee-chen Apr 2, 2021

cee-chen Apr 2, 2021

byronhulcher Apr 5, 2021

cee-chen Apr 2, 2021

cee-chen Apr 2, 2021

cee-chen Apr 2, 2021

byronhulcher Apr 5, 2021

byronhulcher left a comment

byronhulcher Apr 5, 2021

byronhulcher Apr 5, 2021

cee-chen commented Apr 5, 2021

cee-chen commented Apr 5, 2021

cee-chen commented Apr 5, 2021

cee-chen commented Apr 5, 2021

kibanamachine commented Apr 5, 2021

kibanamachine commented Apr 5, 2021

[App Search] API logs: Add log detail flyout #96162

[App Search] API logs: Add log detail flyout #96162

Conversation

cee-chen commented Apr 2, 2021

Summary

Checklist

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

byronhulcher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen commented Apr 5, 2021

cee-chen commented Apr 5, 2021

cee-chen commented Apr 5, 2021

cee-chen commented Apr 5, 2021

kibanamachine commented Apr 5, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

kibanamachine commented Apr 5, 2021

💚 Backport successful