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

MDTZ-1089: Refactor Figma webhook error handling (PR 2/3) #126

Merged
merged 6 commits into from
Oct 27, 2023

Conversation

akostevich-atlassian
Copy link
Collaborator

@akostevich-atlassian akostevich-atlassian commented Oct 26, 2023

Changes

  • refactor: Replaces FigmaWebhookService with figmaWebhookAuthMiddleware for authenticating Figma webhook events.
  • refactor: Deletes ValidationError and related error handling.
  • refactor: Simplifies and improves schema definition for Figma events by defining separate schemas for each event type using JSON Schema Discriminator.
  • test: Resolves test flakiness for the checkAuth endpoints (caused by the usage of time-bound JWT tokens).

@@ -75,44 +75,6 @@ describe('/admin/auth', () => {
.expect({ authorized: true });
});

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: Add new integration tests, reorder existing ones and resolve test flakiness (caused by a time-bound JWT token).

@@ -70,41 +70,6 @@ describe('/auth', () => {
.expect({ type: '3LO', authorized: true });
});

it('should return a response indicating that user is not authorized if no credentials stored', async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: Add new integration tests, reorder existing ones and resolve test flakiness (caused by a time-bound JWT token).

export const FIGMA_WEBHOOK_EVENT_REQUEST_SCHEMA: JSONSchemaTypeWithId<{
body: FigmaWebhookEventPayload;
}> = {
export const FIGMA_WEBHOOK_EVENT_REQUEST_SCHEMA = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: Improve schema definition by using JSON Schema Discriminator (e.g., file_key gets required for the FILE_UPDATE event but is still optional for PING).


export type FigmaWebhookEventPayload = {
readonly event_type: FigmaWebhookEventType;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: Remove fields the app is not interested in.

@akostevich-atlassian akostevich-atlassian changed the title MDTZ-1089: Refactor Figma webhook handling (PR 1/2) MDTZ-1089: Refactor Figma webhook handling (PR 2/3) Oct 26, 2023
@akostevich-atlassian akostevich-atlassian changed the title MDTZ-1089: Refactor Figma webhook handling (PR 2/3) MDTZ-1089: Refactor Figma webhook error handling (PR 2/3) Oct 26, 2023
@akostevich-atlassian akostevich-atlassian merged commit 7bd5163 into main Oct 27, 2023
1 check passed
@akostevich-atlassian akostevich-atlassian deleted the MDTZ-1089-error-handling-improvements-2 branch October 27, 2023 04:58
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