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 support for access request notifications #41621

Merged
merged 1 commit into from
May 22, 2024

Conversation

rudream
Copy link
Contributor

@rudream rudream commented May 15, 2024

Purpose

Part of #37704

Adds support for access request notifications. Creating an access request will notify users capable of reviewing it, and approving or denying it will notify the requester.

e counterpart: https://github.com/gravitational/teleport.e/pull/4173

Demo

From reviewer POV:

image

From requester POV:

image

Access request assumed state:

image

Access request not yet assumable (if it's scheduled) state:

image

Access request assume error state:

image

changelog: Added access request notifications.

@github-actions github-actions bot requested review from avatus and gzdunek May 15, 2024 21:29
@rudream rudream force-pushed the yassine/notifications/access-requests branch from e5ec220 to ebf46c5 Compare May 15, 2024 21:47
Base automatically changed from yassine/notifications/endpoints to master May 15, 2024 21:54
@rudream rudream force-pushed the yassine/notifications/access-requests branch 2 times, most recently from 13ed4d7 to 847ff09 Compare May 16, 2024 10:33
Copy link
Contributor

@gzdunek gzdunek left a comment

Choose a reason for hiding this comment

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

Looks good, although I didn't test it, I will try to find time tomorrow.

@rudream rudream requested a review from avatus May 16, 2024 18:16
@rudream rudream force-pushed the yassine/notifications/access-requests branch from 43a41ca to aed8ab5 Compare May 16, 2024 18:17
@rudream rudream requested a review from gzdunek May 17, 2024 18:05
@gzdunek
Copy link
Contributor

gzdunek commented May 20, 2024

Idk if it's just me, but the current buttons for toggling the notification kind seem too large 😅
I tried decreasing the font size and overall height:
image

If you agree, I'd ask the design team for a confirmation.

Copy link
Contributor

@avatus avatus left a comment

Choose a reason for hiding this comment

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

love it!

lib/auth/auth.go Outdated Show resolved Hide resolved
@rudream rudream force-pushed the yassine/notifications/access-requests branch from 578b385 to 3268332 Compare May 21, 2024 18:49
@rudream rudream enabled auto-merge May 21, 2024 18:50
@rudream rudream force-pushed the yassine/notifications/access-requests branch from cba98ac to 6605d07 Compare May 21, 2024 21:34
@rudream rudream force-pushed the yassine/notifications/access-requests branch from 6605d07 to 1f53170 Compare May 22, 2024 01:50
@rudream rudream added this pull request to the merge queue May 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 22, 2024
@rudream rudream added this pull request to the merge queue May 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 22, 2024
@rudream rudream added this pull request to the merge queue May 22, 2024
Merged via the queue into master with commit 9e4beb3 May 22, 2024
42 checks passed
@rudream rudream deleted the yassine/notifications/access-requests branch May 22, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants