Skip to content

Alerts slack integration UI - #4509

Merged
AdityaHegde merged 9 commits into
mainfrom
adityahegde/alerts-slack-integration-ui
Apr 8, 2024
Merged

Alerts slack integration UI#4509
AdityaHegde merged 9 commits into
mainfrom
adityahegde/alerts-slack-integration-ui

Conversation

@AdityaHegde

@AdityaHegde AdityaHegde commented Apr 4, 2024

Copy link
Copy Markdown
Collaborator

closes #4410

Adds a section to add slack notifications if it is enabled.

Run rill env set "connector.slack.bot_token" "xoxb-..." --project ... to enable slack connector.

@AdityaHegde
AdityaHegde requested a review from ericpgreen2 April 4, 2024 12:39
@AdityaHegde
AdityaHegde marked this pull request as ready for review April 4, 2024 12:39
@ericpgreen2

ericpgreen2 commented Apr 6, 2024

Copy link
Copy Markdown
Contributor

I'm seeing the Slack section, even though I haven't yet enabled the connector. If the connector hasn't been enabled, we should probably replace the section (form inputs) with a note. Something like: "Slack has not been configured for this project. Read the docs (link) to learn more."

image

@ericpgreen2 ericpgreen2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code looks good 👍

export let title: string;
export let description: string = "";
export let padding = "p-3";
export let showEnabled = false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Name suggestion: showSectionToggle

} from "./form-utils";

export let formState: any; // svelte-forms-lib's FormState
export let formState: ReturnType<typeof createForm<AlertFormValues>>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great to see this typing 👍

@AdityaHegde

Copy link
Copy Markdown
Collaborator Author

Good catch about the slack showing up. There was a bug in how i used the signal.

@AdityaHegde
AdityaHegde merged commit b9fb580 into main Apr 8, 2024
@AdityaHegde
AdityaHegde deleted the adityahegde/alerts-slack-integration-ui branch April 8, 2024 06:25
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.

Alerts: Add Slack destination option to UI

2 participants