From 67c9c300cc70c9d94ed9f123fe54702ee993b176 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Mon, 22 Jul 2019 15:46:12 +0800 Subject: [PATCH 1/3] fix value of `isLoading` inside project topics reducer so it's `true` when any kind of topic (private or public) is loading --- src/projects/reducers/projectTopics.js | 46 +++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/src/projects/reducers/projectTopics.js b/src/projects/reducers/projectTopics.js index 0d1733c72..d239330ee 100644 --- a/src/projects/reducers/projectTopics.js +++ b/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, From c3f9ea2bc91e2da29d4c853150927332b3115d29 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Mon, 22 Jul 2019 15:46:53 +0800 Subject: [PATCH 2/3] refactored and fixed redirect old Topics and Post URLs to the new message tab --- .../detail/containers/DashboardContainer.jsx | 53 +------------------ .../containers/MessagesTabContainer.jsx | 21 +++++++- src/projects/routes.jsx | 21 +++++++- 3 files changed, 40 insertions(+), 55 deletions(-) diff --git a/src/projects/detail/containers/DashboardContainer.jsx b/src/projects/detail/containers/DashboardContainer.jsx index 7ac95936e..71fb326d0 100644 --- a/src/projects/detail/containers/DashboardContainer.jsx +++ b/src/projects/detail/containers/DashboardContainer.jsx @@ -7,7 +7,7 @@ import React from 'react' import _ from 'lodash' import { connect } from 'react-redux' -import { Redirect, withRouter, Link } from 'react-router-dom' +import { withRouter, Link } from 'react-router-dom' import Alert from 'react-s-alert' import './DashboardContainer.scss' @@ -70,14 +70,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 +96,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 +113,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 +146,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 +206,6 @@ class DashboardContainer extends React.Component { ]} /> - {matchesTopicUrl && } - {matchesPostUrl && topicIdForPost && } - {(matches) => { diff --git a/src/projects/detail/containers/MessagesTabContainer.jsx b/src/projects/detail/containers/MessagesTabContainer.jsx index b730fc1ff..c72564a0e 100644 --- a/src/projects/detail/containers/MessagesTabContainer.jsx +++ b/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)) diff --git a/src/projects/routes.jsx b/src/projects/routes.jsx index dad13ac90..247ad5664 100644 --- a/src/projects/routes.jsx +++ b/src/projects/routes.jsx @@ -21,7 +21,26 @@ const FileDownloadWithAuth = requiresAuthentication(FileDownload) const ProjectDetailWithAuth = requiresAuthentication(() => ( - } /> + { + 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 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 + } + + return + }} + /> } /> } /> } /> From 7388ea27f86d5586f48fa854f137c140f4b5ec53 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Mon, 22 Jul 2019 16:01:53 +0800 Subject: [PATCH 3/3] fix lint --- src/projects/detail/containers/DashboardContainer.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/projects/detail/containers/DashboardContainer.jsx b/src/projects/detail/containers/DashboardContainer.jsx index 71fb326d0..4673ab4b7 100644 --- a/src/projects/detail/containers/DashboardContainer.jsx +++ b/src/projects/detail/containers/DashboardContainer.jsx @@ -8,7 +8,6 @@ import React from 'react' import _ from 'lodash' import { connect } from 'react-redux' import { withRouter, Link } from 'react-router-dom' -import Alert from 'react-s-alert' import './DashboardContainer.scss' import {