-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Alerting] Use rule.url to generate kibanaFooterLink for email connectors #160196
[Alerting] Use rule.url to generate kibanaFooterLink for email connectors #160196
Conversation
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Pinging @elastic/response-ops (Team:ResponseOps) |
try { | ||
path = new URL(ruleUrl).pathname; | ||
} catch (e) { | ||
path = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the email connector:
kibana/x-pack/plugins/stack_connectors/server/connector_types/email/index.ts
Lines 411 to 418 in 329cc6a
return i18n.translate('xpack.stackConnectors.email.customViewInKibanaMessage', { | |
defaultMessage: 'This message was sent by Elastic. [{kibanaFooterLinkText}]({link}).', | |
values: { | |
kibanaFooterLinkText: kibanaFooterLink.text, | |
link: `${publicBaseUrl}${kibanaFooterLink.path === '/' ? '' : kibanaFooterLink.path}`, | |
}, | |
}); | |
} |
it does this check link: ${publicBaseUrl}${kibanaFooterLink.path === '/' ? '' : kibanaFooterLink.path}
so should we set path to /
in this error scenario?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes i saw it but "/"
becomes ""
anyway, and if we use it somewhere else without checking the trailing "/"
the full URL becomes broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Resolves: #151355
This PR intends to use
rule.url
(provided by the rule type) in the email connector's footer link rather than the hardcoded path.