do not show "Notification not found" error messages (#10931)

* start fixing #10391 * do not show snackbars for notifications not found * revert message changes * update tests * add new test * fix property
HabitRPG · Jan 13, 2019 · f63d2e4 · f63d2e4
1 parent a08cca8
commit f63d2e4
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 22 deletions.
diff --git a/test/api/unit/libs/errors.test.js b/test/api/unit/libs/errors.test.js
@@ -5,7 +5,9 @@ import {
   BadRequest,
   InternalServerError,
   NotFound,
+  NotificationNotFound,
 } from '../../../../website/server/libs/errors';
+import i18n from '../../../../website/common/script/i18n';
 
 describe('Custom Errors', () => {
   describe('CustomError', () => {
@@ -66,6 +68,23 @@ describe('Custom Errors', () => {
 
       expect(notAuthorizedError.message).to.eql('Custom Error Message');
     });
+
+    describe('NotificationNotFound', () => {
+      it('is an instance of NotFound', () => {
+        const notificationNotFoundErr = new NotificationNotFound();
+        expect(notificationNotFoundErr).to.be.an.instanceOf(NotFound);
+      });
+
+      it('it returns an http code of 404', () => {
+        const notificationNotFoundErr = new NotificationNotFound();
+        expect(notificationNotFoundErr.httpCode).to.eql(404);
+      });
+
+      it('returns a standard message', () => {
+        const notificationNotFoundErr = new NotificationNotFound();
+        expect(notificationNotFoundErr.message).to.eql(i18n.t('messageNotificationNotFound'));
+      });
+    });
   });
 
   describe('BadRequest', () => {

diff --git a/test/api/v3/integration/notifications/POST-notifications_notificationId_read.test.js b/test/api/v3/integration/notifications/POST-notifications_notificationId_read.test.js
@@ -16,7 +16,7 @@ describe('POST /notifications/:notificationId/read', () => {
 
     await expect(user.post(`/notifications/${dummyId}/read`)).to.eventually.be.rejected.and.eql({
       code: 404,
-      error: 'NotFound',
+      error: 'NotificationNotFound',
       message: t('messageNotificationNotFound'),
     });
   });

diff --git a/test/api/v3/integration/notifications/POST-notifications_notificationId_see.test.js b/test/api/v3/integration/notifications/POST-notifications_notificationId_see.test.js
@@ -16,7 +16,7 @@ describe('POST /notifications/:notificationId/see', () => {
 
     await expect(user.post(`/notifications/${dummyId}/see`)).to.eventually.be.rejected.and.eql({
       code: 404,
-      error: 'NotFound',
+      error: 'NotificationNotFound',
       message: t('messageNotificationNotFound'),
     });
   });

diff --git a/test/api/v3/integration/notifications/POST-notifications_read.test.js b/test/api/v3/integration/notifications/POST-notifications_read.test.js
@@ -18,7 +18,7 @@ describe('POST /notifications/read', () => {
       notificationIds: [dummyId],
     })).to.eventually.be.rejected.and.eql({
       code: 404,
-      error: 'NotFound',
+      error: 'NotificationNotFound',
       message: t('messageNotificationNotFound'),
     });
   });

diff --git a/test/api/v3/integration/notifications/POST_notifications_see.test.js b/test/api/v3/integration/notifications/POST_notifications_see.test.js
@@ -18,7 +18,7 @@ describe('POST /notifications/see', () => {
       notificationIds: [dummyId],
     })).to.eventually.be.rejected.and.eql({
       code: 404,
-      error: 'NotFound',
+      error: 'NotificationNotFound',
       message: t('messageNotificationNotFound'),
     });
   });

diff --git a/website/client/app.vue b/website/client/app.vue
@@ -362,6 +362,7 @@ export default {
         const errorMessage = errorData.message || errorData;
 
         // Check for conditions to reset the user auth
+        // TODO use a specific error like NotificationNotFound instead of checking for the string
         const invalidUserMessage = [this.$t('invalidCredentials'), 'Missing authentication headers.'];
         if (invalidUserMessage.indexOf(errorMessage) !== -1) {
           this.$store.dispatch('auth:logout');
@@ -371,12 +372,6 @@ export default {
         let snackbarTimeout = false;
         if (error.response.status === 502) snackbarTimeout = true;
 
-        const notificationNotFoundMessage = [
-          this.$t('messageNotificationNotFound'),
-          this.$t('messageNotificationNotFound', 'en'),
-        ];
-        if (notificationNotFoundMessage.indexOf(errorMessage) !== -1) snackbarTimeout = true;
-
         let errorsToShow = [];
         // show only the first error for each param
         let paramErrorsFound = {};
@@ -390,13 +385,17 @@ export default {
         } else {
           errorsToShow.push(errorMessage);
         }
-        // dispatch as one snackbar notification
-        this.$store.dispatch('snackbars:add', {
-          title: 'Habitica',
-          text: errorsToShow.join(' '),
-          type: 'error',
-          timeout: snackbarTimeout,
-        });
+
+        // Ignore NotificationNotFound errors, see https://github.com/HabitRPG/habitica/issues/10391
+        if (errorData.error !== 'NotificationNotFound') {
+          // dispatch as one snackbar notification
+          this.$store.dispatch('snackbars:add', {
+            title: 'Habitica',
+            text: errorsToShow.join(' '),
+            type: 'error',
+            timeout: snackbarTimeout,
+          });
+        }
       }
 
       return Promise.reject(error);

diff --git a/website/server/controllers/api-v3/notifications.js b/website/server/controllers/api-v3/notifications.js
@@ -1,6 +1,6 @@
 import { authWithHeaders } from '../../middlewares/auth';
 import {
-  NotFound,
+  NotificationNotFound,
 } from '../../libs/errors';
 import {
   model as User,
@@ -37,7 +37,7 @@ api.readNotification = {
     });
 
     if (index === -1) {
-      throw new NotFound(res.t('messageNotificationNotFound'));
+      throw new NotificationNotFound(req.language);
     }
 
     user.notifications.splice(index, 1);
@@ -81,7 +81,7 @@ api.readNotifications = {
       });
 
       if (index === -1) {
-        throw new NotFound(res.t('messageNotificationNotFound'));
+        throw new NotificationNotFound(req.language);
       }
 
       user.notifications.splice(index, 1);
@@ -129,7 +129,7 @@ api.seeNotification = {
     });
 
     if (!notification) {
-      throw new NotFound(res.t('messageNotificationNotFound'));
+      throw new NotificationNotFound(req.language);
     }
 
     notification.seen = true;
@@ -179,7 +179,7 @@ api.seeNotifications = {
       });
 
       if (!notification) {
-        throw new NotFound(res.t('messageNotificationNotFound'));
+        throw new NotificationNotFound(req.language);
       }
 
       notification.seen = true;

diff --git a/website/server/libs/errors.js b/website/server/libs/errors.js
@@ -41,6 +41,25 @@ export const BadRequest = common.errors.BadRequest;
  */
 export const NotFound = common.errors.NotFound;
 
+
+/**
+ * @apiDefine NotificationNotFound
+ * @apiError NotificationNotFound The notification was not found.
+ *
+ * @apiErrorExample Error-Response:
+ *     HTTP/1.1 404 Not Found
+ *     {
+ *       "error": "NotificationNotFound",
+ *       "message": "Notification not found."
+ *     }
+ */
+export class NotificationNotFound extends NotFound {
+  constructor (language) {
+    super(common.i18n.t('messageNotificationNotFound', language));
+    this.name = this.constructor.name;
+  }
+}
+
 /**
  * @apiDefine InternalServerError
  * @apiError InternalServerError An unexpected error occurred.