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

Add a link text label to the ClusterAlert resource #45972

Merged
merged 7 commits into from
Sep 5, 2024
Merged

Conversation

bl-nero
Copy link
Contributor

@bl-nero bl-nero commented Aug 28, 2024

This label allows to customize text on a button that contains the alert link.

Background: the newly designed banner alert UX changes the way we render links in cluster alerts. Instead of making the entire message a link, we display the message as regular text, and add an action button that opens a link. This means we need a text for that action button, and it's going to be different for different types of actions.

Since we already use labels for the link URL itself, we introduce another one for the call-to-action text.

This PR will be backported, even though the UI itself is specific to v17, since the alerts may be generated in a version prior to the one that displays them.

Tested manually (together with the UI):

  • Alert about too many Windows desktops — by adding 6 non-AD Windows desktops
  • Alert about inability to report usage — by configuring a phony Prehog URI.
  • Custom alerts - by using tctl alerts create.

Follow-up UI PR: #45973.
Follow-up Enterprise PR: https://github.com/gravitational/teleport.e/pull/4951
Contributes to #44595.

This label allows to customize text on a button that contains the alert
link.
@bl-nero bl-nero added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 labels Aug 28, 2024
api/types/cluster_alert.go Outdated Show resolved Hide resolved
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
@bl-nero bl-nero enabled auto-merge August 29, 2024 09:44
@bl-nero bl-nero disabled auto-merge August 29, 2024 12:25
@bl-nero bl-nero requested a review from zmb3 August 29, 2024 12:26
u, err := url.Parse(val)
if err != nil {
return trace.BadParameter("invalid alert: label link %q is not a valid URL", val)
}
if u.Hostname() != validLinkDestination {
return trace.BadParameter("invalid alert: label link not allowed %q", val)
}
case AlertLinkCTA:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
case AlertLinkCTA:
case AlertLinkText:

@bl-nero bl-nero changed the title Add a CTA label to the ClusterAlert resource Add a link text label to the ClusterAlert resource Sep 2, 2024
@bl-nero
Copy link
Contributor Author

bl-nero commented Sep 2, 2024

@zmb3 I renamed the entire CTA concept per your suggestion in the UI PR.

@bl-nero bl-nero requested a review from zmb3 September 2, 2024 11:46
@bl-nero bl-nero added this pull request to the merge queue Sep 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 4, 2024
@bl-nero bl-nero added this pull request to the merge queue Sep 4, 2024
github-merge-queue bot pushed a commit that referenced this pull request Sep 4, 2024
* Add an alert CTA label

This label allows to customize text on a button that contains the alert
link.

* Comment

* Update api/types/cluster_alert.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Rename CTA to "link text"

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 4, 2024
@bl-nero bl-nero added this pull request to the merge queue Sep 5, 2024
Merged via the queue into master with commit 2092ce7 Sep 5, 2024
40 checks passed
@bl-nero bl-nero deleted the bl-nero/banners branch September 5, 2024 10:42
@public-teleport-github-review-bot

@bl-nero See the table below for backport results.

Branch Result
branch/v16 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v16 no-changelog Indicates that a PR does not require a changelog entry size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants