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

Fix few minor Slack connection issues #3327

Merged
merged 10 commits into from
Nov 13, 2023

Conversation

joeyorlando
Copy link
Contributor

@joeyorlando joeyorlando commented Nov 10, 2023

What this PR does

Closes https://github.com/grafana/oncall-private/issues/2289

  • Fix issue where if you try connecting your Slack user to your OnCall user and the first time around you encounter an error (ex. connecting to the wrong Slack workspace), you will see the same error banner message despite a successful connection. Now we clear the session upon successful connection to ensure that you will not see any previously encountered errors.

  • Fix some alignment issues on the Slack connection buttons
    Before

    Screenshot 2023-11-10 at 15 07 48 Screenshot 2023-11-10 at 15 16 22

    After

    Screenshot 2023-11-10 at 15 10 28 Screenshot 2023-11-10 at 15 16 42
  • On the "User Info" user settings modal tab, render display_name instead of slack_login. Currently we prefix @ before slack_login, which is a bit confusing as it makes you think that this is the handle you would use to @ your user in Slack. display_name corresponds to the handle that would be used to @ your user

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • CHANGELOG.md updated (or pr:no changelog PR label added if not required)

@joeyorlando joeyorlando added pr:no changelog pr:no public docs Added to a PR that does not require public documentation updates labels Nov 10, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

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

minor grammar fixes

@joeyorlando joeyorlando marked this pull request as ready for review November 10, 2023 21:26
@joeyorlando joeyorlando requested review from a team November 10, 2023 21:26
@joeyorlando joeyorlando added this pull request to the merge queue Nov 13, 2023
Merged via the queue into dev with commit dcf0842 Nov 13, 2023
@joeyorlando joeyorlando deleted the jorlando/fix-slack-connection-issue branch November 13, 2023 12:50
brojd pushed a commit that referenced this pull request Sep 18, 2024
# What this PR does

Closes grafana/oncall-private#2289

- Fix issue where if you try connecting your Slack user to your OnCall
user and the first time around you encounter an error (ex. connecting to
the wrong Slack workspace), you will see the same error banner message
despite a successful connection. Now we clear the session upon
successful connection to ensure that you will not see any previously
encountered errors.
- Fix some alignment issues on the Slack connection buttons
  **Before**

<img width="564" alt="Screenshot 2023-11-10 at 15 07 48"
src="https://github.com/grafana/oncall/assets/9406895/1a256ae9-668c-4a76-b101-cb4e0af29c43">
<img width="789" alt="Screenshot 2023-11-10 at 15 16 22"
src="https://github.com/grafana/oncall/assets/9406895/61649d9d-2f11-497a-9f2d-274c6dbe90eb">

  **After**

<img width="470" alt="Screenshot 2023-11-10 at 15 10 28"
src="https://github.com/grafana/oncall/assets/9406895/92c0f1d3-75eb-4de2-a3d3-4e014489f970">
<img width="738" alt="Screenshot 2023-11-10 at 15 16 42"
src="https://github.com/grafana/oncall/assets/9406895/dc825266-c45e-4c48-af78-0cd5c3c5b64e">

- On the "User Info" user settings modal tab, render `display_name`
instead of `slack_login`. Currently we prefix `@` before `slack_login`,
which is a bit confusing as it makes you think that this is the handle
you would use to `@` your user in Slack. `display_name` corresponds to
the handle that would be used to `@` your user

## Checklist

- [ ] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants