Merge pull request #3204 from appirio-tech/hotfix/redirect-comments

Hotfix/post release 2.4.13.6

Author: vikasrohit

src/projects/detail/containers/DashboardContainer.jsx

@@ -7,8 +7,7 @@
 import React from 'react'
 import _ from 'lodash'
 import { connect } from 'react-redux'
-import { Redirect, withRouter, Link } from 'react-router-dom'
-import Alert from 'react-s-alert'
+import { withRouter, Link } from 'react-router-dom'
 import './DashboardContainer.scss'
 
 import {
@@ -70,14 +69,9 @@ class DashboardContainer extends React.Component {
 
     this.state = {
       open: false,
-      matchesTopicUrl: null,
-      matchesPostUrl: null,
-      topicIdForPost: null
     }
     this.onNotificationRead = this.onNotificationRead.bind(this)
     this.toggleDrawer = this.toggleDrawer.bind(this)
-
-    this.alertedFailedTopicRedirect = false
   }
 
   onNotificationRead(notification) {
@@ -101,18 +95,6 @@ class DashboardContainer extends React.Component {
       })
     }
 
-    /*
-    For redirecting old urls to new urls for topics and posts
-    Old TOPIC: '/projects/{{projectId}}/#feed-{{topicId}}',
-    Old POST: '/projects/{{projectId}}/#comment-{{postId}}',
-    */
-    const matchesTopicUrl = location.hash.match(/#feed-(\d+)/)
-    const matchesPostUrl = location.hash.match(/#comment-(\d+)/)
-    this.setState({
-      matchesPostUrl,
-      matchesTopicUrl
-    })
-
     // if the user is a customer and its not a direct link to a particular phase
     // then by default expand all phases which are active
     if (_.isEmpty(location.hash) && this.props.isCustomerUser) {
@@ -130,40 +112,12 @@ class DashboardContainer extends React.Component {
     collapseAllProjectPhases()
   }
 
-  componentWillReceiveProps(nextProps) {
-    const { isFeedsLoading } = nextProps
-    const { matchesPostUrl } = this.state
-
-    // we need topicId for redirecting old post url (/projects/{{projectId}}/#comment-{{postId}})
-    if (!isFeedsLoading && matchesPostUrl && !this.alertedFailedTopicRedirect) {
-      const topicIdForPost = this.getTopicIdForPost(matchesPostUrl[1])
-      this.setState({ topicIdForPost })
-      this.alertFailedTopicRedirection(matchesPostUrl, topicIdForPost, isFeedsLoading)
-    }
-  }
-
   toggleDrawer() {
     this.setState((prevState) => ({
       open: !prevState.open
     }))
   }
 
-  // Get topic id corresponding to the post that we're trying to redirect to
-  getTopicIdForPost(postId) {
-    const {feeds} = this.props
-    const topic = feeds && feeds
-      .find(feed => feed.posts.find(p => p.id === Number(postId)))
-    return topic && topic.id
-  }
-
-  // Alert user in case the post is not available / not accessible to him.
-  alertFailedTopicRedirection(matchesPostUrl, topicIdForPost, isFeedsLoading) {
-    if (matchesPostUrl && !topicIdForPost && !isFeedsLoading) {
-      this.alertedFailedTopicRedirect = true
-      Alert.error('Couldn\'t find the post')
-    }
-  }
-
   render() {
     const {
       project,
@@ -191,9 +145,6 @@ class DashboardContainer extends React.Component {
       location,
       estimationQuestion,
     } = this.props
-    const { matchesPostUrl, matchesTopicUrl, topicIdForPost } = this.state
-
-
     const projectTemplate = project && project.templateId && projectTemplates ? (getProjectTemplateById(projectTemplates, project.templateId)) : null
 
     let template
@@ -254,9 +205,6 @@ class DashboardContainer extends React.Component {
           ]}
         />
 
-        {matchesTopicUrl && <Redirect to={`/projects/${project.id}/messages/${matchesTopicUrl[1]}`} />}
-        {matchesPostUrl && topicIdForPost && <Redirect to={`/projects/${project.id}/messages/${topicIdForPost}#comment-${matchesPostUrl[1]}`}  />}
-
         <TwoColsLayout.Sidebar>
           <MediaQuery minWidth={SCREEN_BREAKPOINT_MD}>
             {(matches) => {

src/projects/detail/containers/MessagesTabContainer.jsx

@@ -1,6 +1,7 @@
 import React from 'react'
 import { connect } from 'react-redux'
-import { Prompt } from 'react-router-dom'
+import { Prompt, withRouter } from 'react-router-dom'
+import Alert from 'react-s-alert'
 import MediaQuery from 'react-responsive'
 import {
   groupBy,
@@ -189,6 +190,22 @@ class MessagesTabContainer extends React.Component {
   }
 
   componentWillReceiveProps(nextProps) {
+    // if we have URL like `/projects/{projectId}/messages#comment-{postId}` without mentioning topicId
+    // then we should try to find topic of such post and redirect to the full URL with topicId
+    const matchesPostUrl = this.props.match.path === '/projects/:projectId/messages' &&
+      this.props.location.hash.match(/#comment-(\d+)/)
+    // as soon as all topics are loaded we will redirect to the correct URL, if there such topic
+    if (this.props.isFeedsLoading && !nextProps.isFeedsLoading && matchesPostUrl) {
+      const postId = parseInt(matchesPostUrl[1], 10)
+      const topic = nextProps.feeds.find(feed => _.find(feed.posts, { id: postId }))
+
+      if (topic) {
+        this.props.history.replace(`/projects/${this.props.match.params.projectId}/messages/${topic.id}#comment-${postId}`)
+      } else {
+        Alert.error('Couldn\'t find the post referred in URL')
+      }
+    }
+
     // reset title and content in the state after successful post creation
     // so that we treat the post editor not changed, thus when we leave the page we don't get confirmation alert
     if (this.props.isCreatingFeed && !nextProps.isCreatingFeed && !nextProps.error) {
@@ -421,4 +438,4 @@ const mapDispatchToProps = {
 export default connect(
   mapStateToProps,
   mapDispatchToProps
-)(MessagesTabContainer)
+)(withRouter(MessagesTabContainer))

src/projects/reducers/projectTopics.js

@@ -37,8 +37,15 @@ import {
 import update from 'react-addons-update'
 import { getLinksFromPost } from '../../helpers/draftJSHelper'
 
-
 const initialState = {
+  // as we can load different kind of topics in parallel we have to track their loading status separately
+  isLoadingPerTag: {
+    MESSAGES: false,
+    PRIMARY: false,
+  },
+  // `isLoading` would indicate the loading of any kind of topic as aggregation value for the values above
+  // technically we could aggregate inside components, but as we already use `isLoading` in many places
+  // so we do it inside the reducer for backward compatibility
   isLoading: false,
   isCreatingFeed: false,
   error: false,
@@ -49,6 +56,28 @@ const initialState = {
   }
 }
 
+/**
+ * Builds updated value of `isLoadingPerTag` state
+ *
+ * @param {Object}  currentIsLoadingPerTag current `isLoadingPerTag` state
+ * @param {String}  tag                    topic tag
+ * @param {Boolean} isLoading              `true` if topic with such tag is loading now
+ *
+ * @returns {Object} update value for `isLoadingPerTag` state
+ */
+const updateIsLoadingPerTag = (currentIsLoadingPerTag, tag, isLoading) => ({
+  ...currentIsLoadingPerTag,
+  [tag]: isLoading,
+})
+
+/**
+ * Calculates values for `isLoading` state value based on `isLoadingPerTag` state
+ *
+ * @param {Object} isLoadingPerTag `isLoadingPerTag` state
+ *
+ * @returns {Boolean} `true` if topic with any tag is loaded
+ */
+const getIsLoading = (isLoadingPerTag) => _.some(_.values(isLoadingPerTag))
 
 export const projectTopics = function (state=initialState, action) {
   const payload = action.payload
@@ -63,9 +92,11 @@ export const projectTopics = function (state=initialState, action) {
     if (action.meta.projectId === state.projectId) {
       const feedUpdateQuery = {}
       feedUpdateQuery[action.meta.tag] = { $merge: { topics: [], totalCount: 0 } }
+      const updatedIsLoadingPerTag = updateIsLoadingPerTag(state.isLoadingPerTag, action.meta.tag, true)
       return update(state, {
         feeds: feedUpdateQuery,
-        isLoading: { $set: true },
+        isLoadingPerTag: { $set: updatedIsLoadingPerTag },
+        isLoading: { $set: getIsLoading(updatedIsLoadingPerTag) },
         error: { $set: false }
       })
     } else {
@@ -101,18 +132,23 @@ export const projectTopics = function (state=initialState, action) {
 
     const feedUpdateQuery = {}
     feedUpdateQuery[action.meta.tag] = { $merge: { topics, totalCount: payload.totalCount } }
+    const updatedIsLoadingPerTag = updateIsLoadingPerTag(state.isLoadingPerTag, action.meta.tag, false)
     return update(state, {
-      isLoading: {$set: false},
+      isLoadingPerTag: {$set: updatedIsLoadingPerTag},
+      isLoading: {$set: getIsLoading(updatedIsLoadingPerTag)},
       error: {$set: false},
       feeds: feedUpdateQuery
     })
   }
   case LOAD_PROJECT_FEEDS_MEMBERS_FAILURE:
-  case LOAD_PROJECT_FEEDS_FAILURE:
+  case LOAD_PROJECT_FEEDS_FAILURE: {
+    const updatedIsLoadingPerTag = updateIsLoadingPerTag(state.isLoadingPerTag, action.meta.tag, false)
     return Object.assign({}, initialState, {
-      isLoading: false,
+      isLoadingPerTag: {$set: updatedIsLoadingPerTag},
+      isLoading: {$set: getIsLoading(updatedIsLoadingPerTag)},
       error: true
     })
+  }
   case CREATE_PROJECT_FEED_PENDING:
     return Object.assign({}, state, {
       isCreatingFeed: true,

src/projects/routes.jsx

@@ -21,7 +21,26 @@ const FileDownloadWithAuth = requiresAuthentication(FileDownload)
 
 const ProjectDetailWithAuth = requiresAuthentication(() =>
   (<Switch>
-    <Route exact path="/projects/:projectId" render={() => <ProjectDetail component={Dashboard} />} />
+    <Route
+      exact
+      path="/projects/:projectId"
+      render={({ match, location }) => {
+        const matchesTopicUrl = location.hash.match(/#feed-(\d+)/)
+        const matchesPostUrl = location.hash.match(/#comment-(\d+)/)
+
+        // redirect old Topic URLs to the topics on the messages tab
+        if (matchesTopicUrl) {
+          return <Redirect to={`/projects/${match.params.projectId}/messages/${matchesTopicUrl[1]}`} />
+
+        // redirect old Posts URLs to the messages tab
+        // as we don't know the topic ID form the URL, message tab should take care about it
+        } else if (matchesPostUrl) {
+          return <Redirect to={`/projects/${match.params.projectId}/messages${location.hash}`} />
+        }
+
+        return <ProjectDetail component={Dashboard} />
+      }}
+    />
     <Route path="/projects/:projectId/status/:statusId" render={() => <ProjectDetail component={Dashboard} />} />
     <Route path="/projects/:projectId/messages/:topicId" render={() => <ProjectDetail component={MessagesTabContainer} />} />
     <Route path="/projects/:projectId/messages" render={() => <ProjectDetail component={MessagesTabContainer} />} />