issue #1985 - simplified version of NotificationsReader - marks notifications as read only on mount. It works faster and still covers all current use-cases.

Author: maxceem

src/components/NotificationsReader/NotificationsReader.jsx

@@ -7,25 +7,13 @@
 import React from 'react'
 import PT from 'prop-types'
 import { connect } from 'react-redux'
-import { startReadingNotifications, stopReadingNotifications } from '../../routes/notifications/actions'
+import { markNotificationsReadByCriteria } from '../../routes/notifications/actions'
 
 class NotificationsReader extends React.Component {
-  constructor(props) {
-    super(props)
-
-    this.notificationsReaderUid = null
-  }
-
   componentWillMount() {
-    const { id, criteria, startReadingNotifications } = this.props
-
-    this.notificationsReaderUid = startReadingNotifications(criteria, id)
-  }
-
-  componentWillUnmount() {
-    const { stopReadingNotifications } = this.props
+    const { criteria } = this.props
 
-    stopReadingNotifications(this.notificationsReaderUid)
+    markNotificationsReadByCriteria(criteria)
   }
 
   render() {
@@ -43,8 +31,7 @@ NotificationsReader.propTypes = {
 const mapStateToProps = () => ({})
 
 const mapDispatchToProps = {
-  startReadingNotifications,
-  stopReadingNotifications,
+  markNotificationsReadByCriteria,
 }
 
 export default connect(mapStateToProps, mapDispatchToProps)(NotificationsReader)

src/config/constants.js

@@ -21,8 +21,6 @@ export const HIDE_OLDER_NOTIFICATIONS_SUCCESS = 'HIDE_OLDER_NOTIFICATIONS_SUCCES
 export const NOTIFICATIONS_PENDING = 'NOTIFICATIONS_PENDING'
 export const TOGGLE_NOTIFICATIONS_DROPDOWN_MOBILE = 'TOGGLE_NOTIFICATIONS_DROPDOWN_MOBILE'
 export const TOGGLE_NOTIFICATIONS_DROPDOWN_WEB = 'TOGGLE_NOTIFICATIONS_DROPDOWN_WEB'
-export const START_READING_NOTIFICATIONS = 'START_READING_NOTIFICATIONS'
-export const STOP_READING_NOTIFICATIONS = 'STOP_READING_NOTIFICATIONS'
 export const MARK_NOTIFICATIONS_READ = 'MARK_NOTIFICATIONS_READ'
 
 // Settings

src/projects/detail/containers/FeedContainer.js

@@ -85,10 +85,6 @@ class FeedView extends React.Component {
     window.removeEventListener('beforeunload', this.onLeave)
   }
 
-  shouldComponentUpdate(nextProps) {
-    console.warn('shouldComponentUpdate')
-  }
-
   // Notify user if they navigate away while the form is modified.
   onLeave(e = {}) {
     if (this.isChanged()) {
@@ -370,7 +366,6 @@ class FeedView extends React.Component {
   }
 
   render () {
-    console.warn('FeedContainer render')
     const {currentUser, currentMemberRole, isCreatingFeed, error, allMembers,
       toggleNotificationRead, notifications, project, isSuperUser } = this.props
     const { feeds, isNewPostMobileOpen, fullscreenFeedId } = this.state

src/routes/notifications/actions/index.js

@@ -15,8 +15,6 @@ import {
   NOTIFICATIONS_PENDING,
   TOGGLE_NOTIFICATIONS_DROPDOWN_MOBILE,
   TOGGLE_NOTIFICATIONS_DROPDOWN_WEB,
-  START_READING_NOTIFICATIONS,
-  STOP_READING_NOTIFICATIONS,
   MARK_NOTIFICATIONS_READ,
 } from '../../../config/constants'
 import notificationsService from '../services/notifications.js'
@@ -165,52 +163,4 @@ export const markNotificationsRead = (notificationIds) => (dispatch) => {
   }).catch(err => {
     Alert.error(`Failed to mark notification read. ${err.message}`)
   })
-}
-
-/**
- * After this action is performed all notification which met the provided criteria
- * will be marked as read.
- * New notifications which are loaded will be also marked as read if met the provided criteria.
- * 
- * @param {Object|Array} criteria rules or list of rules for notifications which has to be marked as read
- * 
- * @returns {String}              this action returns uid which can be used to stop reading notifications
- */
-export const startReadingNotifications = (criteria, id) => (dispatch, getState) => {
-  // if custom id was provided, make sure it's unique
-  if (id) {
-    const existentReader = _.get(getState(), `notifications.readers[${id}]`)
-
-    if (existentReader) {
-      console.warn(`Notification reader with id '${id}' is already registered.`)
-      return null
-    }
-  }
-  
-  // if id wasn't provided, generate unique id
-  const uid = id ? id : _.uniqueId('reader-')
-
-  dispatch({
-    type: START_READING_NOTIFICATIONS,
-    payload: {
-      uid,
-      criteria,
-    }
-  })
-
-  markNotificationsReadByCriteria(criteria)(dispatch, getState)
-
-  return uid
-}
-
-/**
- * This action will stop marking notifications as read.
- * 
- * @param {String} uid uid of the notification reader
- */
-export const stopReadingNotifications = (uid) => (dispatch) => dispatch({
-  type: STOP_READING_NOTIFICATIONS,
-  payload: {
-    uid,
-  }
-})
+}

src/routes/notifications/reducers/index.js

@@ -16,8 +16,6 @@ import {
   TOGGLE_NOTIFICATIONS_DROPDOWN_MOBILE,
   TOGGLE_NOTIFICATIONS_DROPDOWN_WEB,
   MARK_NOTIFICATIONS_READ,
-  START_READING_NOTIFICATIONS,
-  STOP_READING_NOTIFICATIONS,
 } from '../../../config/constants'
 import _ from 'lodash'
 
@@ -120,23 +118,6 @@ export default (state = initialState, action) => {
     return newState
   }
 
-  case START_READING_NOTIFICATIONS: {
-    return {
-      ...state,
-      readers: {
-        ...state.readers,
-        [action.payload.uid]: action.payload.criteria
-      }
-    }
-  }
-
-  case STOP_READING_NOTIFICATIONS: {
-    return {
-      ...state,
-      readers: _.omit(state.readers, action.payload.uid)
-    }
-  }
-
   case VIEW_OLDER_NOTIFICATIONS_SUCCESS:
     return {...state,
       oldSourceIds: [...state.oldSourceIds, action.payload]