chore: cleanup deprecated feedback API

shared/types/form/form_feedback.ts

@@ -18,8 +18,7 @@ export type FormFeedbackBase = {
   formId: FormDto['_id']
   created?: Date
   lastModified?: Date
-  // TODO #3964: Update to not optional once we fully migrate to /submissions/{submissionId}/feedback endpoint
-  submissionId?: SubmissionResponseDto['submissionId']
+  submissionId: SubmissionResponseDto['submissionId']
 }
 
 // Convert to serialized version.

src/app/models/__tests__/form_feedback.server.model.spec.ts

@@ -17,6 +17,7 @@ describe('form_feedback.server.model', () => {
   describe('Schema', () => {
     const DEFAULT_PARAMS: IFormFeedback = {
       formId: new ObjectId(),
+      submissionId: new ObjectId(),
       rating: 5,
       comment: 'feedback comment',
     }
@@ -74,6 +75,18 @@ describe('form_feedback.server.model', () => {
         mongoose.Error.ValidationError,
       )
     })
+
+    it('should throw validation error when submissionId param is missing', async () => {
+      // Act
+      const actualPromise = new FeedbackModel(
+        omit(DEFAULT_PARAMS, 'submissionId'),
+      ).save()
+
+      // Assert
+      await expect(actualPromise).rejects.toThrowError(
+        mongoose.Error.ValidationError,
+      )
+    })
   })
 
   describe('Statics', () => {
@@ -82,8 +95,10 @@ describe('form_feedback.server.model', () => {
         // Arrange
         // Create document
         const mockFormId = new ObjectId()
+        const mockSubmissionId = new ObjectId()
         const mockFeedbackDoc = await FeedbackModel.create({
           formId: mockFormId,
+          submissionId: mockSubmissionId,
           rating: 5,
           comment: 'feedback comment',
         })

src/app/models/form_feedback.server.model.ts

@@ -18,8 +18,7 @@ const FormFeedbackSchema = new Schema<IFormFeedbackSchema, IFormFeedbackModel>(
     submissionId: {
       type: Schema.Types.ObjectId,
       ref: SUBMISSION_SCHEMA_ID,
-      // TODO #3964: Update to true once we fully migrate to /submissions/{submissionId}/feedback endpoint
-      required: false,
+      required: true,
     },
     rating: {
       type: Number,

src/app/modules/examples/__tests__/helpers/prepareTestData.ts

@@ -9,6 +9,7 @@ import {
   IAgencySchema,
   IFormDocument,
   IFormFeedbackSchema,
+  ISubmissionSchema,
   IUserSchema,
 } from 'src/types'
 
@@ -43,6 +44,8 @@ export type TestData = {
     expectedFormInfo: FormInfo[]
     // Feedbacks for each of the forms.
     feedbacks: IFormFeedbackSchema[]
+    // Submissions for each of the forms.
+    submissions: ISubmissionSchema[]
   }
 }
 
@@ -64,6 +67,7 @@ const prepareTestData = async (
       forms: [],
       expectedFormInfo: [],
       feedbacks: [],
+      submissions: [],
     },
     second: {
       searchTerm: SearchTerm.Second,
@@ -72,6 +76,7 @@ const prepareTestData = async (
       forms: [],
       expectedFormInfo: [],
       feedbacks: [],
+      submissions: [],
     },
     total: {
       searchTerm: '',
@@ -80,6 +85,7 @@ const prepareTestData = async (
       forms: [],
       expectedFormInfo: [],
       feedbacks: [],
+      submissions: [],
     },
   }
 
@@ -126,6 +132,8 @@ const prepareTestData = async (
       ),
     ),
   )
+  testData.first.submissions = await Promise.all(firstSubmissionPromises)
+
   const secondSubmissionPromises = flatten(
     testData.second.forms.map((form) =>
       times(testData.second.submissionCount, () =>
@@ -138,10 +146,15 @@ const prepareTestData = async (
       ),
     ),
   )
+  testData.second.submissions = await Promise.all(secondSubmissionPromises)
 
   // Assign all forms in test data.
   testData.total.forms = testData.first.forms.concat(testData.second.forms)
 
+  testData.total.submissions = testData.first.submissions.concat(
+    testData.second.submissions,
+  )
+
   // Add form statistics for "submissions" for both form prefixes.
   const formStatsPromises = testData.first.forms
     .map((form) =>
@@ -176,6 +189,7 @@ const prepareTestData = async (
     return FeedbackModel.create({
       rating: score,
       formId: testData.total.forms[i]._id,
+      submissionId: testData.total.submissions[i]._id,
       comment: 'very good test',
     })
   })

src/app/modules/feedback/__tests__/feedback.service.spec.ts

@@ -30,11 +30,13 @@ describe('feedback.service', () => {
       // Arrange
       // Insert 3 form feedbacks.
       const validFormId = new ObjectId()
+      const validSubmissionId = new ObjectId().toHexString()
       const expectedFeedbackCount = 3
       const feedbackPromises = times(expectedFeedbackCount, (count) =>
         FormFeedback.create({
           comment: `test feedback ${count}`,
           formId: validFormId,
+          submissionId: validSubmissionId,
           rating: 5,
         }),
       )
@@ -109,20 +111,24 @@ describe('feedback.service', () => {
   })
 
   describe('getFormFeedbacks', () => {
+    const MOCK_SUBMISSION_ID = new ObjectId().toHexString()
+
     it('should return correct feedback responses', async () => {
       // Arrange
       const expectedCount = 3
       const mockFormId = new ObjectId().toHexString()
       const expectedFbPromises = times(expectedCount, (count) =>
         FormFeedback.create({
           formId: mockFormId,
+          submissionId: MOCK_SUBMISSION_ID,
           comment: `cool form ${count}`,
           rating: 5 - count,
         }),
       )
       // Add another feedback with a different form id.
       await FormFeedback.create({
         formId: new ObjectId(),
+        submissionId: MOCK_SUBMISSION_ID,
         comment: 'boo this form sux',
         rating: 1,
       })
@@ -194,6 +200,7 @@ describe('feedback.service', () => {
       const mockFormId = new ObjectId().toHexString()
       const createdFb = await FormFeedback.create({
         formId: mockFormId,
+        submissionId: MOCK_SUBMISSION_ID,
         // Missing comment key value.
         rating: 3,
       })
@@ -281,6 +288,7 @@ describe('feedback.service', () => {
       await FormFeedback.create({
         comment: `test feedback`,
         formId: MOCK_FORM_ID,
+        submissionId: MOCK_SUBMISSION_ID,
         rating: 5,
       })
 

src/app/modules/feedback/feedback.controller.ts

@@ -35,18 +35,16 @@ const validateSubmitFormFeedbackParams = celebrate({
  * @returns 422 if duplicate feedback with the same submissionId and formId exists
  * @returns 410 if form has been archived
  * @returns 500 if database error occurs
- *
- * TODO #3964: Rename to `submitFormFeedback` once we fully migrate feedback endpoint to /submissions/{submissionId}/feedback
  */
-const submitFormFeedbackV2: ControllerHandler<
+const submitFormFeedback: ControllerHandler<
   { formId: string; submissionId: string },
   { message: string } | ErrorDto | PrivateFormErrorDto,
   { rating: number; comment: string }
 > = async (req, res) => {
   const { formId, submissionId } = req.params
   const { rating, comment } = req.body
   const logMeta = {
-    action: 'submitFormFeedbackV2',
+    action: 'submitFormFeedback',
     ...createReqMeta(req),
     formId,
     submissionId,
@@ -91,5 +89,5 @@ const submitFormFeedbackV2: ControllerHandler<
 
 export const handleSubmitFormFeedback = [
   validateSubmitFormFeedbackParams,
-  submitFormFeedbackV2,
+  submitFormFeedback,
 ] as ControllerHandler[]

src/app/modules/form/admin-form/__tests__/admin-form.routes.spec.ts

@@ -2,6 +2,7 @@
 import { ObjectId } from 'bson-ext'
 import { format, subDays } from 'date-fns'
 import { cloneDeep, omit, times } from 'lodash'
+import { ObjectID } from 'mongodb'
 import mongoose from 'mongoose'
 import { errAsync, okAsync } from 'neverthrow'
 import SparkMD5 from 'spark-md5'
@@ -3128,8 +3129,18 @@ describe('admin-form.routes', () => {
     it('should return 200 with form feedback meta when feedback exists', async () => {
       // Arrange
       const formFeedbacks = [
-        { formId: formForFeedback._id, rating: 5, comment: 'nice' },
-        { formId: formForFeedback._id, rating: 2, comment: 'not nice' },
+        {
+          formId: formForFeedback._id,
+          rating: 5,
+          comment: 'nice',
+          submissionId: new ObjectID().toHexString(),
+        },
+        {
+          formId: formForFeedback._id,
+          rating: 2,
+          comment: 'not nice',
+          submissionId: new ObjectID().toHexString(),
+        },
       ]
       await insertFormFeedback(formFeedbacks[0])
       await insertFormFeedback(formFeedbacks[1])
@@ -3308,8 +3319,18 @@ describe('admin-form.routes', () => {
     it('should return 200 with feedback count when feedback exists', async () => {
       // Arrange
       const formFeedbacks = [
-        { formId: formForFeedback._id, rating: 5, comment: 'nice' },
-        { formId: formForFeedback._id, rating: 2, comment: 'not nice' },
+        {
+          formId: formForFeedback._id,
+          rating: 5,
+          comment: 'nice',
+          submissionId: new ObjectID().toHexString(),
+        },
+        {
+          formId: formForFeedback._id,
+          rating: 2,
+          comment: 'not nice',
+          submissionId: new ObjectID().toHexString(),
+        },
       ]
       await insertFormFeedback(formFeedbacks[0])
       await insertFormFeedback(formFeedbacks[1])
@@ -3452,8 +3473,18 @@ describe('admin-form.routes', () => {
     it('should return 200 with feedback stream when feedbacks exist', async () => {
       // Arrange
       const formFeedbacks = [
-        { formId: formForFeedback._id, rating: 5, comment: 'nice' },
-        { formId: formForFeedback._id, rating: 2, comment: 'not nice' },
+        {
+          formId: formForFeedback._id,
+          rating: 5,
+          comment: 'nice',
+          submissionId: new ObjectID().toHexString(),
+        },
+        {
+          formId: formForFeedback._id,
+          rating: 2,
+          comment: 'not nice',
+          submissionId: new ObjectID().toHexString(),
+        },
       ]
       await insertFormFeedback(formFeedbacks[0])
       await insertFormFeedback(formFeedbacks[1])