[Fleet] showing agent policy creation error message on UI #125931

juliaElastic · 2022-02-17T13:37:16Z

Summary

Closes #125904

Showing error message from API if available on agent policy creation error.

To verify:

Create agent policy with name "Agent policy 1" in Add agent flyout
Try again to create with the same name
Expect an error message saying there is already a policy with the same name

elasticmachine · 2022-02-17T13:37:21Z

Pinging @elastic/fleet (Team:Fleet)

nchaulet · 2022-02-17T14:06:39Z

~~@juliaElastic code looks and it work as expected, in most of the place we are using toast notifications for errors, not sure if we should do the same here.~~

How I see the error existed before my bad no reason to change this.

nchaulet

🚀

hop-dev

Tested locally, code looks good 👍

kpollich

Presenting the ID in the error message like this seems like it might be a bit of unnecessary noise and confusion. We rarely reference agent policies by ID anywhere else in the UI, and in general, they are only displayed in URL's/links. For example, we don't even present the ID in the Agent Policy listing table - only the name.

It is a bit of a challenge when we handle errors like this because the simplest thing to do is just pass the server error on to the UI, but then we are left with the decision to make all server thrown errors presentable in the UI. The other option would be to do something like an error mapping utility that maps server error codes to presentable errors.

However, I think all of this is moot because this error looks like it's coming directly from the SO client, or at least I can't otherwise find the error message in our codebase. With that in mind, I think we can just move on with this error message. 🚀

juliaElastic · 2022-02-17T14:37:03Z

Presenting the ID in the error message like this seems like it might be a bit of unnecessary noise and confusion. We rarely reference agent policies by ID anywhere else in the UI, and in general, they are only displayed in URL's/links. For example, we don't even present the ID in the Agent Policy listing table - only the name.

It is a bit of a challenge when we handle errors like this because the simplest thing to do is just pass the server error on to the UI, but then we are left with the decision to make all server thrown errors presentable in the UI. The other option would be to do something like an error mapping utility that maps server error codes to presentable errors.

However, I think all of this is moot because this error looks like it's coming directly from the SO client, or at least I can't otherwise find the error message in our codebase. With that in mind, I think we can just move on with this error message. 🚀

@kpollich Yes, I've thought about this and what I could do is map the error message based on http status code. e.g. 409 would be conflict - policy already exists.
For other statuses I could hide that extra message to avoid showing something too technical like the id. (I'm not really sure how to simulate other errors like 400, 401, etc. to check the exact message coming from backend).

@nchaulet to your point, the callout design is coming from UX design doc of the #108456 feature

juliaElastic · 2022-02-17T15:45:17Z

@kpollich updated to map 409 error to fix the raised issue only

juliaElastic · 2022-02-21T07:34:36Z

@elasticmachine merge upstream

kibana-ci · 2022-02-21T08:57:25Z

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id	before	after	diff
`fleet`	660.8KB	661.2KB	+325.0B

History

💚 Build #25009 succeeded b6b06d0
💔 Build #24962 failed 1333a06

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

cc @juliaElastic

kibanamachine · 2022-02-21T09:00:35Z

💚 All backports created successfully

Status	Branch	Result
✅	8.1

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…5931) * showing agent policy creation error message on UI * mapping the error instead of showing from the backend Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 1df6ecf)

…126043) * showing agent policy creation error message on UI * mapping the error instead of showing from the backend Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 1df6ecf) Co-authored-by: Julia Bardi <[email protected]>

…5931) * showing agent policy creation error message on UI * mapping the error instead of showing from the backend Co-authored-by: Kibana Machine <[email protected]>

amolnater-qasource · 2022-03-01T08:53:38Z

Hi @juliaElastic
We have validated this PR with respect to reported issue at #125904 on 8.1 BC-4 Kibana cloud environment.
We have found it working fine now.

Descriptive error message for creating duplicate policy from Add Agent flyout is there.

Build details:
BUILD: 50428
COMMIT: 015578b

Screenshot:

Thanks

…5931) * showing agent policy creation error message on UI * mapping the error instead of showing from the backend Co-authored-by: Kibana Machine <[email protected]>

* WIP * WIP2 * Use new cases context hooks to open and close the flyout * Update timelines to use new hooks * CLose flyout on create success * Add back sucess toast * Move code to a dedicated component * Add CasesContext to observability * Remove dependency * Small refactor * Use observabilityAppId instead of observabilityFeatureId for buttons * Add CasesContext to timetable * Fix detection engine test cases * Fix broken tests * Fix broken tests * Rename hook * Add test cases for cases context ui * Add test for new hook * Remove state from the provider context * Remove basevalue * apply suggested renaming * Add usecallback * Add reducer types, fix test type, remove redundant check * Accept attachments as a prop for the cases select modal * Expose useCasesAddToExistingCase hook, reducer code and global component * use the new hook to open the select cases modasl * Fix tests and types * Add tests for cases global components * [Fleet] showing agent policy creation error message on UI (#125931) * showing agent policy creation error message on UI * mapping the error instead of showing from the backend Co-authored-by: Kibana Machine <[email protected]> * [ResponseOps] Adds tooltip to time window selector in ES query rule flyout (#125764) * [Lens] Allow detaching from global time range (#125563) * allow detaching from global time range * add test * fix time field recognition * fix tests Co-authored-by: Kibana Machine <[email protected]> * [Fleet] refactor auto upgrade package policies logic (#125909) * refactor upgrade package policies * fixed tests * code cleanup * review improvements * added api test Co-authored-by: Kibana Machine <[email protected]> * skip flaky suite (#126027) * Remove deprecated api (#125524) * [Fleet] Remove deprecated kibana APIs - License * Remove basePath from FleetApp * Replace AsyncPlugin with Plugin * Get fieldFormats from fieldFormats plugin rather than data plugin * Fix ts errors * Attempt fixing wrong type * Move licenseService to FleetStartDeps * Fix types and mocks Co-authored-by: Kibana Machine <[email protected]> * Upgrade `markdown-it` dependency (`10.0.0` → `12.3.2`). (#125526) * skipping failing tests (#126039) * remove unused deprecated code and use field format plugin directly for data view field editor (#126029) * [data views] Improve preview pane (#126013) * fix preview pane * fix preview pane * one less span tag Co-authored-by: Kibana Machine <[email protected]> * [Alerting] Provide services to set context for recovered alerts (#124972) * Rename alert instance to alert and add create fn to alert factory * Rename alert instance to alert and add create fn to alert factory * Fixing types * Fixing types * Adding flag for rule types to opt into setting recovery context * Only showing context in action variable menu if flag set to true * Adding recovery functions to createAlertFactory * Setting recovery in index threshold and fixing types * Fixing lint issues and some refactoring * Cleanup * Functional tests for index threshold rule recovery context * Return array of recovered alerts instead of record * Fixing types * Fixing types * Cleanup * Handling nulls and more tests * Updating developer docs * Making getRecoveryAlerts non-optional * Setting unknown in index threshold recovery value * PR feedback * Adding a test Co-authored-by: Kibana Machine <[email protected]> * [Discover] Re-introduce saved_searches test (#126059) * [Archive Migration] index pattern without timefield (#125870) * kbn_archive date_nanos * kbn_archive date_nanos in context and discover * kbn_archiver more date_nanos tests * split out kbnArchiver for index_pattern_without_timefield * remove date_nanos files from a different PR * update another test for usage of the same archives * set default index pattern for test * remove duplicate const kibanaServer Co-authored-by: Kibana Machine <[email protected]> * delete unused es_archive visualize_embedding (#126001) * delete unused es_archive * remove other unused es_archive * more unused es_archives Co-authored-by: Kibana Machine <[email protected]> * Bump packages (#126119) * url-parse 1.5.3 -> 1.5.9 * follow-redirects 1.y.z -> 1.14.9 * Add tests for all cases selector and attachments * Add tests for use add to existing case hook * First version of the cases timeline actions * export add alert to new case button from cases plugin * Make Cases ECS compatible with timelines and security_solution * Delete new case button * Add helpers * Use the cases hook directly for add to new case * Remov unused dependencies * Rename callbacks, remove timelines calls * Fixing tests for the dropdown * Fix broken test * mocking cases for tests * Fix detectiosn tests * Observability now uses the new cases hooks * Wrap events viewer into cases context * Open the create case flyout if create case was selected in the modal * Fix cases mocks for security_solution * Update tests * Add tests for use cases toast * Improve cases mock * delete security mock * replace tests mocks for cases * fix import mock * Do not require onRowClick * Show the toast inside the modal * show the toast inside the flyout * remove toast logic from the consumer plugin * fix typescript types * Rename type * Fix broken test * Fix file name and broken test * Use internal navigation hook * Update hook dependencies * Move useCaseToast * Fix mock paths * fix eslint * Add test cases for the toast content * Add cases context to the overview page Co-authored-by: Julia Bardi <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: István Zoltán Szabó <[email protected]> Co-authored-by: Joe Reuter <[email protected]> Co-authored-by: Tiago Costa <[email protected]> Co-authored-by: Cristina Amico <[email protected]> Co-authored-by: Aleh Zasypkin <[email protected]> Co-authored-by: Gloria Hornero <[email protected]> Co-authored-by: Matthew Kime <[email protected]> Co-authored-by: Ying Mao <[email protected]> Co-authored-by: Maja Grubic <[email protected]> Co-authored-by: Lee Drengenberg <[email protected]> Co-authored-by: Joe Portner <[email protected]>

showing agent policy creation error message on UI

1333a06

juliaElastic added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 v8.2.0 labels Feb 17, 2022

juliaElastic requested a review from a team as a code owner February 17, 2022 13:37

juliaElastic self-assigned this Feb 17, 2022

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Feb 17, 2022

nchaulet approved these changes Feb 17, 2022

View reviewed changes

hop-dev approved these changes Feb 17, 2022

View reviewed changes

kpollich approved these changes Feb 17, 2022

View reviewed changes

mapping the error instead of showing from the backend

b6b06d0

Merge branch 'main' into fix-agent-policy-error-message

4f7478d

juliaElastic enabled auto-merge (squash) February 21, 2022 07:34

juliaElastic merged commit 1df6ecf into elastic:main Feb 21, 2022

kibanamachine mentioned this pull request Feb 21, 2022

[8.1] [Fleet] showing agent policy creation error message on UI (#125931) #126043

Merged

juliaElastic deleted the fix-agent-policy-error-message branch February 21, 2022 09:16

amolnater-qasource mentioned this pull request Mar 1, 2022

[Fleet]: No descriptive error message for creating duplicate policy from Add Agent flyout. #125904

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] showing agent policy creation error message on UI #125931

[Fleet] showing agent policy creation error message on UI #125931

juliaElastic commented Feb 17, 2022

elasticmachine commented Feb 17, 2022

nchaulet commented Feb 17, 2022 •

edited

Loading

nchaulet left a comment

hop-dev left a comment

kpollich left a comment

juliaElastic commented Feb 17, 2022 •

edited

Loading

juliaElastic commented Feb 17, 2022

juliaElastic commented Feb 21, 2022

kibana-ci commented Feb 21, 2022

kibanamachine commented Feb 21, 2022

amolnater-qasource commented Mar 1, 2022

[Fleet] showing agent policy creation error message on UI #125931

[Fleet] showing agent policy creation error message on UI #125931

Conversation

juliaElastic commented Feb 17, 2022

Summary

elasticmachine commented Feb 17, 2022

nchaulet commented Feb 17, 2022 • edited Loading

nchaulet left a comment

Choose a reason for hiding this comment

hop-dev left a comment

Choose a reason for hiding this comment

kpollich left a comment

Choose a reason for hiding this comment

juliaElastic commented Feb 17, 2022 • edited Loading

juliaElastic commented Feb 17, 2022

juliaElastic commented Feb 21, 2022

kibana-ci commented Feb 21, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

History

kibanamachine commented Feb 21, 2022

💚 All backports created successfully

Questions ?

amolnater-qasource commented Mar 1, 2022

nchaulet commented Feb 17, 2022 •

edited

Loading

juliaElastic commented Feb 17, 2022 •

edited

Loading