1
0
mirror of https://github.com/LaCasemate/fab-manager.git synced 2025-01-17 06:52:27 +01:00

[bug] too many unread notifications cause system memory overflow

This commit is contained in:
Sylvain 2017-01-05 15:06:54 +01:00
parent 7f98cdb76d
commit c479502dd0
14 changed files with 167 additions and 88 deletions

View File

@ -1,5 +1,10 @@
# Changelog Fab Manager
## v2.4.10 2017 January 5
- Optimized notifications system
- Fix a bug: when many users with too many unread notifications are connected at the same time, the system kill the application due to memory overflow
## v2.4.9 2017 January 4
- Mask new notifications alerts when more than 3

View File

@ -60,7 +60,9 @@ Application.Controllers.controller 'ApplicationController', ["$rootScope", "$sco
Session.destroy()
$rootScope.currentUser = null
$rootScope.toCheckNotifications = false
$scope.notifications = []
$scope.notifications =
total: 0
unread: 0
$state.go('app.public.home')
, (error) ->
# An error occurred logging out.
@ -261,33 +263,30 @@ Application.Controllers.controller 'ApplicationController', ["$rootScope", "$sco
$rootScope.toCheckNotifications = true
unless $rootScope.checkNotificationsIsInit or !$rootScope.currentUser
setTimeout ->
$scope.notifications = Notification.query {is_read: false}
# we request the most recent notifications
Notification.last_unread (notifications) ->
$rootScope.lastCheck = new Date()
$scope.notifications = notifications.totals
toDisplay = []
angular.forEach notifications.notifications, (n) ->
toDisplay.push(n)
if toDisplay.length < notifications.totals.unread
toDisplay.push({message: {description: _t('and_NUMBER_other_notifications', {NUMBER: notifications.totals.unread - toDisplay.length}, "messageformat")}})
angular.forEach toDisplay, (notification) ->
growl.info(notification.message.description)
, 2000
$scope.$watch 'notifications', (newValue, oldValue) ->
diff = []
angular.forEach newValue, (value) ->
find = false
for i in [0..oldValue.length] by 1
if oldValue[i] and (value.id is oldValue[i].id)
find = true
break
unless find
diff.push(value)
remain = 3
if diff.length >= remain
diff.splice(remain, (diff.length - remain), {message: {description: _t('and_NUMBER_other_notifications', {NUMBER: diff.length - remain})}})
angular.forEach diff, (notification, key) ->
growl.info(notification.message.description)
, true
checkNotifications = ->
if $rootScope.toCheckNotifications
Notification.query({is_read: false}).$promise.then (data) ->
$scope.notifications = data;
Notification.polling({last_poll: $rootScope.lastCheck}).$promise.then (data) ->
$rootScope.lastCheck = new Date()
$scope.notifications = data.totals
angular.forEach data.notifications, (notification) ->
growl.info(notification.message.description)
$interval(checkNotifications, NOTIFICATIONS_CHECK_PERIOD)
$rootScope.checkNotificationsIsInit = true

View File

@ -225,7 +225,9 @@ Application.Controllers.controller "EditProfileController", ["$scope", "$rootSco
Session.destroy()
$rootScope.currentUser = null
$rootScope.toCheckNotifications = false
$scope.notifications = []
$scope.notifications =
total: 0
unread: 0
$window.location.href = $scope.activeProvider.link_to_sso_connect

View File

@ -2,7 +2,7 @@
##
# Controller used in notifications page
# inherits $scope.$parent.notifications (unread notifications) from ApplicationController
# inherits $scope.$parent.notifications (global notifications state) from ApplicationController
##
Application.Controllers.controller "NotificationsController", ["$scope", 'Notification', ($scope, Notification) ->
@ -20,6 +20,15 @@ Application.Controllers.controller "NotificationsController", ["$scope", 'Notifi
## Array containg the archived notifications (already read)
$scope.notificationsRead = []
## Array containg the new notifications (not read)
$scope.notificationsUnread = []
## Total number of notifications for the current user
$scope.total = 0
## Total number of unread notifications for the current user
$scope.totalUnread = 0
## By default, the pagination mode is activated to limit the page size
$scope.paginateActive = true
@ -39,10 +48,15 @@ Application.Controllers.controller "NotificationsController", ["$scope", 'Notifi
Notification.update {id: notification.id},
id: notification.id
is_read: true
, ->
index = $scope.$parent.notifications.indexOf(notification)
$scope.$parent.notifications.splice(index,1)
$scope.notificationsRead.push notification
, (updatedNotif) ->
# remove notif from unreads
index = $scope.notificationsUnread.indexOf(notification)
$scope.notificationsUnread.splice(index,1)
# add update notif to read
$scope.notificationsRead.push updatedNotif
# update counters
$scope.$parent.notifications.unread -= 1
$scope.totalUnread -= 1
@ -52,21 +66,32 @@ Application.Controllers.controller "NotificationsController", ["$scope", 'Notifi
$scope.markAllAsRead = ->
Notification.update {}
, -> # success
angular.forEach $scope.$parent.notifications, (n)->
# add notifs to read
angular.forEach $scope.notificationsUnread, (n)->
n.is_read = true
$scope.notificationsRead.push n
$scope.$parent.notifications.splice(0, $scope.$parent.notifications.length)
# clear unread
$scope.notificationsUnread = []
# update counters
$scope.$parent.notifications.unread = 0
$scope.totalUnread = 0
##
# Request the server to retrieve the next undisplayed notifications and add them
# to the archived notifications list.
# Request the server to retrieve the next notifications and add them
# to their corresponding notifications list (read or unread).
##
$scope.addMoreNotificationsReaded = ->
Notification.query {is_read: true, page: $scope.page}, (notifications) ->
$scope.notificationsRead = $scope.notificationsRead.concat notifications
$scope.paginateActive = false if notifications.length < NOTIFICATIONS_PER_PAGE
$scope.addMoreNotifications = ->
Notification.query {page: $scope.page}, (notifications) ->
$scope.total = notifications.totals.total
$scope.totalUnread = notifications.totals.unread
angular.forEach notifications.notifications, (notif) ->
if notif.is_read
$scope.notificationsRead.push(notif)
else
$scope.notificationsUnread.push(notif)
$scope.paginateActive = (notifications.totals.total > ($scope.notificationsRead.length + $scope.notificationsUnread.length))
$scope.page += 1
@ -78,7 +103,7 @@ Application.Controllers.controller "NotificationsController", ["$scope", 'Notifi
# Kind of constructor: these actions will be realized first when the controller is loaded
##
initialize = ->
$scope.addMoreNotificationsReaded()
$scope.addMoreNotifications()

View File

@ -170,7 +170,9 @@ Application.Controllers.controller "CompleteProfileController", ["$scope", "$roo
Session.destroy()
$rootScope.currentUser = null
$rootScope.toCheckNotifications = false
$scope.notifications = []
$scope.notifications =
total: 0
unread: 0
$window.location.href = activeProviderPromise.link_to_sso_connect

View File

@ -3,6 +3,14 @@
Application.Services.factory 'Notification', ["$resource", ($resource)->
$resource "/api/notifications/:id",
{id: "@id"},
query:
isArray: false
update:
method: 'PUT'
polling:
url: '/api/notifications/polling'
method: 'GET'
last_unread:
url: '/api/notifications/last_unread'
method: 'GET'
]

View File

@ -19,7 +19,7 @@
<div class="row">
<div class="col-md-12">
<button type="button" class="btn btn-warning m-t-sm m-b" ng-click="markAllAsRead()" ng-disabled="notifications.length == 0">{{ 'mark_all_as_read' | translate }} ({{notifications.length}})</button>
<button type="button" class="btn btn-warning m-t-sm m-b" ng-click="markAllAsRead()" ng-disabled="totalUnread == 0">{{ 'mark_all_as_read' | translate }} ({{totalUnread}})</button>
<table class="table">
<thead>
@ -31,7 +31,7 @@
</tr>
</thead>
<tbody>
<tr ng-repeat="notification in notifications" ng-if="notifications.length > 0">
<tr ng-repeat="notification in notificationsUnread" ng-if="notificationsUnread.length > 0">
<td>
<button class="btn btn-sm btn-warning" ng-click="markAsRead(notification, $event)">
<i class="fa fa-check"></i>
@ -41,45 +41,47 @@
<td ng-bind-html="notification.message.description"></td>
</tr>
<tr ng-if="notifications.length == 0">
<tr ng-if="notificationsUnread.length == 0">
<td colspan="3" translate>{{ 'no_new_notifications' }}</td>
</tr>
</tbody>
</table>
<h5 translate>{{ 'archives' }}</h5>
<div ng-hide="notificationsRead.length == 0 && notificationsUnread.length < total">
<h5 translate>{{ 'archives' }}</h5>
<table class="table">
<thead>
<tr>
<th style="width:10%"></th>
<th style="width:20%"></th>
<th style="width:70%"></th>
<table class="table">
<thead>
<tr>
<th style="width:10%"></th>
<th style="width:20%"></th>
<th style="width:70%"></th>
</tr>
</thead>
<tbody>
</tr>
</thead>
<tbody>
<tr class="read" ng-repeat="n in notificationsRead | orderBy:'created_at':true" ng-if="notificationsRead.length > 0">
<td>
</td>
<td>{{ n.created_at | amDateFormat:'LLL' }}</td>
<td ng-bind-html="n.message.description"></td>
<tr class="read" ng-repeat="n in notificationsRead | orderBy:'created_at':true" ng-if="notificationsRead.length > 0">
<td>
</td>
<td>{{ n.created_at | amDateFormat:'LLL' }}</td>
<td ng-bind-html="n.message.description"></td>
</tr>
</tr>
<tr ng-if="notificationsRead.length == 0">
<td colspan="3" translate>{{ 'no_archived_notifications' }}</td>
</tr>
<tr ng-if="notificationsRead.length == 0">
<td colspan="3" translate>{{ 'no_archived_notifications' }}</td>
</tr>
</tbody>
</table>
</tbody>
</table>
</div>
<a class="btn btn-default" ng-click="addMoreNotificationsReaded()" ng-if="paginateActive" translate>{{ 'load_the_next_notifications' }}</a>
<a class="btn btn-default" ng-click="addMoreNotifications()" ng-if="paginateActive" translate>{{ 'load_the_next_notifications' }}</a>
</div>

View File

@ -23,7 +23,7 @@
<!-- Top Nav -->
<ul class="nav navbar-nav navbar-right m-n hidden-xs nav-user user">
<li class="notification-open" ng-if="isAuthenticated()">
<a ui-sref="app.logged.notifications"><i class="fa fa-bell fa-2x black"></i> <span class="badge" ng-class="{'bg-red': notifications.length > 0}">{{notifications.length}}</span></a>
<a ui-sref="app.logged.notifications"><i class="fa fa-bell fa-2x black"></i> <span class="badge" ng-class="{'bg-red': notifications.unread > 0}">{{notifications.unread}}</span></a>
</li>
<li class="dropdown user-profile-nav" ng-if="isAuthenticated()" uib-dropdown>
<a href="#" class="dropdown-toggle" uib-dropdown-toggle>

View File

@ -3,14 +3,49 @@ class API::NotificationsController < API::ApiController
before_action :authenticate_user!
def index
if params[:is_read]
if params[:is_read] == 'true'
@notifications = current_user.notifications.where(is_read: true).page(params[:page]).per(15).order('created_at DESC')
else
@notifications = current_user.notifications.where(is_read: false).order('created_at DESC')
end
else
@notifications = current_user.notifications.order('created_at DESC')
loop do
@notifications = current_user.notifications.page(params[:page]).per(15).order('created_at DESC')
# we delete obsolete notifications on first access
break unless delete_obsoletes(@notifications)
end
@totals = {
total: current_user.notifications.count,
unread: current_user.notifications.where(is_read: false).count
}
render :index
end
def last_unread
loop do
@notifications = current_user.notifications.where(is_read: false).limit(3).order('created_at DESC')
# we delete obsolete notifications on first access
break unless delete_obsoletes(@notifications)
end
@totals = {
total: current_user.notifications.count,
unread: current_user.notifications.where(is_read: false).count
}
render :index
end
def polling
@notifications = current_user.notifications.where('is_read = false AND created_at >= :date', date: params[:last_poll]).order('created_at DESC')
@totals = {
total: current_user.notifications.count,
unread: current_user.notifications.where(is_read: false).count
}
render :index
end
private
def delete_obsoletes(notifications)
cleaned = false
notifications.each do |n|
if !Module.const_get(n.attached_object_type) or !n.attached_object
n.destroy!
cleaned = true
end
end
cleaned
end
end

View File

@ -1,13 +1,12 @@
json.array!(@notifications) do |notification|
if Module.const_get(notification.attached_object_type) and notification.attached_object # WHY WERE WE DOING Object.const_defined?(notification.attached_object_type) ??? Why not just deleting obsolete notifications ?! Object.const_defined? was introducing a bug! Module.const_get is a TEMPORARY fix, NOT a solution
json.extract! notification, :id, :notification_type_id, :notification_type, :created_at, :is_read
json.attached_object notification.attached_object
json.message do
if notification.notification_type.nil?
json.partial! 'api/notifications/undefined_notification', notification: notification
else
json.partial! "/api/notifications/#{notification.notification_type}", notification: notification
end
json.totals @totals
json.notifications(@notifications) do |notification|
json.extract! notification, :id, :notification_type_id, :notification_type, :created_at, :is_read
json.attached_object notification.attached_object
json.message do
if notification.notification_type.nil?
json.partial! 'api/notifications/undefined_notification', notification: notification
else
json.partial! "/api/notifications/#{notification.notification_type}", notification: notification
end
end
end.delete_if {|n| n['id'] == nil }

View File

@ -104,7 +104,7 @@ en:
version: "Version:"
# Notifications
and_NUMBER_other_notifications: "and {{NUMBER}} other notifications..." # angular interpolation
and_NUMBER_other_notifications: "and {NUMBER, plural, =0{no other notifications} =1{one other notification} other{{NUMBER} other notifications}}..." # messageFormat interpolation
about:
# about page

View File

@ -104,7 +104,7 @@ fr:
version: "Version :"
# Notifications
and_NUMBER_other_notifications: "et {{NUMBER}} autres notifications ..." # angular interpolation
and_NUMBER_other_notifications: "et {NUMBER, plural, =0{aucune autre notification} =1{une autre notification} other{{NUMBER} autres notifications}} ..." # messageFormat interpolation
about:
# page à propos

View File

@ -260,7 +260,7 @@ fr:
your_invoice_is_ready_html: "Votre facture n°%{REFERENCE}, d'un montant de %{AMOUNT}, est prête. <a href='api/invoices/%{INVOICE_ID}/download' target='_blank'>Cliquez ici pour la télécharger</a>."
undefined_notification:
unknown_notification: "Notification inconnue"
notification_ID_wrong_type_TYPE_unknown: "Notification {ID} erronée (type {TYPE} inconnu)."
notification_ID_wrong_type_TYPE_unknown: "Notification %{ID} erronée (type %{TYPE} inconnu)."
notify_user_wallet_is_credited:
your_wallet_is_credited: "Votre porte-monnaie a bien été crédité de %{AMOUNT} par l'administrateur"
notify_admin_user_wallet_is_credited:

View File

@ -53,6 +53,8 @@ Rails.application.routes.draw do
resources :reservations, only: [:show, :create, :index, :update]
resources :notifications, only: [:index, :show, :update] do
match :update_all, path: '/', via: [:put, :patch], on: :collection
get 'polling', action: 'polling', on: :collection
get 'last_unread', action: 'last_unread', on: :collection
end
resources :wallet, only: [] do
get '/by_user/:user_id', action: 'by_user', on: :collection