[Actions] Taking space id into account when creating email footer link by ymao1 · Pull Request #100734 · elastic/kibana

ymao1 · 2021-05-26T19:49:55Z

Resolves #99300

Summary

Passing in spaceId to the function that creates the footer link. If spaceId is not default, prepends /s/${spaceId} to the footer link.

To Verify

Create a rule in the default space and in another space. Add an email action to both rules. Verify that the email received has a footer link that correctly resolves to the rule details page, including space id if required.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2021-05-27T14:20:16Z

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

chrisronline

LGTM! Great work! I left one question but it's minor

chrisronline · 2021-05-27T14:43:05Z

x-pack/plugins/alerting/server/task_runner/inject_action_params.ts

  // Inject kibanaFooterLink if action type is email. This is used by the email action type
  // to inject a "View alert in Kibana" with a URL in the email's footer.
  if (actionTypeId === '.email') {
+    const spacePrefix = spaceId.length > 0 && spaceId !== 'default' ? `/s/${spaceId}` : '';


Is the spaceId.length > 0 check necessary? Is it possible to not be in a space, or in a space without a name?

I think it's not possible? I wanted to cover all my bases am happy to remove.

I'd defer to your judgment here, but I wasn't sure if it was possible or not

The only thing I can think of is testing the behaviour when the spaces plugin is disabled xpack.spaces.enabled: false.

Good call @mikecote. spaceId is undefined in this scenario 😅 . Updated to handle that here 64414fc

…ing/email-links-space-id

mikecote

Changes LGTM! I see there were also some conversations about customers forgetting to include Space ID when using {{kibanaBaseUrl}} (#99300 (comment)), should we open a follow-up issue for that scenario?

mikecote · 2021-05-28T13:38:43Z

x-pack/plugins/alerting/server/task_runner/inject_action_params.ts

+        path: `${spacePrefix}/app/management/insightsAndAlerting/triggersActions/rule/${ruleId}`,
        text: i18n.translate('xpack.alerting.injectActionParams.email.kibanaFooterLinkText', {
-          defaultMessage: 'View alert in Kibana',
+          defaultMessage: 'View rule in Kibana',


ymao1 · 2021-05-28T14:04:31Z

@mikecote Created issue here: #100890

ymao1 · 2021-05-28T14:05:08Z

@elasticmachine merge upstream

kibanamachine · 2021-05-28T15:53:02Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💚 Build #127706 succeeded 54faefb
💔 Build #127701 failed 89e113f
💚 Build #127525 succeeded 8d466ed

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

cc @ymao1

elastic#100734) * Taking space id into account when creating email footer link * Handling undefined space when spaces is disabled * Handling undefined space when spaces is disabled Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-05-28T16:00:23Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

#100734) (#100913) * Taking space id into account when creating email footer link * Handling undefined space when spaces is disabled * Handling undefined space when spaces is disabled Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: ymao1 <ying.mao@elastic.co>

* master: (77 commits) [RAC][Security Solution] Register Security Detection Rules with Rule Registry (elastic#96015) [Enterprise Search] Log warning for Kibana/EntSearch version mismatches (elastic#100809) updating the saved objects test to include more saved object types (elastic#100828) [ML] Fix categorization job view examples link when datafeed uses multiple indices (elastic#100789) Fixing ES archive mapping failure (elastic#100835) Fix bug with Observability > APM header navigation (elastic#100845) [Security Solution][Endpoint] Add event filters summary card to the fleet endpoint tab (elastic#100668) [Actions] Taking space id into account when creating email footer link (elastic#100734) Ensure comments on parameters in arrow functions are captured in the docs and ci metrics. (elastic#100823) [Security Solution] Improve find rule and find rule status route performance (elastic#99678) [DOCS] Adds video to introduction (elastic#100906) [Fleet] Improve combo box for fleet settings (elastic#100603) [Security Solution][Endpoint] Endpoint generator and data loader support for Host Isolation (elastic#100813) [DOCS] Adds Lens video (elastic#100898) [TSVB] [Table tab] Fix "Math" aggregation (elastic#100765) chore(NA): moving @kbn/io-ts-utils into bazel (elastic#100810) [Alerting] Adding feature flag for enabling/disabling rule import and export (elastic#100718) [TSVB] Fix Upgrading from 7.12.1 to 7.13.0 breaks TSVB (elastic#100864) [Lens] Adds dynamic table cell coloring (elastic#95217) [Security Solution][Endpoint] Do not display searchbar in security-trusted apps if there are no items (elastic#100853) ...

Taking space id into account when creating email footer link

8d466ed

ymao1 self-assigned this May 27, 2021

ymao1 added Feature:Actions release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.14.0 v8.0.0 labels May 27, 2021

ymao1 marked this pull request as ready for review May 27, 2021 14:20

ymao1 requested a review from a team as a code owner May 27, 2021 14:20

chrisronline self-requested a review May 27, 2021 14:28

chrisronline approved these changes May 27, 2021

View reviewed changes

ymao1 added 3 commits May 27, 2021 11:32

Handling undefined space when spaces is disabled

64414fc

Merge branch 'master' of https://github.com/elastic/kibana into alert…

89e113f

…ing/email-links-space-id

Handling undefined space when spaces is disabled

54faefb

mikecote self-requested a review May 28, 2021 13:29

mikecote approved these changes May 28, 2021

View reviewed changes

Merge branch 'master' into alerting/email-links-space-id

1db3f62

ymao1 added the auto-backport Deprecated - use backport:version if exact versions are needed label May 28, 2021

ymao1 merged commit b4e8cfe into elastic:master May 28, 2021

kibanamachine mentioned this pull request May 28, 2021

[7.x] [Actions] Taking space id into account when creating email footer link (#100734) #100913

Merged

Conversation

ymao1 commented May 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

To Verify

Checklist

Uh oh!

elasticmachine commented May 27, 2021

Uh oh!

chrisronline left a comment

Choose a reason for hiding this comment

Uh oh!

chrisronline May 27, 2021

Choose a reason for hiding this comment

Uh oh!

ymao1 May 27, 2021

Choose a reason for hiding this comment

Uh oh!

chrisronline May 27, 2021

Choose a reason for hiding this comment

Uh oh!

mikecote May 27, 2021

Choose a reason for hiding this comment

Uh oh!

ymao1 May 27, 2021

Choose a reason for hiding this comment

Uh oh!

mikecote left a comment

Choose a reason for hiding this comment

Uh oh!

mikecote May 28, 2021

Choose a reason for hiding this comment

Uh oh!

ymao1 commented May 28, 2021

Uh oh!

ymao1 commented May 28, 2021

Uh oh!

kibanamachine commented May 28, 2021

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

kibanamachine commented May 28, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ymao1 commented May 26, 2021 •

edited

Loading