fix: new review

mattermost-community · Nov 9, 2023 · 99b959c · 99b959c
1 parent 2d27842
commit 99b959c
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 39 deletions.
diff --git a/server/plugin.go b/server/plugin.go
@@ -22,6 +22,8 @@ const (
 
 	// WSEventConfigUpdate is the WebSocket event to update the Todo list's configurations on webapp
 	WSEventConfigUpdate = "config_update"
+
+	ErrorMsgAddIssue = "Unable to add issue"
 )
 
 // ListManager represents the logic on the lists
@@ -207,9 +209,8 @@ func (p *Plugin) handleAdd(w http.ResponseWriter, r *http.Request) {
 	if addRequest.SendTo == "" {
 		_, err = p.listManager.AddIssue(userID, addRequest.Message, addRequest.Description, addRequest.PostID)
 		if err != nil {
-			msg := "Unable to add issue"
-			p.API.LogError(msg, "err", err.Error())
-			p.handleErrorWithCode(w, http.StatusInternalServerError, msg, err)
+			p.API.LogError(ErrorMsgAddIssue, "err", err.Error())
+			p.handleErrorWithCode(w, http.StatusInternalServerError, ErrorMsgAddIssue, err)
 			return
 		}
 
@@ -226,17 +227,16 @@ func (p *Plugin) handleAdd(w http.ResponseWriter, r *http.Request) {
 	receiver, appErr := p.API.GetUserByUsername(addRequest.SendTo)
 	if appErr != nil {
 		msg := "Unable to find user"
-		p.API.LogError(msg, "err", err.Error())
-		p.handleErrorWithCode(w, http.StatusInternalServerError, msg, err)
+		p.API.LogError(msg, "err", appErr.Error())
+		p.handleErrorWithCode(w, http.StatusInternalServerError, msg, appErr)
 		return
 	}
 
 	if receiver.Id == userID {
 		_, err = p.listManager.AddIssue(userID, addRequest.Message, addRequest.Description, addRequest.PostID)
 		if err != nil {
-			msg := "Unable to add issue"
-			p.API.LogError(msg, "err", err.Error())
-			p.handleErrorWithCode(w, http.StatusInternalServerError, msg, err)
+			p.API.LogError(ErrorMsgAddIssue, "err", err.Error())
+			p.handleErrorWithCode(w, http.StatusInternalServerError, ErrorMsgAddIssue, err)
 			return
 		}
 
@@ -403,8 +403,8 @@ func (p *Plugin) handleChangeAssignment(w http.ResponseWriter, r *http.Request)
 	receiver, appErr := p.API.GetUserByUsername(changeRequest.SendTo)
 	if appErr != nil {
 		msg := "username not valid"
-		p.API.LogError(msg, "err", err.Error())
-		p.handleErrorWithCode(w, http.StatusNotFound, msg, err)
+		p.API.LogError(msg, "err", appErr.Error())
+		p.handleErrorWithCode(w, http.StatusNotFound, msg, appErr)
 		return
 	}
 

diff --git a/webapp/src/components/sidebar_buttons/index.js b/webapp/src/components/sidebar_buttons/index.js
@@ -6,13 +6,13 @@ import {bindActionCreators} from 'redux';
 
 import {fetchAllIssueLists, updateRhsState, telemetry} from '../../actions';
 
-import {getIssues, getInIssues, getOutIssues} from '../../selectors';
+import {getMyIssues, getInIssues, getOutIssues} from '../../selectors';
 
 import SidebarButtons from './sidebar_buttons.jsx';
 
 function mapStateToProps(state) {
     return {
-        myIssues: getIssues(state),
+        myIssues: getMyIssues(state),
         inIssues: getInIssues(state),
         outIssues: getOutIssues(state),
         showRHSPlugin: state['plugins-com.mattermost.plugin-todo'].rhsPluginAction,

diff --git a/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx b/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx
@@ -13,9 +13,9 @@ export default class SidebarButtons extends React.PureComponent {
         theme: PropTypes.object.isRequired,
         isTeamSidebar: PropTypes.bool,
         showRHSPlugin: PropTypes.func.isRequired,
-        myIssues: PropTypes.array,
-        inIssues: PropTypes.array,
-        outIssues: PropTypes.array,
+        myIssues: PropTypes.array.isRequired,
+        inIssues: PropTypes.array.isRequired,
+        outIssues: PropTypes.array.isRequired,
         actions: PropTypes.shape({
             updateRhsState: PropTypes.func.isRequired,
             telemetry: PropTypes.func.isRequired,
@@ -67,7 +67,7 @@ export default class SidebarButtons extends React.PureComponent {
                         }}
                     >
                         <i className='icon icon-check'/>
-                        {' ' + myIssues ? myIssues.length : 0}
+                        {' ' + myIssues.length }
                     </a>
                 </OverlayTrigger>
                 <OverlayTrigger
@@ -83,7 +83,7 @@ export default class SidebarButtons extends React.PureComponent {
                         style={button}
                     >
                         <i className='icon icon-arrow-down'/>
-                        {' ' + inIssues ? inIssues.length : 0}
+                        {' ' + inIssues.length }
                     </a>
                 </OverlayTrigger>
                 <OverlayTrigger
@@ -99,7 +99,7 @@ export default class SidebarButtons extends React.PureComponent {
                         style={button}
                     >
                         <i className='icon icon-arrow-up'/>
-                        {' ' + outIssues ? outIssues.length : 0}
+                        {' ' + outIssues.length }
                     </a>
                 </OverlayTrigger>
             </div>

diff --git a/webapp/src/components/sidebar_right/index.js b/webapp/src/components/sidebar_right/index.js
@@ -4,14 +4,14 @@
 import {connect} from 'react-redux';
 import {bindActionCreators} from 'redux';
 
-import {getSiteURL, getTodoToast, getIssues, getInIssues, getOutIssues} from '../../selectors';
+import {getSiteURL, getTodoToast, getMyIssues, getInIssues, getOutIssues} from '../../selectors';
 import {remove, fetchAllIssueLists, openAssigneeModal, openAddCard, closeAddCard, complete, bump, accept, telemetry, setRhsVisible} from '../../actions';
 
 import SidebarRight from './sidebar_right.jsx';
 
 function mapStateToProps(state) {
     return {
-        myIssues: getIssues(state),
+        myIssues: getMyIssues(state),
         inIssues: getInIssues(state),
         outIssues: getOutIssues(state),
         todoToast: getTodoToast(state),

diff --git a/webapp/src/components/sidebar_right/sidebar_right.jsx b/webapp/src/components/sidebar_right/sidebar_right.jsx
@@ -51,9 +51,9 @@ const InListName = 'in';
 
 export default class SidebarRight extends React.PureComponent {
     static propTypes = {
-        myIssues: PropTypes.array,
-        inIssues: PropTypes.array,
-        outIssues: PropTypes.array,
+        myIssues: PropTypes.array.isRequired,
+        inIssues: PropTypes.array.isRequired,
+        outIssues: PropTypes.array.isRequired,
         todoToast: PropTypes.object,
         theme: PropTypes.object.isRequired,
         siteURL: PropTypes.string.isRequired,
@@ -123,18 +123,6 @@ export default class SidebarRight extends React.PureComponent {
         }
     }
 
-    getInIssues() {
-        return this.props.inIssues.length;
-    }
-
-    getOutIssues() {
-        return this.props.outIssues.length;
-    }
-
-    getMyIssues() {
-        return this.props.myIssues.length;
-    }
-
     addTodoItem() {
         this.props.actions.openAddCard('');
     }
@@ -152,12 +140,12 @@ export default class SidebarRight extends React.PureComponent {
 
         switch (this.state.list) {
         case MyListName:
-            todos = this.props.myIssues || [];
+            todos = this.props.myIssues;
             addButton = 'Add Todo';
-            inboxList = this.props.inIssues || [];
+            inboxList = this.props.inIssues;
             break;
         case OutListName:
-            todos = this.props.outIssues || [];
+            todos = this.props.outIssues;
             listHeading = 'Sent Todos';
             addButton = 'Request a Todo from someone';
             break;

diff --git a/webapp/src/selectors.js b/webapp/src/selectors.js
@@ -24,7 +24,7 @@ export const getMessage = (state) => {
     }
     return post.message;
 };
-export const getIssues = (state) => getAllIssues(state).my;
+export const getMyIssues = (state) => getAllIssues(state).my;
 export const getInIssues = (state) => getAllIssues(state).in;
 export const getOutIssues = (state) => getAllIssues(state).out;
 export const getAllIssues = (state) => getPluginState(state).allIssues;