Skip to content

Commit

Permalink
feat(webhooks): streamline webhook response data (#1696)
Browse files Browse the repository at this point in the history
* feat: remove useless fields and make others required

* feat: remove logic to store useless fields

* feat: allow data and headers to be empty strings

* test: update tests
  • Loading branch information
mantariksh authored Apr 22, 2021
1 parent 336fd8d commit ad597a7
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 30 deletions.
12 changes: 9 additions & 3 deletions src/app/models/__tests__/submission.server.model.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { times } from 'lodash'
import mongoose from 'mongoose'

import getSubmissionModel, {
getEmailSubmissionModel,
getEncryptSubmissionModel,
} from 'src/app/models/submission.server.model'

Expand All @@ -17,6 +18,7 @@ import {

const Submission = getSubmissionModel(mongoose)
const EncryptedSubmission = getEncryptSubmissionModel(mongoose)
const EmailSubmission = getEmailSubmissionModel(mongoose)

// TODO: Add more tests for the rest of the submission schema.
describe('Submission Model', () => {
Expand Down Expand Up @@ -107,7 +109,7 @@ describe('Submission Model', () => {
// Arrange
const formId = new ObjectID()

const submission = await Submission.create({
const submission = await EncryptedSubmission.create({
submissionType: SubmissionType.Encrypt,
form: formId,
encryptedContent: MOCK_ENCRYPTED_CONTENT,
Expand Down Expand Up @@ -136,7 +138,7 @@ describe('Submission Model', () => {
it('should return null view with non-encryptSubmission type', async () => {
// Arrange
const formId = new ObjectID()
const submission = await Submission.create({
const submission = await EmailSubmission.create({
submissionType: SubmissionType.Email,
form: formId,
encryptedContent: MOCK_ENCRYPTED_CONTENT,
Expand Down Expand Up @@ -182,7 +184,6 @@ describe('Submission Model', () => {
response: {
data: '{"result":"test-result"}',
status: 200,
statusText: 'success',
headers: '{}',
},
}) as IWebhookResponse
Expand Down Expand Up @@ -223,6 +224,11 @@ describe('Submission Model', () => {
created: submission.created,
signature: 'some signature',
webhookUrl: 'https://form.gov.sg/endpoint',
response: {
status: 200,
headers: '',
data: '',
},
} as IWebhookResponse

const invalidSubmissionId = new ObjectID().toHexString()
Expand Down
17 changes: 12 additions & 5 deletions src/app/models/submission.server.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,19 @@ EmailSubmissionSchema.methods.getWebhookView = function (): null {

const webhookResponseSchema = new Schema<IWebhookResponseSchema>(
{
webhookUrl: String,
signature: String,
errorMessage: String,
webhookUrl: {
type: String,
required: true,
},
signature: {
type: String,
required: true,
},
response: {
status: Number,
statusText: String,
status: {
type: Number,
required: true,
},
headers: String,
data: String,
},
Expand Down
11 changes: 0 additions & 11 deletions src/app/modules/webhook/__tests__/webhook.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { getEncryptSubmissionModel } from 'src/app/models/submission.server.mode
import { WebhookValidationError } from 'src/app/modules/webhook/webhook.errors'
import * as WebhookValidationModule from 'src/app/modules/webhook/webhook.validation'
import { transformMongoError } from 'src/app/utils/handle-mongo-error'
import * as HasPropModule from 'src/app/utils/has-prop'
import {
IEncryptedSubmissionSchema,
IWebhookResponse,
Expand Down Expand Up @@ -59,15 +58,13 @@ const MOCK_WEBHOOK_SUCCESS_RESPONSE: Pick<IWebhookResponse, 'response'> = {
response: {
data: '{"result":"test-result"}',
status: 200,
statusText: 'success',
headers: '{}',
},
}
const MOCK_WEBHOOK_FAILURE_RESPONSE: Pick<IWebhookResponse, 'response'> = {
response: {
data: '{"result":"test-result"}',
status: 400,
statusText: 'failed',
headers: '{}',
},
}
Expand All @@ -78,7 +75,6 @@ const MOCK_WEBHOOK_DEFAULT_FORMAT_RESPONSE: Pick<
response: {
data: '',
status: 0,
statusText: '',
headers: '',
},
}
Expand Down Expand Up @@ -305,7 +301,6 @@ describe('webhook.service', () => {

// Assert
const expectedResult = {
errorMessage: AXIOS_ERROR_MSG,
...MOCK_WEBHOOK_FAILURE_RESPONSE,
signature: testSignature,
webhookUrl: MOCK_WEBHOOK_URL,
Expand Down Expand Up @@ -334,7 +329,6 @@ describe('webhook.service', () => {

// Assert
const expectedResult = {
errorMessage: DEFAULT_ERROR_MSG,
...MOCK_WEBHOOK_DEFAULT_FORMAT_RESPONSE,
signature: testSignature,
webhookUrl: MOCK_WEBHOOK_URL,
Expand All @@ -359,9 +353,6 @@ describe('webhook.service', () => {

MockAxios.post.mockRejectedValue(mockOriginalError)
MockAxios.isAxiosError.mockReturnValue(false)
const hasPropSpy = jest
.spyOn(HasPropModule, 'hasProp')
.mockReturnValueOnce(false)

// Act
const actual = await sendWebhook(
Expand All @@ -371,7 +362,6 @@ describe('webhook.service', () => {

// Assert
const expectedResult = {
errorMessage: '',
...MOCK_WEBHOOK_DEFAULT_FORMAT_RESPONSE,
signature: testSignature,
webhookUrl: MOCK_WEBHOOK_URL,
Expand All @@ -380,7 +370,6 @@ describe('webhook.service', () => {
expect(
MockWebhookValidationModule.validateWebhookUrl,
).toHaveBeenCalledWith(MOCK_WEBHOOK_URL)
expect(hasPropSpy).toHaveBeenCalledWith(mockOriginalError, 'message')
expect(MockAxios.post).toHaveBeenCalledWith(
MOCK_WEBHOOK_URL,
testSubmissionWebhookView,
Expand Down
7 changes: 0 additions & 7 deletions src/app/modules/webhook/webhook.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import formsgSdk from '../../config/formsg-sdk'
import { createLoggerWithLabel } from '../../config/logger'
import { getEncryptSubmissionModel } from '../../models/submission.server.model'
import { transformMongoError } from '../../utils/handle-mongo-error'
import { hasProp } from '../../utils/has-prop'
import { PossibleDatabaseError } from '../core/core.errors'
import { SubmissionNotFoundError } from '../submission/submission.errors'

Expand Down Expand Up @@ -159,14 +158,9 @@ export const sendWebhook = (

// Webhook was posted but failed
if (error instanceof WebhookFailedWithUnknownError) {
const originalError = error.meta.originalError
const errorMessage = hasProp(originalError, 'message')
? originalError.message
: ''
return okAsync({
signature,
webhookUrl,
errorMessage,
// Not Axios error so no guarantee of having response.
// Hence allow formatting function to return default shape.
response: formatWebhookResponse(),
Expand All @@ -177,7 +171,6 @@ export const sendWebhook = (
return okAsync({
signature,
webhookUrl,
errorMessage: axiosError.message,
response: formatWebhookResponse(axiosError.response),
})
})
Expand Down
1 change: 0 additions & 1 deletion src/app/modules/webhook/webhook.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ export const formatWebhookResponse = (
response?: AxiosResponse<unknown>,
): IWebhookResponse['response'] => ({
status: response?.status ?? 0,
statusText: response?.statusText ?? '',
headers: stringifySafe(response?.headers) ?? '',
data: stringifySafe(response?.data) ?? '',
})
6 changes: 3 additions & 3 deletions src/types/submission.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { AxiosResponse } from 'axios'
import { Document, Model, QueryCursor } from 'mongoose'

import { MyInfoAttribute } from './field'
Expand Down Expand Up @@ -105,9 +104,10 @@ export type IEncryptedSubmissionSchema = IEncryptedSubmission &
export interface IWebhookResponse {
webhookUrl: string
signature: string
errorMessage?: string
response?: Omit<AxiosResponse<string>, 'config' | 'request' | 'headers'> & {
response: {
status: number
headers: string
data: string
}
}

Expand Down

0 comments on commit ad597a7

Please sign in to comment.