From 8e129c6001e95005ff68f52ffcc2ed041eac04ec Mon Sep 17 00:00:00 2001
From: roschaefer <git@roschaefer.de>
Date: Thu, 31 Oct 2019 23:04:04 +0100
Subject: [PATCH] fix: update counter when notification is read

@mattwr18 vue-apollo rocks! Taking the time to study the docs is a
rewarding investment.

My first idea was to cache the `unreadNotificationsCount` with Vuex.
But the docs of apollo even suggest to use apollo's local state as a
complete replacement of Vuex:
https://vue-apollo.netlify.com/guide/local-state.html

Then I further investigated why the updated `NOTIFIED` objects won't
update the notification counter. Turns out: They don't have an ID and
the computed property didn't fire when the notifications array would
change. I fixed both in this commit and yes, it works as expected.

No additional code required :muscle:
---
 backend/src/schema/resolvers/notifications.js |  6 +++
 backend/src/schema/types/type/NOTIFIED.gql    |  1 +
 .../NotificationMenu/NotificationMenu.spec.js |  4 +-
 .../NotificationMenu/NotificationMenu.vue     | 46 +++++--------------
 webapp/graphql/User.js                        |  2 +
 5 files changed, 22 insertions(+), 37 deletions(-)

diff --git a/backend/src/schema/resolvers/notifications.js b/backend/src/schema/resolvers/notifications.js
index 93feb3781..8d0683258 100644
--- a/backend/src/schema/resolvers/notifications.js
+++ b/backend/src/schema/resolvers/notifications.js
@@ -78,4 +78,10 @@ export default {
       return notification
     },
   },
+  NOTIFIED: {
+    id: async (parent) => {
+      // serialize an ID to help the client update the cache
+      return `${parent.reason}/${parent.from.id}/${parent.to.id}`
+    }
+  }
 }
diff --git a/backend/src/schema/types/type/NOTIFIED.gql b/backend/src/schema/types/type/NOTIFIED.gql
index 42da6a39b..1182f2743 100644
--- a/backend/src/schema/types/type/NOTIFIED.gql
+++ b/backend/src/schema/types/type/NOTIFIED.gql
@@ -1,4 +1,5 @@
 type NOTIFIED {
+  id: ID!
   from: NotificationSource
   to: User
   createdAt: String
diff --git a/webapp/components/NotificationMenu/NotificationMenu.spec.js b/webapp/components/NotificationMenu/NotificationMenu.spec.js
index 2145b7d6e..87576a5f3 100644
--- a/webapp/components/NotificationMenu/NotificationMenu.spec.js
+++ b/webapp/components/NotificationMenu/NotificationMenu.spec.js
@@ -50,7 +50,7 @@ describe('NotificationMenu.vue', () => {
       beforeEach(() => {
         data = () => {
           return {
-            displayedNotifications: [
+            notifications: [
               {
                 id: 'notification-41',
                 read: true,
@@ -85,7 +85,7 @@ describe('NotificationMenu.vue', () => {
       beforeEach(() => {
         data = () => {
           return {
-            displayedNotifications: [
+            notifications: [
               {
                 id: 'notification-41',
                 read: false,
diff --git a/webapp/components/NotificationMenu/NotificationMenu.vue b/webapp/components/NotificationMenu/NotificationMenu.vue
index 79a16157c..ff50ea1e9 100644
--- a/webapp/components/NotificationMenu/NotificationMenu.vue
+++ b/webapp/components/NotificationMenu/NotificationMenu.vue
@@ -1,5 +1,5 @@
 <template>
-  <ds-button v-if="!notificationsCount" class="notifications-menu" disabled icon="bell">
+  <ds-button v-if="!notifications.length" class="notifications-menu" disabled icon="bell">
     {{ unreadNotificationsCount }}
   </ds-button>
   <dropdown v-else class="notifications-menu" offset="8" :placement="placement">
@@ -10,7 +10,7 @@
     </template>
     <template slot="popover">
       <div class="notifications-menu-popover">
-        <notification-list :notifications="displayedNotifications" @markAsRead="markAsRead" />
+        <notification-list :notifications="notifications" @markAsRead="markAsRead" />
       </div>
       <div class="notifications-link-container">
         <nuxt-link :to="{ name: 'notifications' }">
@@ -26,6 +26,7 @@ import Dropdown from '~/components/Dropdown'
 import { NOTIFICATIONS_POLL_INTERVAL } from '~/constants/notifications'
 import { notificationQuery, markAsReadMutation } from '~/graphql/User'
 import NotificationList from '../NotificationList/NotificationList'
+import unionBy from 'lodash/unionBy'
 
 export default {
   name: 'NotificationMenu',
@@ -35,7 +36,6 @@ export default {
   },
   data() {
     return {
-      displayedNotifications: [],
       notifications: [],
     }
   },
@@ -46,39 +46,21 @@ export default {
     async markAsRead(notificationSourceId) {
       const variables = { id: notificationSourceId }
       try {
-        const {
-          data: { markAsRead },
-        } = await this.$apollo.mutate({
+        await this.$apollo.mutate({
           mutation: markAsReadMutation(this.$i18n),
           variables,
         })
-        if (!(markAsRead && markAsRead.read === true)) return
-        this.displayedNotifications = this.displayedNotifications.map(n => {
-          return this.equalNotification(n, markAsRead) ? markAsRead : n
-        })
       } catch (err) {
         this.$toast.error(err.message)
       }
     },
-    equalNotification(a, b) {
-      return a.from.id === b.from.id && a.createdAt === b.createdAt && a.reason === b.reason
-    },
   },
   computed: {
-    notificationsCount() {
-      return (this.displayedNotifications || []).length
-    },
     unreadNotificationsCount() {
-      let countUnread = 0
-      if (this.displayedNotifications) {
-        this.displayedNotifications.forEach(notification => {
-          if (!notification.read) countUnread++
-        })
-      }
-      return countUnread
-    },
-    updateNotifications() {
-      return this.notificationRead
+      const result = this.notifications.reduce((count, n) => {
+        return n.read ? count : count + 1
+      }, 0)
+      return result
     },
   },
   apollo: {
@@ -94,15 +76,9 @@ export default {
       },
       pollInterval: NOTIFICATIONS_POLL_INTERVAL,
       update({ notifications }) {
-        const newNotifications = notifications.filter(newN => {
-          return !this.displayedNotifications.find(oldN => this.equalNotification(newN, oldN))
-        })
-        this.displayedNotifications = newNotifications
-          .concat(this.displayedNotifications)
-          .sort((a, b) => {
-            return new Date(b.createdAt) - new Date(a.createdAt)
-          })
-        return notifications
+        return unionBy(notifications, this.notifications, n => n.id).sort(
+          (a, b) => new Date(b.createdAt) - new Date(a.createdAt),
+        )
       },
       error(error) {
         this.$toast.error(error.message)
diff --git a/webapp/graphql/User.js b/webapp/graphql/User.js
index 7b2a012a4..fda023d06 100644
--- a/webapp/graphql/User.js
+++ b/webapp/graphql/User.js
@@ -53,6 +53,7 @@ export const notificationQuery = i18n => {
 
     query($read: Boolean, $orderBy: NotificationOrdering, $first: Int, $offset: Int) {
       notifications(read: $read, orderBy: $orderBy, first: $first, offset: $offset) {
+        id
         read
         reason
         createdAt
@@ -81,6 +82,7 @@ export const markAsReadMutation = i18n => {
 
     mutation($id: ID!) {
       markAsRead(id: $id) {
+        id
         read
         reason
         createdAt