fix feedback update bug (#779)

* fix feedback update bug * add test for feedback with special character * allow only alphanumeric for field name
line · Oct 10, 2024 · 8878ea7 · 8878ea7
1 parent 4a9a579
commit 8878ea7
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 4 deletions.
diff --git a/apps/api/integration-test/test-specs/channel.integration-spec.ts b/apps/api/integration-test/test-specs/channel.integration-spec.ts
@@ -205,6 +205,22 @@ describe('ChannelController (integration)', () => {
           expect(body.fields[4].name).toBe('TestFieldUpdated');
         });
     });
+
+    it('should return 400 error when update channel fields with special character', async () => {
+      const dto = new UpdateChannelFieldsRequestDto();
+      const fieldDto = new UpdateChannelRequestFieldDto();
+      fieldDto.id = 5;
+      fieldDto.format = FieldFormatEnum.text;
+      fieldDto.key = 'testField';
+      fieldDto.name = '!';
+      dto.fields = [fieldDto];
+
+      await request(app.getHttpServer() as Server)
+        .put(`/admin/projects/${project.id}/channels/1/fields`)
+        .set('Authorization', `Bearer ${accessToken}`)
+        .send(dto)
+        .expect(400);
+    });
   });
 
   describe('/admin/projects/:projectId/channels/:channelId (DELETE)', () => {

diff --git a/apps/api/integration-test/test-specs/feedback.integration-spec.ts b/apps/api/integration-test/test-specs/feedback.integration-spec.ts
@@ -283,6 +283,65 @@ describe('FeedbackController (integration)', () => {
           }
         });
     });
+
+    it('should update a feedback with special character', async () => {
+      const dto: CreateFeedbackDto = {
+        channelId: channel.id,
+        data: {},
+      };
+      let availableFieldKey = '';
+      fields
+        .filter(
+          ({ key }) =>
+            key !== 'id' &&
+            key !== 'issues' &&
+            key !== 'createdAt' &&
+            key !== 'updatedAt',
+        )
+        .forEach(({ key, format, options }) => {
+          dto.data[key] = getRandomValue(format, options);
+          availableFieldKey = key;
+        });
+
+      const feedback = await feedbackService.create(dto);
+
+      return request(app.getHttpServer() as Server)
+        .put(
+          `/admin/projects/${project.id}/channels/${channel.id}/feedbacks/${feedback.id}`,
+        )
+        .set('Authorization', `Bearer ${accessToken}`)
+        .send({
+          [availableFieldKey]: '?',
+        })
+        .expect(200)
+        .then(async () => {
+          if (configService.get('opensearch.use')) {
+            const esResult = await osService.get<OpenSearchResponse>({
+              id: feedback.id.toString(),
+              index: channel.id.toString(),
+            });
+
+            ['id', 'createdAt', 'updatedAt'].forEach(
+              (field) => delete esResult.body._source[field],
+            );
+
+            dto.data[availableFieldKey] = '?';
+            expect(dto.data).toMatchObject(esResult.body._source);
+          } else {
+            const updatedFeedback = await feedbackService.findById({
+              channelId: channel.id,
+              feedbackId: feedback.id,
+            });
+
+            ['id', 'createdAt', 'updatedAt', 'issues'].forEach(
+              (field) => delete updatedFeedback[field],
+            );
+
+            dto.data[availableFieldKey] = '?';
+            expect(dto.data).toMatchObject(updatedFeedback);
+          }
+        });
+    });
   });
 
   afterAll(async () => {

diff --git a/apps/api/src/domains/admin/channel/field/field.mysql.service.ts b/apps/api/src/domains/admin/channel/field/field.mysql.service.ts
@@ -50,6 +50,11 @@ export class FieldMySQLService {
     if (!validateUnique(fields, 'key')) {
       throw new FieldKeyDuplicatedException();
     }
+    fields.forEach(({ name }) => {
+      if (/^[a-z0-9_-]+$/i.test(name) === false) {
+        throw new BadRequestException('field name should be alphanumeric');
+      }
+    });
     fields.forEach(({ format, options }) => {
       if (!this.isValidField(format, options ?? [])) {
         throw new BadRequestException('only select format field has options');

diff --git a/apps/api/src/domains/admin/feedback/feedback.mysql.service.ts b/apps/api/src/domains/admin/feedback/feedback.mysql.service.ts
@@ -292,12 +292,13 @@ export class FeedbackMySQLService {
           }
           let query = `JSON_SET(IFNULL(feedbacks.data,'{}'), `;
           for (const [index, fieldKey] of Object.entries(Object.keys(data))) {
-            query += `'$."${fieldKey}"', ${
+            query += `'$.${fieldKey}',
+            ${
               Array.isArray(data[fieldKey]) ?
                 data[fieldKey].length === 0 ?
                   'JSON_ARRAY()'
                 : 'JSON_ARRAY("' + data[fieldKey].join('","') + '")'
-              : '"' + data[fieldKey] + '"'
+              : `:${fieldKey}`
             }`;
 
             if (parseInt(index) + 1 !== Object.entries(data).length) {
@@ -311,6 +312,7 @@ export class FeedbackMySQLService {
         updatedAt: () => `'${DateTime.utc().toFormat('yyyy-MM-dd HH:mm:ss')}'`,
       })
       .where('id = :feedbackId', { feedbackId })
+      .setParameters(data)
       .execute();
   }
 

diff --git a/apps/api/src/test-utils/fixtures.ts b/apps/api/src/test-utils/fixtures.ts
@@ -78,8 +78,8 @@ export const createFieldDto = (input: Partial<CreateFieldDto> = {}) => {
   const property = input.property ?? getRandomEnumValue(FieldPropertyEnum);
   const status = input.status ?? getRandomEnumValue(FieldStatusEnum);
   return {
-    name: faker.string.alphanumeric(20),
-    key: faker.string.alphanumeric(20),
+    name: `_${faker.string.alphanumeric(20)}`,
+    key: `_${faker.string.alphanumeric(20)}`,
     description: faker.lorem.lines(2),
     format,
     property,