Fixes #39 (#40)

* Fixes #39, enables the validation on the feedback submission route --------- Signed-off-by: Peter Macdonald <[email protected]>
Parsifal-M · Jul 26, 2024 · 11be08e · 11be08e
1 parent fc4b198
commit 11be08e
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 41 deletions.
diff --git a/plugins/open-feedback-backend/src/service/router.test.ts b/plugins/open-feedback-backend/src/service/router.test.ts
@@ -3,10 +3,11 @@ import express from 'express';
 import request from 'supertest';
 import { createRouter } from './router';
 import { OpenFeedbackDatabaseHandler } from '../database/DatabaseHandler';
+import { AppFeedback } from '@parsifal-m/backstage-plugin-open-feedback-common';
 
 const mockDatabaseHandler = {
   addFeedback: jest.fn(),
-  getFeedback: jest.fn(),
+  getFeedback: jest.fn<Promise<AppFeedback[]>, []>(),
   removeFeedback: jest.fn(),
 } as unknown as OpenFeedbackDatabaseHandler;
 
@@ -50,6 +51,10 @@ describe('createRouter', () => {
         userRef: 'user1',
       };
 
+      (mockDatabaseHandler.addFeedback as jest.Mock).mockResolvedValueOnce(
+        undefined,
+      );
+
       const response = await request(app)
         .post('/feedback/submit')
         .send(feedback);
@@ -62,14 +67,15 @@ describe('createRouter', () => {
       expect(mockDatabaseHandler.addFeedback).toHaveBeenCalledWith(feedback);
     });
   });
+
   describe('GET /feedback', () => {
     it('returns feedback list', async () => {
       const feedbackList = [
         { rating: 5, url: 'test-url', comment: 'Great!', userRef: 'user1' },
       ];
-      mockDatabaseHandler.getFeedback = jest
-        .fn()
-        .mockResolvedValue(feedbackList);
+      (mockDatabaseHandler.getFeedback as jest.Mock).mockResolvedValueOnce(
+        feedbackList,
+      );
 
       const response = await request(app).get('/feedback');
 
@@ -82,9 +88,9 @@ describe('createRouter', () => {
   describe('DELETE /feedback/:id', () => {
     it('returns ok when feedback is removed', async () => {
       const feedbackId = 1;
-      mockDatabaseHandler.removeFeedback = jest
-        .fn()
-        .mockResolvedValue(feedbackId);
+      (mockDatabaseHandler.removeFeedback as jest.Mock).mockResolvedValueOnce(
+        undefined,
+      );
 
       const response = await request(app).delete(`/feedback/${feedbackId}`);
 

diff --git a/plugins/open-feedback-backend/src/service/router.ts b/plugins/open-feedback-backend/src/service/router.ts
@@ -9,7 +9,7 @@ import {
 } from '@backstage/backend-plugin-api';
 import express, { Request, Response } from 'express';
 import Router from 'express-promise-router';
-import { body } from 'express-validator';
+import { body, validationResult } from 'express-validator';
 import { OpenFeedbackDatabaseHandler } from '../database/DatabaseHandler';
 import { SubmitFeedback } from '@parsifal-m/backstage-plugin-open-feedback-common';
 
@@ -23,9 +23,9 @@ export interface RouterOptions {
 
 const feedbackValidator = [
   body('rating').isNumeric().notEmpty(),
-  body('comment').isString().notEmpty().withMessage('Comment must be a string'),
+  body('comment').isString().withMessage('Comment must be a string'),
   body('userRef').isString(),
-  body('url').isString().notEmpty(),
+  body('url').isString(),
 ];
 
 export async function createRouter(
@@ -44,48 +44,61 @@ export async function createRouter(
   router.post(
     '/feedback/submit',
     feedbackValidator,
-    async (req: Request, res: Response) => {
+    (req: Request, res: Response) => {
+      const errors = validationResult(req);
+      if (!errors.isEmpty()) {
+        return res.status(400).json({ errors: errors.array() });
+      }
+
       const { rating, url, comment, userRef } = req.body;
 
       const feedback: SubmitFeedback = { userRef, url, rating, comment };
 
-      try {
-        await databaseHandler.addFeedback(feedback);
-        logger.info(`Received feedback from ${userRef} with rating ${rating}`);
-        res.status(201).json({ status: 'ok', message: 'Feedback added' });
-      } catch (error) {
-        logger.error(`Failed to add feedback: ${error}`);
-        res
-          .status(500)
-          .json({ status: 'error', message: 'Failed to add feedback' });
-      }
+      return databaseHandler
+        .addFeedback(feedback)
+        .then(() => {
+          logger.info(
+            `Received feedback from ${userRef} with rating ${rating}`,
+          );
+          res.status(201).json({ status: 'ok', message: 'Feedback added' });
+        })
+        .catch(error => {
+          logger.error(`Failed to add feedback: ${error}`);
+          res
+            .status(500)
+            .json({ status: 'error', message: 'Failed to add feedback' });
+        });
     },
   );
 
-  router.get('/feedback', async (_, res: Response) => {
-    try {
-      const feedback = await databaseHandler.getFeedback();
-      res.status(200).json(feedback);
-    } catch (error) {
-      logger.error(`Failed to get feedback: ${error}`);
-      res
-        .status(500)
-        .json({ status: 'error', message: 'Failed to get feedback' });
-    }
+  router.get('/feedback', (_, res: Response) => {
+    databaseHandler
+      .getFeedback()
+      .then(feedback => {
+        res.status(200).json(feedback);
+      })
+      .catch(error => {
+        logger.error(`Failed to get feedback: ${error}`);
+        res
+          .status(500)
+          .json({ status: 'error', message: 'Failed to get feedback' });
+      });
   });
 
-  router.delete('/feedback/:id', async (req: Request, res: Response) => {
+  router.delete('/feedback/:id', (req: Request, res: Response) => {
     const { id } = req.params;
 
-    try {
-      await databaseHandler.removeFeedback(Number(id));
-      res.status(200).json({ status: 'ok', message: 'Feedback removed' });
-    } catch (error) {
-      logger.error(`Failed to remove feedback: ${error}`);
-      res
-        .status(500)
-        .json({ status: 'error', message: 'Failed to remove feedback' });
-    }
+    databaseHandler
+      .removeFeedback(Number(id))
+      .then(() => {
+        res.status(200).json({ status: 'ok', message: 'Feedback removed' });
+      })
+      .catch(error => {
+        logger.error(`Failed to remove feedback: ${error}`);
+        res
+          .status(500)
+          .json({ status: 'error', message: 'Failed to remove feedback' });
+      });
   });
 
   router.use(errorHandler());