Chore: FCM Push Fixes (#934)

app/controllers/api/v1/accounts/notifications_controller.rb

@@ -4,8 +4,13 @@ class Api::V1::Accounts::NotificationsController < Api::V1::Accounts::BaseContro
   before_action :fetch_notification, only: [:update]
 
   def index
-    @notifications = current_user.notifications.where(account_id: Current.account.id)
+    @unread_count = current_user.notifications.where(account_id: current_account.id, read_at: nil).count
-    render json: @notifications
+    @notifications = current_user.notifications.where(account_id: current_account.id).page params[:page]
+  end
+
+  def read_all
+    current_user.notifications.where(account_id: current_account.id, read_at: nil).update(read_at: DateTime.now.utc)
+    head :ok
   end
 
   def update

app/models/notification.rb

@@ -38,6 +38,29 @@ class Notification < ApplicationRecord
 
   after_create_commit :process_notification_delivery
 
+  def push_event_data
+    {
+      id: id,
+      notification_type: notification_type,
+      primary_actor: primary_actor&.push_event_data,
+      read_at: read_at,
+      secondary_actor: secondary_actor&.push_event_data,
+      user: user&.push_event_data,
+      created_at: created_at
+    }
+  end
+
+  # TODO: move to a data presenter
+  def push_message_title
+    if notification_type == 'conversation_creation'
+      return "A new conversation [ID -#{primary_actor.display_id}] has been created in #{primary_actor.inbox.name}"
+    end
+
+    return "A new conversation [ID -#{primary_actor.display_id}] has been assigned to you." if notification_type == 'conversation_assignment'
+
+    ''
+  end
+
   private
 
   def process_notification_delivery

app/services/notification/push_notification_service.rb

@@ -29,21 +29,9 @@ class Notification::PushNotificationService
     @conversation ||= notification.primary_actor
   end
 
-  def push_message_title
-    if notification.notification_type == 'conversation_creation'
-      return "A new conversation [ID -#{conversation.display_id}] has been created in #{conversation.inbox.name}"
-    end
-
-    if notification.notification_type == 'conversation_assignment'
-      return "A new conversation [ID -#{conversation.display_id}] has been assigned to you."
-    end
-
-    ''
-  end
-
   def push_message
     {
-      title: push_message_title,
+      title: notification.push_message_title,
       tag: "#{notification.notification_type}_#{conversation.display_id}",
       url: push_url
     }
@@ -78,11 +66,14 @@ class Notification::PushNotificationService
     return unless subscription.fcm?
 
     fcm = FCM.new(ENV['FCM_SERVER_KEY'])
-    options = { "notification": {
+    options = {
-      "title": notification.notification_type.titleize,
+      "notification": {
-      "body": push_message_title,
+        "title": notification.notification_type.titleize,
-      "notification": notification.to_json
+        "body": notification.push_message_title
-    } }
+      },
+      "data": { notification: notification.push_event_data.to_json }
+    }
+
     response = fcm.send([subscription.subscription_attributes['push_token']], options)
     subscription.destroy! if JSON.parse(response[:body])['results']&.first&.keys&.include?('error')
   end

app/views/api/v1/accounts/notifications/index.json.jbuilder

@@ -0,0 +1,18 @@
+json.data do
+  json.meta do
+    json.unread_count @unread_count
+  end
+
+  json.payload do
+    json.array! @notifications do |notification|
+      json.id notification.id
+      json.notification_type notification.notification_type
+      json.push_message_title notification.push_message_title
+      json.primary_actor notification.primary_actor&.push_event_data
+      json.read_at notification.read_at
+      json.secondary_actor notification.secondary_actor&.push_event_data
+      json.user notification.user&.push_event_data
+      json.created_at notification.created_at.to_i
+    end
+  end
+end

config/routes.rb

@@ -80,7 +80,11 @@ Rails.application.routes.draw do
           resources :inbox_members, only: [:create, :show], param: :inbox_id
           resources :labels, only: [:index, :show, :create, :update, :destroy]
 
-          resources :notifications, only: [:index, :update]
+          resources :notifications, only: [:index, :update] do
+            collection do
+              post :read_all
+            end
+          end
           resource :notification_settings, only: [:show, :update]
 
           resources :webhooks, except: [:show]

spec/controllers/api/v1/accounts/notifications_controller_spec.rb

@@ -21,8 +21,38 @@ RSpec.describe 'Notifications API', type: :request do
             headers: admin.create_new_auth_token,
             as: :json
 
+        response_json = JSON.parse(response.body)
         expect(response).to have_http_status(:success)
         expect(response.body).to include(notification.notification_type)
+        expect(response_json['data']['meta']['unread_count']).to eq 1
+      end
+    end
+  end
+
+  describe 'POST /api/v1/accounts/{account.id}/notifications/read_all' do
+    let(:admin) { create(:user, account: account, role: :administrator) }
+    let!(:notification1) { create(:notification, account: account, user: admin) }
+    let!(:notification2) { create(:notification, account: account, user: admin) }
+
+    context 'when it is an unauthenticated user' do
+      it 'returns unauthorized' do
+        post "/api/v1/accounts/#{account.id}/notifications/read_all"
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context 'when it is an authenticated user' do
+      let(:admin) { create(:user, account: account, role: :administrator) }
+
+      it 'updates all the notifications read at' do
+        post "/api/v1/accounts/#{account.id}/notifications/read_all",
+             headers: admin.create_new_auth_token,
+             as: :json
+
+        expect(response).to have_http_status(:success)
+        expect(notification1.reload.read_at).not_to eq('')
+        expect(notification2.reload.read_at).not_to eq('')
       end
     end
   end