[MI-2547]:Fixed review fixes for Github issue 613 and PR 626

server/plugin/api.go

@@ -704,6 +704,7 @@ func (p *Plugin) getUnreadsData(c *UserContext) []*FilteredNotification {
 }
 
 func (p *Plugin) getReviewsData(c *UserContext) []*github.Issue {
+	var issue []*github.Issue
 	config := p.getConfiguration()
 
 	githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)
@@ -713,13 +714,14 @@ func (p *Plugin) getReviewsData(c *UserContext) []*github.Issue {
 	result, _, err := githubClient.Search.Issues(c.Ctx, query, &github.SearchOptions{})
 	if err != nil {
 		c.Log.WithError(err).With(logger.LogContext{"query": query}).Warnf("Failed to search for review")
-		return nil
+		return issue
 	}
 
 	return result.Issues
 }
 
 func (p *Plugin) getYourPrsData(c *UserContext) []*github.Issue {
+	var issue []*github.Issue
 	config := p.getConfiguration()
 
 	githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)
@@ -729,7 +731,7 @@ func (p *Plugin) getYourPrsData(c *UserContext) []*github.Issue {
 	result, _, err := githubClient.Search.Issues(c.Ctx, query, &github.SearchOptions{})
 	if err != nil {
 		c.Log.WithError(err).With(logger.LogContext{"query": query}).Warnf("Failed to search for PRs")
-		return nil
+		return issue
 	}
 
 	return result.Issues
@@ -978,6 +980,7 @@ func (p *Plugin) createIssueComment(c *UserContext, w http.ResponseWriter, r *ht
 }
 
 func (p *Plugin) getYourAssignmentsData(c *UserContext) []*github.Issue {
+	var issue []*github.Issue
 	config := p.getConfiguration()
 
 	githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)
@@ -987,7 +990,7 @@ func (p *Plugin) getYourAssignmentsData(c *UserContext) []*github.Issue {
 	result, _, err := githubClient.Search.Issues(c.Ctx, query, &github.SearchOptions{})
 	if err != nil {
 		c.Log.WithError(err).With(logger.LogContext{"query": query}).Warnf("Failed to search for assignments")
-		return nil
+		return issue
 	}
 
 	return result.Issues

server/plugin/plugin.go

@@ -804,10 +804,6 @@ func (p *Plugin) sendRefreshEvent(userID string) {
 		return
 	}
 
-	context.Log = context.Log.With(logger.LogContext{
-		"github username": info.GitHubUsername,
-	})
-
 	userContext := &UserContext{
 		Context: *context,
 		GHInfo:  info,
@@ -817,7 +813,7 @@ func (p *Plugin) sendRefreshEvent(userID string) {
 
 	contentMap, err := convertContentToMap(sidebarContent)
 	if err != nil {
-		p.API.LogWarn("Failed to convert sidebar content to map", "error", apiErr.Error())
+		p.API.LogWarn("Failed to convert sidebar content to map", "error", err.Error())
 		return
 	}
 

webapp/src/action_types/index.js

@@ -7,6 +7,7 @@ export default {
     RECEIVED_REPOSITORIES: pluginId + '_received_repositories',
     RECEIVED_REVIEWS_DETAILS: pluginId + '_received_reviews_details',
     RECEIVED_YOUR_PRS_DETAILS: pluginId + '_received_your_prs_details',
+    RECEIVED_SIDEBAR_CONTENT: pluginId + '_received_sidebar_content',
     RECEIVED_MENTIONS: pluginId + '_received_mentions',
     RECEIVED_CONNECTED: pluginId + '_received_connected',
     RECEIVED_CONFIGURATION: pluginId + '_received_configuration',
@@ -19,5 +20,4 @@ export default {
     CLOSE_ATTACH_COMMENT_TO_ISSUE_MODAL: pluginId + '_close_attach_modal',
     OPEN_ATTACH_COMMENT_TO_ISSUE_MODAL: pluginId + '_open_attach_modal',
     RECEIVED_ATTACH_COMMENT_RESULT: pluginId + '_received_attach_comment',
-    RECEIVED_SIDEBAR_CONTENT: pluginId + '_received_sidebar_content',
 };

webapp/src/client/client.js

@@ -15,7 +15,9 @@ export default class Client {
         return this.doGet(`${this.url}/connected?reminder=${reminder}`);
     }
 
-    getSidebarContent = async () => this.doGet(`${this.url}/sidebar-content`);
+    getSidebarContent = async () => {
+        return this.doGet(`${this.url}/sidebar-content`);
+    }
 
     getPrsDetails = async (prList) => {
         return this.doPost(`${this.url}/prsdetails`, prList);

webapp/src/components/sidebar_buttons/sidebar_buttons.jsx

@@ -60,9 +60,7 @@ export default class SidebarButtons extends React.PureComponent {
         }
 
         this.setState({refreshing: true});
-        await Promise.all([
-            this.props.actions.getSidebarContent(),
-        ]);
+        await Promise.resolve(this.props.actions.getSidebarContent());
         this.setState({refreshing: false});
     }
 

webapp/src/components/sidebar_right/index.jsx

@@ -5,46 +5,22 @@ import {connect} from 'react-redux';
 import {bindActionCreators} from 'redux';
 
 import {getReviewsDetails, getYourPrsDetails} from '../../actions';
-import {id as pluginId} from '../../manifest';
 
-import SidebarRight from './sidebar_right.jsx';
-
-function mapPrsToDetails(prs, details) {
-    if (!prs) {
-        return [];
-    }
-
-    return prs.map((pr) => {
-        let foundDetails;
-        if (details) {
-            foundDetails = details.find((prDetails) => {
-                return (pr.repository_url === prDetails.url) && (pr.number === prDetails.number);
-            });
-        }
-        if (!foundDetails) {
-            return pr;
-        }
+import {getSidebarData} from 'src/selectors';
 
-        return {
-            ...pr,
-            status: foundDetails.status,
-            mergeable: foundDetails.mergeable,
-            requestedReviewers: foundDetails.requestedReviewers,
-            reviews: foundDetails.reviews,
-        };
-    });
-}
+import SidebarRight from './sidebar_right.jsx';
 
 function mapStateToProps(state) {
+    const {username, reviews, yourPrs, yourAssignments, unreads, enterpriseURL, org, rhsState} = getSidebarData(state);
     return {
-        username: state[`plugins-${pluginId}`].username,
-        reviews: mapPrsToDetails(state[`plugins-${pluginId}`].sidebarContent.reviews, state[`plugins-${pluginId}`].reviewsDetails),
-        yourPrs: mapPrsToDetails(state[`plugins-${pluginId}`].sidebarContent.prs, state[`plugins-${pluginId}`].yourPrsDetails),
-        yourAssignments: state[`plugins-${pluginId}`].sidebarContent.assignments,
-        unreads: state[`plugins-${pluginId}`].sidebarContent.unreads,
-        enterpriseURL: state[`plugins-${pluginId}`].enterpriseURL,
-        org: state[`plugins-${pluginId}`].organization,
-        rhsState: state[`plugins-${pluginId}`].rhsState,
+        username,
+        reviews,
+        yourPrs,
+        yourAssignments,
+        unreads,
+        enterpriseURL,
+        org,
+        rhsState,
     };
 }
 

webapp/src/components/sidebar_right/sidebar_right.jsx

@@ -103,37 +103,38 @@ export default class SidebarRight extends React.PureComponent {
     render() {
         const baseURL = this.props.enterpriseURL ? this.props.enterpriseURL : 'https://github.com';
         const orgQuery = this.props.org ? '+org%3A' + this.props.org : '';
+        const {yourPrs, reviews, unreads, yourAssignments, username, rhsState} = this.props;
 
         let title = '';
         let githubItems = [];
         let listUrl = '';
 
-        switch (this.props.rhsState) {
+        switch (rhsState) {
         case RHSStates.PRS:
 
-            githubItems = this.props.yourPrs || [];
+            githubItems = yourPrs;
             title = 'Your Open Pull Requests';
-            listUrl = baseURL + '/pulls?q=is%3Aopen+is%3Apr+author%3A' + this.props.username + '+archived%3Afalse' + orgQuery;
+            listUrl = baseURL + '/pulls?q=is%3Aopen+is%3Apr+author%3A' + username + '+archived%3Afalse' + orgQuery;
 
             break;
         case RHSStates.REVIEWS:
 
-            githubItems = this.props.reviews || [];
-            listUrl = baseURL + '/pulls?q=is%3Aopen+is%3Apr+review-requested%3A' + this.props.username + '+archived%3Afalse' + orgQuery;
+            githubItems = reviews;
+            listUrl = baseURL + '/pulls?q=is%3Aopen+is%3Apr+review-requested%3A' + username + '+archived%3Afalse' + orgQuery;
             title = 'Pull Requests Needing Review';
 
             break;
         case RHSStates.UNREADS:
 
-            githubItems = this.props.unreads || [];
+            githubItems = unreads;
             title = 'Unread Messages';
             listUrl = baseURL + '/notifications';
             break;
         case RHSStates.ASSIGNMENTS:
 
-            githubItems = this.props.yourAssignments || [];
+            githubItems = yourAssignments;
             title = 'Your Assignments';
-            listUrl = baseURL + '/pulls?q=is%3Aopen+archived%3Afalse+assignee%3A' + this.props.username + orgQuery;
+            listUrl = baseURL + '/pulls?q=is%3Aopen+archived%3Afalse+assignee%3A' + username + orgQuery;
             break;
         default:
             break;

webapp/src/constants/index.js

@@ -11,8 +11,3 @@ export const RHSStates = {
     UNREADS: 'unreads',
     ASSIGNMENTS: 'assignments',
 };
-
-export const JitterForReconnectAPICall = {
-    MAX_TIME_IN_SEC: 1,
-    MIN_TIME_IN_SEC: 10,
-};

webapp/src/reducers/index.js

@@ -6,6 +6,13 @@ import {combineReducers} from 'redux';
 import ActionTypes from '../action_types';
 import Constants from '../constants';
 
+const defaultSidebarContent = {
+    reviews: [],
+    prs: [],
+    assignments: [],
+    unreads: [],
+};
+
 function connected(state = false, action) {
     switch (action.type) {
     case ActionTypes.RECEIVED_CONNECTED:
@@ -86,7 +93,7 @@ function reviewsDetails(state = [], action) {
     }
 }
 
-function sidebarContent(state = [], action) {
+function sidebarContent(state = defaultSidebarContent, action) {
     switch (action.type) {
     case ActionTypes.RECEIVED_SIDEBAR_CONTENT:
         return action.data;

webapp/src/selectors.js

@@ -1,7 +1,11 @@
 import {getConfig} from 'mattermost-redux/selectors/entities/general';
 
+import {createSelector} from 'reselect';
+
 import {id as pluginId} from './manifest';
 
+const emptyArray = [];
+
 const getPluginState = (state) => state['plugins-' + pluginId] || {};
 
 export const isEnabled = (state) => getPluginState(state).enabled;
@@ -19,4 +23,46 @@ export const getServerRoute = (state) => {
     return basePath;
 };
 
+function mapPrsToDetails(prs, details) {
+    if (!prs) {
+        return [];
+    }
+
+    return prs.map((pr) => {
+        let foundDetails;
+        if (details) {
+            foundDetails = details.find((prDetails) => {
+                return (pr.repository_url === prDetails.url) && (pr.number === prDetails.number);
+            });
+        }
+        if (!foundDetails) {
+            return pr;
+        }
+
+        return {
+            ...pr,
+            status: foundDetails.status,
+            mergeable: foundDetails.mergeable,
+            requestedReviewers: foundDetails.requestedReviewers,
+            reviews: foundDetails.reviews,
+        };
+    });
+}
+
+export const getSidebarData = createSelector(
+    getPluginState,
+    (pluginState) => {
+        const {username, sidebarContent, reviewDetails, yourPrDetails, organization, rhsState} = pluginState;
+        return {
+            username,
+            reviews: mapPrsToDetails(sidebarContent.reviews || emptyArray, reviewDetails),
+            yourPrs: mapPrsToDetails(sidebarContent.prs || emptyArray, yourPrDetails),
+            yourAssignments: sidebarContent.assignments || emptyArray,
+            unreads: sidebarContent.unreads || emptyArray,
+            org: organization,
+            rhsState,
+        };
+    },
+);
+
 export const configuration = (state) => getPluginState(state).configuration;

webapp/src/websocket/index.js

@@ -2,7 +2,7 @@
 // See LICENSE.txt for license information.
 
 import ActionTypes from '../action_types';
-import Constants, {JitterForReconnectAPICall} from '../constants';
+import Constants from '../constants';
 import {
     getConnected,
     getSidebarContent,
@@ -12,6 +12,7 @@ import {
 import {id as pluginId} from '../manifest';
 
 let timeoutId;
+const RECONNECT_JITTER_MAX_TIME = 10;
 export function handleConnect(store) {
     return (msg) => {
         if (!msg.data) {
@@ -68,7 +69,7 @@ export function handleReconnect(store, reminder = false) {
                 clearTimeout(timeoutId);
             }
 
-            const rand = Math.floor(Math.random() * (JitterForReconnectAPICall.MAX_TIME_IN_SEC - JitterForReconnectAPICall.MIN_TIME_IN_SEC + 1)) + JitterForReconnectAPICall.MIN_TIME_IN_SEC; //eslint-disable-line no-mixed-operators
+            const rand = Math.floor(Math.random() * RECONNECT_JITTER_MAX_TIME) + 1;
             timeoutId = setTimeout(() => {
                 getSidebarContent()(store.dispatch, store.getState);
                 timeoutId = undefined; //eslint-disable-line no-undefined