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

[Slack] restructure alert actions, add 'visit site' button #3886

Merged

Conversation

DaanMeijer
Copy link
Contributor

@DaanMeijer DaanMeijer commented Oct 13, 2023

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Fixes #3351: adds the monitor url as a button to the slack message. Also, restructured the code which adds the "Visit Uptime Kuma" button to be more readable and modular.

Type of change

Please delete any options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
    (including JSDoc for methods)
  • My changes generate no new warnings
Before After
image image

@louislam louislam added this to the 2.1.0 milestone Oct 13, 2023
@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Oct 13, 2023

(sorry for providing everything as separate reviews, I thought I was in a review session)

Mostly, this looks good, except for a few stylistic things.
Please attach screenshots of the before/after result to make sure that this has been tested.

@septemberservices

This comment was marked as resolved.

CommanderStorm

This comment was marked as resolved.

DaanMeijer and others added 8 commits October 14, 2023 10:24
Fixed casing

Co-authored-by: Frank Elsinga <frank@elsinga.de>
Co-authored-by: Frank Elsinga <frank@elsinga.de>
Co-authored-by: Frank Elsinga <frank@elsinga.de>
Co-authored-by: Frank Elsinga <frank@elsinga.de>
Co-authored-by: Frank Elsinga <frank@elsinga.de>
Co-authored-by: Frank Elsinga <frank@elsinga.de>
@chakflying

This comment was marked as resolved.

@DaanMeijer

This comment was marked as resolved.

server/notification-providers/slack.js Outdated Show resolved Hide resolved
server/notification-providers/slack.js Outdated Show resolved Hide resolved
Co-authored-by: Nelson Chan <3271800+chakflying@users.noreply.github.com>
@chakflying chakflying changed the title fix for #3351: restructure alert actions, add 'visit site' button Improve Slack notification: restructure alert actions, add 'visit site' button Dec 2, 2023
@chakflying chakflying added the area:notifications Everything related to notifications label Dec 2, 2023
@CommanderStorm CommanderStorm changed the title Improve Slack notification: restructure alert actions, add 'visit site' button [Slack] restructure alert actions, add 'visit site' button Dec 17, 2023
@jaknz

This comment was marked as off-topic.

@CommanderStorm

This comment was marked as resolved.

@CommanderStorm
Copy link
Collaborator

Thanks for the change to the notification provider!

All changes in this PR are small and uncontroversial
⇒ pulling out of 2.1 and merging into 2.0 with junior maintainer approval

@CommanderStorm CommanderStorm merged commit effd019 into louislam:master Apr 2, 2024
14 checks passed
@DaanMeijer DaanMeijer deleted the fix/issue-3351 branch April 10, 2024 15:39
@kmcrawford
Copy link

I see this is merged, do we know when it will be released?

@CommanderStorm
Copy link
Collaborator

We have written down what needs to happen before v2.0.0-beta1 can happen in #4500

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:notifications Everything related to notifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Slack] Include URL in slack alert
7 participants