Skip to content

Add messaging providers route - #602

Merged
TorstenDittmann merged 9 commits into
feat-messaging-overviewfrom
feat-messaging-providers
Jan 2, 2024
Merged

Add messaging providers route#602
TorstenDittmann merged 9 commits into
feat-messaging-overviewfrom
feat-messaging-providers

Conversation

@stnguyen90

@stnguyen90stnguyen90 commented Oct 24, 2023

Copy link
Copy Markdown
Contributor

What does this PR do?

Add messaging providers route

Test Plan

TBD

Related PRs and Issues

Parent:

Children:

Have you read the Contributing Guidelines on issues?

Yes

@stnguyen90
stnguyen90 marked this pull request as draft October 24, 2023 23:57
@vercel

vercelBot commented Oct 24, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

NameStatusPreviewCommentsUpdated (UTC)
console✅ Ready (Inspect)Visit Preview💬 Add feedbackJan 2, 2024 2:53pm
console-cloud🛑 Canceled (Inspect)Jan 2, 2024 2:53pm
console-preview✅ Ready (Inspect)Visit Preview💬 Add feedbackJan 2, 2024 2:53pm
console-preview-cloud🛑 Canceled (Inspect)Jan 2, 2024 2:53pm

{ id: 'status', title: 'Status', show: true }
]);

export const providers = {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@TorstenDittmann, what do you think about having this JSON for the wizard for all providers. I wanted to try and minimize the number of places you'd need to go if we add a messaging provider in the future.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Torsten: for now, let's continue with this.

Comment threadsrc/lib/components/labelCard.svelte
try {
let response = { $id: '', name: '' };
const providerId = $providerParams[$provider].providerId || ID.unique();
switch ($provider) {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@TorstenDittmann, this is going to be long and annoying because there's a different method for each provider. Any thoughts on how to improve this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Torsten: for now, let's go with this.

Comment on lines +28 to +103
export const providerParams = writable<{ mailgun: Partial<MailgunProviderParams> }>({
mailgun: null
});

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@TorstenDittmann, I wanted to do this to make the create/update part easier, but do you think it's not worth it and I should just have a writable for each provider?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Torsten: for now, let's go with this.

@stnguyen90
stnguyen90force-pushed the feat-messaging-overview branch from fe1ab68 to e84bea0CompareNovember 24, 2023 19:55
@stnguyen90
stnguyen90force-pushed the feat-messaging-providers branch from 190cb67 to 4f8e32bCompareNovember 24, 2023 19:59
@stnguyen90
stnguyen90force-pushed the feat-messaging-overview branch 2 times, most recently from 99d1eb1 to 5ec3593CompareNovember 29, 2023 04:38
@stnguyen90
stnguyen90force-pushed the feat-messaging-providers branch 2 times, most recently from 180624d to 525401cCompareNovember 29, 2023 04:46
@stnguyen90
stnguyen90force-pushed the feat-messaging-overview branch 2 times, most recently from d4d6578 to c8e1786CompareNovember 29, 2023 23:14
@stnguyen90
stnguyen90force-pushed the feat-messaging-providers branch from 525401c to f46f432CompareNovember 29, 2023 23:19
@stnguyen90stnguyen90 mentioned this pull request Nov 30, 2023
Comment threadsrc/routes/console/project-[project]/messaging/providers/update.svelte Outdated
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@stnguyen90@TorstenDittmann