[PROD RELEASE] - Bug fixes #1648
Conversation
fix(PM-1137): search broken in projects list
fix(PM-1140): show spinner when changing between projects
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -251,7 +253,7 @@ class Users extends Component { | |||
| ) | |||
| } | |||
| { | |||
| membersExist && ( | |||
| !isLoadingProject && membersExist && ( | |||
There was a problem hiding this comment.
Consider checking the isLoadingProject condition earlier in the logic to potentially avoid unnecessary checks or rendering. This might improve readability and performance if isLoadingProject is a common state.
| @@ -305,6 +307,8 @@ class Users extends Component { | |||
| ) | |||
| } | |||
|
|
|||
| {isLoadingProject && <Loader />} | |||
There was a problem hiding this comment.
Consider adding a conditional rendering check to ensure that the Loader component is only displayed when necessary. This will prevent unnecessary rendering and improve performance.
| @@ -322,6 +326,7 @@ Users.propTypes = { | |||
| projects: PropTypes.arrayOf(PropTypes.object), | |||
| projectMembers: PropTypes.arrayOf(PropTypes.object), | |||
| invitedMembers: PropTypes.arrayOf(PropTypes.object), | |||
| isLoadingProject: PropTypes.bool.isRequired, | |||
There was a problem hiding this comment.
The addition of isLoadingProject as a required prop is clear, but ensure that this prop is being used correctly within the component to handle loading states. Double-check if there are any related changes needed in the component logic to accommodate this new prop.
src/containers/Users/index.js
Outdated
| @@ -21,7 +21,8 @@ class Users extends Component { | |||
| loginUserRoleInProject: '', | |||
| projectMembers: null, | |||
| invitedMembers: null, | |||
| isAdmin: false | |||
| isAdmin: false, | |||
| isLoadingProject: false | |||
| } | |||
| this.loadProject = this.loadProject.bind(this) | |||
| this.updateProjectNember = this.updateProjectNember.bind(this) | |||
There was a problem hiding this comment.
Typo in method name updateProjectNember. It should likely be updateProjectMember.
There was a problem hiding this comment.
@hentrymartin can you please commit a fix for this in develop?
There was a problem hiding this comment.
@kkartunov I've added a fix for this!
| @@ -80,9 +81,10 @@ class Users extends Component { | |||
| } | |||
|
|
|||
| loadProject (projectId) { | |||
| this.setState({ isLoadingProject: true }) | |||
There was a problem hiding this comment.
Consider handling potential errors from fetchProjectById to ensure isLoadingProject state is properly reset in case of failure.
| @@ -91,7 +93,8 @@ class Users extends Component { | |||
| invitedMembers: invitedMembers.map(m => ({ | |||
| ...m, | |||
| email: m.email || invitedUsers[m.userId].handle | |||
| })) | |||
| })), | |||
| isLoadingProject: false | |||
There was a problem hiding this comment.
The addition of isLoadingProject: false seems to be outside the scope of the invitedMembers mapping. Ensure that this property is intended to be part of the state update and not mistakenly included within the invitedMembers transformation.
| @@ -42,11 +42,11 @@ class UserCard extends Component { | |||
| isUpdatingPermission: true | |||
| }) | |||
|
|
|||
| const { user, updateProjectNember } = this.props | |||
| const { user, updateProjectMember } = this.props | |||
There was a problem hiding this comment.
Typo fix: The variable updateProjectNember was corrected to updateProjectMember. Ensure that all references to this function are updated throughout the codebase to prevent any potential errors.
src/components/Users/index.js
Outdated
| </div> | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| Users.propTypes = { | ||
| loadProject: PropTypes.func.isRequired, | ||
| updateProjectNember: PropTypes.func.isRequired, | ||
| updateProjectMember: PropTypes.func.isRequired, | ||
| removeProjectNember: PropTypes.func.isRequired, |
There was a problem hiding this comment.
Typo correction needed: removeProjectNember should be removeProjectMember to match the corrected spelling of updateProjectMember.
src/containers/Users/index.js
Outdated
| } | ||
| this.loadProject = this.loadProject.bind(this) | ||
| this.updateProjectNember = this.updateProjectNember.bind(this) | ||
| this.updateProjectMember = this.updateProjectMember.bind(this) | ||
| this.removeProjectNember = this.removeProjectNember.bind(this) |
There was a problem hiding this comment.
The method removeProjectNember seems to have a typo in its name. Consider renaming it to removeProjectMember to maintain consistency with the corrected method name updateProjectMember.
| }) | ||
| const { loggedInUser } = this.props | ||
| this.updateLoginUserRoleInProject(projectMembers, loggedInUser) | ||
| }) | ||
| } | ||
|
|
||
| updateProjectNember (newMemberInfo) { | ||
| updateProjectMember (newMemberInfo) { |
There was a problem hiding this comment.
The function name updateProjectMember has been corrected from updateProjectNember. Ensure that all references to this function throughout the codebase are updated to prevent any potential errors.
src/containers/Users/index.js
Outdated
| } = this.state | ||
| return ( | ||
| <UsersComponent | ||
| projects={projects} | ||
| loadProject={this.loadProject} | ||
| updateProjectNember={this.updateProjectNember} | ||
| updateProjectMember={this.updateProjectMember} | ||
| removeProjectNember={this.removeProjectNember} |
There was a problem hiding this comment.
The function name removeProjectNember seems to have a typo similar to the one corrected for updateProjectMember. Consider renaming it to removeProjectMember for consistency and clarity.
| @@ -109,15 +110,15 @@ class Users extends Component { | |||
| async onRemoveConfirmClick () { | |||
| if (this.state.isRemoving) { return } | |||
|
|
|||
| const { removeProjectNember, invitedMembers } = this.props | |||
| const { removeProjectMember, invitedMembers } = this.props | |||
There was a problem hiding this comment.
Typo fix: The variable removeProjectNember was corrected to removeProjectMember. Ensure that this change is consistent throughout the codebase where this variable is used.
| @@ -111,7 +114,7 @@ class Users extends Component { | |||
| this.updateLoginUserRoleInProject(newProjectMembers, loggedInUser) | |||
| } | |||
|
|
|||
| removeProjectNember (projectMember) { | |||
| removeProjectMember (projectMember) { | |||
There was a problem hiding this comment.
Typo fix: The method name was corrected from removeProjectNember to removeProjectMember. Ensure that all references to this method throughout the codebase are updated accordingly to prevent any potential errors.
https://topcoder.atlassian.net/browse/PM-1137
https://topcoder.atlassian.net/browse/PM-1140