Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cases] ServiceNow connectors UI changes #114234

Merged
merged 134 commits into from
Oct 18, 2021

Conversation

semd
Copy link
Contributor

@semd semd commented Oct 7, 2021

Summary

Design improvements for #105440
Design ticket: https://github.com/elastic/stack-design-team/issues/109
Design Round 2: https://whimsical.com/servicenow-certification-ux-updates-FtUbWGLgxXppnWNSnwabwu

Note: this branch was created from #105440 branch so it contains all of its commits, though they are all already in master.

List of changes:

Connector's list:

  • icon margin fixed

Screenshot 2021-10-14 at 3 05 27 PM

Edit connector form:

  • Reduced distance between "Authenticate" title and info callout below

edit_connector

Update connector form:

  • From a modal to a flyout (on top of "Edit connector" flyout)
  • Warning flyout banner added

update_connector

Dropdown deprecated connectors updated:

  • icon margin fixed
  • (deprecated) text added

dropdown

Edit connector in case:

  • warning color changed from danger to warning in the form callout
  • connector icon moved from the left (EuiCard icon) to a new column aligned to the right

edit_connector
card

Connector's parameters form:

  • After talking with the Alerting team the toggle/checkbox to update incident has been removed and got replaced by a text field (Correlation ID)
  • Add Correlation display text field
  • Remove observables fields from SIR (destination IP, source IP, malware hash and malware URL)

ITSM:
Screenshot 2021-10-14 at 3 09 38 PM

SIR:
Screenshot 2021-10-14 at 3 13 57 PM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making those edits!

@jonathan-buttner
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other copy suggestions:

Deprecation callout

Title: This connector type is deprecated
Description: Update this connector or create a new one.

ServiceNowInstance URL

Add ending period to this sentence: Include the full URL.

Authentication

You must authenticate each time you edit the connector.

Malware Hashes

Sentence case? Malware hashes

children: (
<FormattedMessage
id="xpack.triggersActionsUI.components.builtinActionTypes.serviceNowAction.serviceNowAppRunning"
defaultMessage="The Elastic App from the ServiceNow App Store must be installed prior to running the update. {visitLink} to install the app"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

Go to the ServiceNow app store to install the Elastic app.

where "ServiceNow app store" is the link.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! The design team wanted to be like this (a text and a button at the bottom):

Screenshot 2021-10-15 at 11 56 15 AM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comments here @gchaps , our thinking for this was to provide the link to the app store prominently since it's a requirement to use the connector—so I'd like to try and keep that if possible.
Are you ok merging this as is and then doing a fix post FF so we don't block them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, i like this design. Here is a suggestion for the text, including ending period and consistent capitalization for ServiceNow App Store.

To use this connector, first install the Elastic app from the ServiceNow app store.

color="warning"
title={i18n.translate(
'xpack.triggersActionsUI.sections.actionsConnectorsList.connectorsListTable.columns.actions.legacyConnectorTitle',
{ defaultMessage: 'Deprecated connector' }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about making this text similar to the deprecation message in the callout. Also as this is a tooltip, how about putting it all together instead of title & description.

This connector is deprecated. Update it or create a new one.

Copy link
Member

@cnasikas cnasikas Oct 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will delegate that to @mdefazio 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I'm all for making it more succinct!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!
Screenshot 2021-10-15 at 6 38 54 PM
Screenshot 2021-10-15 at 6 37 00 PM

@cnasikas cnasikas force-pushed the 1769_connectors_ui_changes branch from b35ae77 to 2937675 Compare October 15, 2021 15:24
Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with a few comments

children: (
<FormattedMessage
id="xpack.triggersActionsUI.components.builtinActionTypes.serviceNowAction.serviceNowAppRunning"
defaultMessage="The Elastic App from the ServiceNow App Store must be installed prior to running the update. {visitLink} to install the app"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, i like this design. Here is a suggestion for the text, including ending period and consistent capitalization for ServiceNow App Store.

To use this connector, first install the Elastic app from the ServiceNow app store.

@semd semd enabled auto-merge (squash) October 18, 2021 08:23
@cnasikas cnasikas added the auto-backport Deprecated - use backport:version if exact versions are needed label Oct 18, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
triggersActionsUi 378 379 +1

Async chunks

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

id before after diff
cases 316.6KB 317.5KB +900.0B
triggersActionsUi 793.5KB 789.4KB -4.1KB
total -3.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 302.8KB 302.9KB +58.0B

History

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

cc @semd

@semd semd merged commit c80b96c into elastic:master Oct 18, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Oct 18, 2021
* POC

* Before and after saving connector callbacks

* Draft callbacks on SN

* Migrate legacy connectors

* Add deprecated connector

* Fix callbacks types

* Pass isEdit to connector forms

* Get application info hook

* Validate instance on save

* Support both legacy and new app

* Seperate SIR

* Log application version & and throw otherwise

* Deprecated tooltip cases

* Deprecated tooltip alerts

* Improve message

* Improve translation

* Change to elastic table & fix types

* Add callbacks to add modal

* Pass new props to tests

* Change health api url to production

* Better installation message

* Migrate connectors functionality

* Change migration version to 7.16

* Fix bug

* Improve message

* Use feature flag

* Create credentials component

* Add form to migration modal

* Improve installation callout

* Improve deprecated callout

* Improve modal

* Improve application required modal

* Improve SN form

* Support both connectors

* Support correlation attributes

* Use same component for SIR

* Prevent using legacy connectors when creating a case

* Add observables

* Unique observables

* Push only if there are observables

* Change labels to plural

* Pass correlation ID and value

* Show errors on the callout

* Improve alerts tooltip

* Improve cases tooltip

* Warning callout on cases configuration page

* Fix tooltip content

* Add help text

* Change from string to array

* Fix i18n

* Fix spelling

* Update incidents for ITSM

* Update incidents for SIR

* Fix types

* Fix backend tests

* Fix frontend tests

* Add service tests

* Fix i18n

* Fix cypress test

* Improve ServiceNow intergration tests

* Fix cases integration tests

* Fix triggers actions ui end to end test

* Fix tests

* Rename modal

* Show error message on modal

* Create useOldConnector helper

* Show the update incident toggle only on new connectors

* Add observables for old connectors

* Fix error when obs are empty

* Enable SIR for alerts

* Fix types

* Improve combineObservables

* Add test for the sir api

* Add test for the sir service

* Add documentation

* PR feedback

* Improve cases deprecated callouts

* Improve observables format

* Add integration tests for SIR

* Fix doc error

* Add config tests

* Add getIncident tests

* Add util tests

* Add migration tests

* Add tests for connectors and improve callouts

* Add more tests

* Add more UI tests

* update connector modal to flyout

* PR feedback

* Test CI

* restore auth callout

* edit connector form spacing

* Improve integration tests

* Add 8 pixels to the left of the connector icon

* update switch to checkboxes

* case detail ui

* Seperate ServiceNow integration tests

* Remove observables fields

* Add correlation values

* Fix merge

* add deprecated text in the dropdown

* update card icon to the right

* new update connetor test and other tests fixes

* PR feedback

* Remove observables from docs

* Remove unused translations

* Using eui theme for styling

* Content feeback

* Add more unit tests

* Fix i18n

* Fix types

* Fixes

* Fixes

* test properly

* fix duplicated translation

* Simplify tooltip

* Writing feedback

Co-authored-by: Christos Nasikas <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Oct 18, 2021
* POC

* Before and after saving connector callbacks

* Draft callbacks on SN

* Migrate legacy connectors

* Add deprecated connector

* Fix callbacks types

* Pass isEdit to connector forms

* Get application info hook

* Validate instance on save

* Support both legacy and new app

* Seperate SIR

* Log application version & and throw otherwise

* Deprecated tooltip cases

* Deprecated tooltip alerts

* Improve message

* Improve translation

* Change to elastic table & fix types

* Add callbacks to add modal

* Pass new props to tests

* Change health api url to production

* Better installation message

* Migrate connectors functionality

* Change migration version to 7.16

* Fix bug

* Improve message

* Use feature flag

* Create credentials component

* Add form to migration modal

* Improve installation callout

* Improve deprecated callout

* Improve modal

* Improve application required modal

* Improve SN form

* Support both connectors

* Support correlation attributes

* Use same component for SIR

* Prevent using legacy connectors when creating a case

* Add observables

* Unique observables

* Push only if there are observables

* Change labels to plural

* Pass correlation ID and value

* Show errors on the callout

* Improve alerts tooltip

* Improve cases tooltip

* Warning callout on cases configuration page

* Fix tooltip content

* Add help text

* Change from string to array

* Fix i18n

* Fix spelling

* Update incidents for ITSM

* Update incidents for SIR

* Fix types

* Fix backend tests

* Fix frontend tests

* Add service tests

* Fix i18n

* Fix cypress test

* Improve ServiceNow intergration tests

* Fix cases integration tests

* Fix triggers actions ui end to end test

* Fix tests

* Rename modal

* Show error message on modal

* Create useOldConnector helper

* Show the update incident toggle only on new connectors

* Add observables for old connectors

* Fix error when obs are empty

* Enable SIR for alerts

* Fix types

* Improve combineObservables

* Add test for the sir api

* Add test for the sir service

* Add documentation

* PR feedback

* Improve cases deprecated callouts

* Improve observables format

* Add integration tests for SIR

* Fix doc error

* Add config tests

* Add getIncident tests

* Add util tests

* Add migration tests

* Add tests for connectors and improve callouts

* Add more tests

* Add more UI tests

* update connector modal to flyout

* PR feedback

* Test CI

* restore auth callout

* edit connector form spacing

* Improve integration tests

* Add 8 pixels to the left of the connector icon

* update switch to checkboxes

* case detail ui

* Seperate ServiceNow integration tests

* Remove observables fields

* Add correlation values

* Fix merge

* add deprecated text in the dropdown

* update card icon to the right

* new update connetor test and other tests fixes

* PR feedback

* Remove observables from docs

* Remove unused translations

* Using eui theme for styling

* Content feeback

* Add more unit tests

* Fix i18n

* Fix types

* Fixes

* Fixes

* test properly

* fix duplicated translation

* Simplify tooltip

* Writing feedback

Co-authored-by: Christos Nasikas <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>

Co-authored-by: Sergi Massaneda <[email protected]>
Co-authored-by: Christos Nasikas <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants