Skip to content

WIP: webhook demo #508

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

Merged
merged 5 commits into from
Oct 30, 2021
Merged

WIP: webhook demo #508

merged 5 commits into from
Oct 30, 2021

Conversation

ShMcK
Copy link
Member

@ShMcK ShMcK commented Aug 29, 2021

Webhook API for coderoad.

Signed-off-by: shmck [email protected]

Signed-off-by: shmck <[email protected]>
@ShMcK ShMcK mentioned this pull request Aug 29, 2021
Copy link
Contributor

@moT01 moT01 left a comment

Choose a reason for hiding this comment

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

This seems to be working on my testing - nice job @ShMcK 👍

Left a few suggestions.

Edit: Wondering what happens if the request doesn't go through. At the moment, I only plan on having an event for completing an entire tutorial, which would send one request at the end. If it doesn't go through, users would have to do the whole tutorial again. It's unlikely the request won't go through, but maybe some way to verify that the request made it, and if not, a way to resend. I don't think that's needed for now, just thinking out loud.

Comment on lines +95 to +101
type WebhookEevntTutorialComplete = { tutorialId: string; version: string }

export const onTutorialComplete = (event: WebhookEevntTutorialComplete): void => {
if (WEBHOOK_EVENTS.tutorial_complete) {
callWebhookEndpoint<WebhookEevntTutorialComplete>(event)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type WebhookEevntTutorialComplete = { tutorialId: string; version: string }
export const onTutorialComplete = (event: WebhookEevntTutorialComplete): void => {
if (WEBHOOK_EVENTS.tutorial_complete) {
callWebhookEndpoint<WebhookEevntTutorialComplete>(event)
}
}
type WebhookEventTutorialComplete = { tutorialId: string; version?: string }
export const onTutorialComplete = (event: WebhookEventTutorialComplete): void => {
if (WEBHOOK_EVENTS.tutorial_complete) {
callWebhookEndpoint<WebhookEventTutorialComplete>(event)
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, I'll merge for now with the note that it may require more of a guarantee.

Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty sure I was just having issues running the extension without the ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The published version has the version?. I added it in a separate PR.

@moT01 can you post an issue if you have any more details?

}
}

type WebhookEventLevelComplete = { tutorialId: string; version: string; levelId: string }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type WebhookEventLevelComplete = { tutorialId: string; version: string; levelId: string }
type WebhookEventLevelComplete = { tutorialId: string; version?: string; levelId: string }

}
}

type WebhookEventStepComplete = { tutorialId: string; version: string; levelId: string; stepId: string }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type WebhookEventStepComplete = { tutorialId: string; version: string; levelId: string; stepId: string }
type WebhookEventStepComplete = { tutorialId: string; version?: string; levelId: string; stepId: string }

@ShMcK ShMcK marked this pull request as ready for review October 30, 2021 21:54
@ShMcK ShMcK merged commit 268cd26 into master Oct 30, 2021
@ShMcK ShMcK deleted the webhook branch October 30, 2021 21:54
Sign up for free to 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