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

[Actions] Adds a "Test Connector" button on the Connectors List to make discovery of the Test tab easier #78746

Merged
merged 18 commits into from
Sep 30, 2020

Conversation

gmmorris
Copy link
Contributor

@gmmorris gmmorris commented Sep 29, 2020

Summary

Follow up from #77365
Adds a "Test Connector" button on the Connectors List to make discovery of the Test tab easier.
It also adds a "Save & test" button when creating a Connector, which navigates the user straight to the Test tab after saving.

This also fixed a broken behaviour on Main where navigating to the Test tab in Case Management crashes Kibana.

Run button in Connectors list

Screenshot 2020-09-16 at 18 29 13

Checklist

Delete any items that are not applicable to this PR.

For maintainers

* master: (65 commits)
  [Security Solution][Resolver] Analyzed event styling (elastic#77115)
  filter invalid SOs from the searc hresults in Task Manager (elastic#76891)
  [RUM Dashboard] Visitors by region map (elastic#77135)
  [Security Solution][Endpoint][Admin] Task/endpoint list actions (elastic#76555)
  [Ingest pipelines] Forms for processors T-U (elastic#76710)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  ...
* master: (76 commits)
  Fixing service maps API test (elastic#77586)
  [Grok] Fix missing error message in error toasts (elastic#77499)
  [Alerting] Exempt Alerts pre 7.10 from RBAC on their Action execution until updated (elastic#75563)
  [ML] fix type in apply_influencer_filters_action (elastic#77495)
  [Lens] create reusable component for filters and range aggregation (elastic#77453)
  fix eslint violations
  Collapse alert chart previews by default (elastic#77479)
  [ML] Fixing field caps wrapper endpoint (elastic#77546)
  showing service maps when filte by environment not defined (elastic#77483)
  [Lens] Settings panel redesign and separate settings per y axis (elastic#76373)
  log request body in new ES client (elastic#77150)
  use `navigateToUrl` to navigate to recent nav links (elastic#77446)
  Move core config service to `kbn/config` package (elastic#76874)
  [UBI] Copy license to /licenses folder (elastic#77563)
  Skip flaky Events Viewer Cypress test
  [Lens] Remove dynamic names in telemetry fields (elastic#76988)
  [Maps] Add DynamicStyleProperty#getMbPropertyName and DynamicStyleProperty#getMbPropertyValue (elastic#77366)
  [Enterprise Search] Add flag to restrict width of layout (elastic#77539)
  [Security Solutions][Cases - Timeline] Fix bug when adding a timeline to a case (elastic#76967)
  [Security Solution][Detections] Integration test for Editing a Rule (elastic#77090)
  ...
* master: (365 commits)
  making expression debug info serializable (elastic#78727)
  fix lodahs imports in app-arch code (elastic#78582)
  Make Field a React.lazy export (elastic#78483)
  [Security Solution] Improves detections tests (elastic#77295)
  [TSVB] Different field format on different series is ignored (elastic#78138)
  RFC: Improve saved object migrations (elastic#66056)
  [Security Solution] Fixes url timeline flaky test (elastic#78556)
  adds retryability feature (elastic#78611)
  Aligns several module versions across the repository (elastic#78327)
  Empty prompt and loading spinner for service map (elastic#78382)
  Change progress bar to spinner (elastic#78460)
  [QA][Code Coverage] Coverage teams lookup w/o Additional Config (elastic#77111)
  Slim down core bundle (elastic#75912)
  [Alerting] retry internal OCC calls within alertsClient (elastic#77838)
  [kbn/optimizer] only build xpack examples when building xpack plugins (elastic#78656)
  [Ingest Manager] Ingest setup upgrade (elastic#78081)
  [Ingest Manager] Surface saved object client 10,000 limitation to bulk actions UI (elastic#78520)
  fix name without a category or if field end with .text (elastic#78655)
  [Security Solution] [Detections] Log message enhancements (elastic#78429)
  [ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url (elastic#78303)
  ...
@gmmorris gmmorris requested review from a team as code owners September 29, 2020 10:09
@gmmorris gmmorris added Feature:Actions release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Sep 29, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@gmmorris gmmorris requested a review from cnasikas September 29, 2020 11:56
Copy link
Contributor

@andreadelrio andreadelrio left a comment

Choose a reason for hiding this comment

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

Design changes look good, just one comment about the sass file.

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

My proposal is to use kibanaReact and useKibana object for adding all common dependancies - this will allow us to remove a lot of repeating code that we path throught the components props. All the plugins have this Kibana context available. I'm not sure how much time it will take for refactoring this, maybe a follow up issue should be created. But we definitely should switch to kibanaReact.

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

code LGTM and worked as expected

I did notice clicking the triangle on the right in the actions list causes the same error described here, for pre-configured email connectors: #78892 .

I feel like we should probably be doing e2e testing on both a dynamically created connector and a pre-configured connector - I noticed the prior PR only has a test for a dynamically created connector. Open an issue to track if you agree, or maybe it's simple to add a new one.

Email is a good one as it has config, secrets, and params. There's a special value of the config param service we can use that provides a loop-back response, without having to have a real email server running (which we don't have right now); relevant code here:

if (service === JSON_TRANSPORT_SERVICE) {
transportConfig.jsonTransport = true;
delete transportConfig.auth;
} else if (service != null) {

Co-authored-by: Andrea Del Rio <delrio.andre@gmail.com>
@gmmorris
Copy link
Contributor Author

My proposal is to use kibanaReact and useKibana object for adding all common dependancies - this will allow us to remove a lot of repeating code that we path throught the components props. All the plugins have this Kibana context available. I'm not sure how much time it will take for refactoring this, maybe a follow up issue should be created. But we definitely should switch to kibanaReact.

Thats a great idea!
As this refactor is broader than this issue (and Main is broken on this right now), I'd suggest making that a separate issue.

I'll open an issue.

@gmmorris
Copy link
Contributor Author

code LGTM and worked as expected

I did notice clicking the triangle on the right in the actions list causes the same error described here, for pre-configured email connectors: #78892 .

I feel like we should probably be doing e2e testing on both a dynamically created connector and a pre-configured connector - I noticed the prior PR only has a test for a dynamically created connector. Open an issue to track if you agree, or maybe it's simple to add a new one.

Email is a good one as it has config, secrets, and params. There's a special value of the config param service we can use that provides a loop-back response, without having to have a real email server running (which we don't have right now); relevant code here:

if (service === JSON_TRANSPORT_SERVICE) {
transportConfig.jsonTransport = true;
delete transportConfig.auth;
} else if (service != null) {

This seems unrelated to this PR, and should be fixed in #78892
If we add the missing e2e test in this PR it'll block this PR, as the breakage isn't introduced in this PR.

I'll prioritise #78892 as it feels like a 7.10 blocker to me.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id value diff baseline
triggers_actions_ui 1.5MB +3.3KB 1.5MB

History

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

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for opening the issue!

@gmmorris gmmorris merged commit ac18bc0 into elastic:master Sep 30, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 30, 2020
…ke discovery of the Test tab easier (elastic#78746)

Adds a "Test Connector" button on the Connectors List to make discovery of the Test tab easier.
It also adds a "Save & test" button when creating a Connector, which navigates the user straight to the Test tab after saving.

This also fixed a broken behaviour on Main where navigating to the Test tab in Case Management crashes Kibana.
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 30, 2020
* master: (97 commits)
  [Actions] Adds a "Test Connector" button on the Connectors List to make discovery of the Test tab easier (elastic#78746)
  [Discover] Fix functional time picker test permissions (elastic#78564)
  [ML] Fixing module datafeed overrides (elastic#78925)
  Adds some missing licenses to the CSV export (elastic#78719)
  [dev/cli] ensure plugins/ and all watch source dirs exist (elastic#78973)
  [Lens] Stop using scripted metric to collect telemetry (elastic#78687)
  [Lens] fix wrong message in fields accordion (elastic#78924)
  [Enterprise Search][App Search] Credentials Logic updates (elastic#78644)
  [Monitoring] Disk usage alerting (elastic#75419)
  [SECURITY_SOLUTION] Trusted apps list expand/collapse details (elastic#78601)
  Update content on interstitial page (elastic#78881)
  chore(NA): include hjson as a prod dependency (elastic#78941)
  Fix empty meta fields input in Advanced Settings  (elastic#78576)
  [Lens] Maintain order of operations in dimension panel (elastic#78864)
  Fix plugin doc title (elastic#78880)
  load apm-rum agent lazily (elastic#78760)
  [ML] Skip full ML access permission test
  Optimize charts plugin (elastic#78922)
  ui_actions service initial docs (elastic#78902)
  skip failing suite (elastic#78942)
  ...
phillipb added a commit to phillipb/kibana that referenced this pull request Sep 30, 2020
…aly-detection-partition-field

* 'master' of github.com:elastic/kibana: (37 commits)
  Fixes for the Ticket 78375 (elastic#79004)
  [Security] Alert Telemetry for the Security app (elastic#77200)
  [Search bar] Remove duplicate `popoverProps` (elastic#79025)
  [Security Solution][Detections] Add rule overrides for single event EQL rules (elastic#78876)
  [SECURITY_SOLUTION][ENDPOINT] Improve Endpoint Host data generator to also integrate with Ingest (elastic#74305)
  remove file accidentally checked in (elastic#79005)
  [ML] DF Analytics creation wizard: replace select input with job type cards with icons (elastic#78872)
  [Design] A couple fixes for 7.10 (elastic#78801)
  Fix KQL autocomplete value suggestions (elastic#78676)
  [Security Solution][Resolver] New mock with cursor (elastic#78863)
  Embeddables: basic documentation (elastic#78900)
  [security solution] only import beat_schema when needed (elastic#78708)
  [Reporting] API Integration tests: fix flaky tests for Spaces CSV formatting (elastic#78849)
  [Actions] Adds a "Test Connector" button on the Connectors List to make discovery of the Test tab easier (elastic#78746)
  [Discover] Fix functional time picker test permissions (elastic#78564)
  [ML] Fixing module datafeed overrides (elastic#78925)
  Adds some missing licenses to the CSV export (elastic#78719)
  [dev/cli] ensure plugins/ and all watch source dirs exist (elastic#78973)
  [Lens] Stop using scripted metric to collect telemetry (elastic#78687)
  [Lens] fix wrong message in fields accordion (elastic#78924)
  ...
gmmorris added a commit that referenced this pull request Sep 30, 2020
…ke discovery of the Test tab easier (#78746) (#79008)

Adds a "Test Connector" button on the Connectors List to make discovery of the Test tab easier.
It also adds a "Save & test" button when creating a Connector, which navigates the user straight to the Test tab after saving.

This also fixed a broken behaviour on Main where navigating to the Test tab in Case Management crashes Kibana.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Actions release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants