Skip to content

[PROD RELEASE] - Bug fixes #1648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 13, 2025
Merged

[PROD RELEASE] - Bug fixes #1648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
879c08c
fix: search broken in projects
hentrymartin Apr 24, 2025
0ebe42d
removed console log
hentrymartin Apr 24, 2025
3cf1900
fix: show spinner when changing between projects
hentrymartin Apr 25, 2025
0493017
Merge pull request #1646 from topcoder-platform/pm-1137
kkartunov Apr 28, 2025
d673228
Merge pull request #1647 from topcoder-platform/pm-1140
kkartunov Apr 28, 2025
f089527
fix: typo for updateProjectMember
hentrymartin May 12, 2025
1fc4c52
fix: typo
hentrymartin May 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/components/UserCard/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ class UserCard extends Component {
isUpdatingPermission: true
})

const { user, updateProjectNember } = this.props
const { user, updateProjectMember } = this.props
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


try {
const newUserInfoRole = await updateProjectMemberRole(user.projectId, user.id, newRole)
updateProjectNember(newUserInfoRole)
updateProjectMember(newUserInfoRole)
this.setState({ showSuccessModal: true })
} catch (e) {
const error = _.get(
Expand Down Expand Up @@ -190,7 +190,7 @@ class UserCard extends Component {
UserCard.propTypes = {
isInvite: PropTypes.bool,
user: PropTypes.object,
updateProjectNember: PropTypes.func.isRequired,
updateProjectMember: PropTypes.func.isRequired,
onRemoveClick: PropTypes.func.isRequired,
isEditable: PropTypes.bool
}
Expand Down
23 changes: 14 additions & 9 deletions src/components/Users/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { deleteProjectMemberInvite } from '../../services/projectMemberInvites'
import ConfirmationModal from '../Modal/ConfirmationModal'
import UserAddModalContent from './user-add.modal'
import InviteUserModalContent from './invite-user.modal' // Import the new component
import Loader from '../Loader'

const theme = {
container: styles.modalContainer
Expand Down Expand Up @@ -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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo fix: The variable removeProjectNember was corrected to removeProjectMember. Ensure that this change is consistent throughout the codebase where this variable is used.

const userToRemove = this.state.userToRemove
const isInvite = !!_.find(invitedMembers, { email: userToRemove.email })
try {
this.setState({ isRemoving: true })
await (
isInvite ? deleteProjectMemberInvite(userToRemove.projectId, userToRemove.id) : removeUserFromProject(userToRemove.projectId, userToRemove.id)
)
removeProjectNember(userToRemove)
removeProjectMember(userToRemove)

this.resetRemoveUserState()
} catch (e) {
Expand Down Expand Up @@ -156,11 +157,12 @@ class Users extends Component {
projects,
projectMembers,
invitedMembers,
updateProjectNember,
updateProjectMember,
isEditable,
isSearchingUserProjects,
resultSearchUserProjects,
loadNextProjects
loadNextProjects,
isLoadingProject
} = this.props
const {
searchKey
Expand Down Expand Up @@ -251,7 +253,7 @@ class Users extends Component {
)
}
{
membersExist && (
!isLoadingProject && membersExist && (
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

<>
<div className={styles.header}>
<div className={cn(styles.col5)}>
Expand All @@ -278,7 +280,7 @@ class Users extends Component {
<UserCard
user={member}
onRemoveClick={this.onRemoveClick}
updateProjectNember={updateProjectNember}
updateProjectMember={updateProjectMember}
isEditable={isEditable} />
</li>
)
Expand All @@ -294,7 +296,7 @@ class Users extends Component {
isInvite
user={member}
onRemoveClick={this.onRemoveClick}
updateProjectNember={updateProjectNember}
updateProjectMember={updateProjectMember}
isEditable={isEditable} />
</li>
)
Expand All @@ -305,15 +307,17 @@ class Users extends Component {
)
}

{isLoadingProject && <Loader />}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


</div>
)
}
}

Users.propTypes = {
loadProject: PropTypes.func.isRequired,
updateProjectNember: PropTypes.func.isRequired,
removeProjectNember: PropTypes.func.isRequired,
updateProjectMember: PropTypes.func.isRequired,
removeProjectMember: PropTypes.func.isRequired,
addNewProjectInvite: PropTypes.func.isRequired,
addNewProjectMember: PropTypes.func.isRequired,
auth: PropTypes.object,
Expand All @@ -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,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

searchUserProjects: PropTypes.func.isRequired,
resultSearchUserProjects: PropTypes.arrayOf(PropTypes.object),
loadNextProjects: PropTypes.func.isRequired
Expand Down
1 change: 1 addition & 0 deletions src/config/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ export const SPECIAL_CHALLENGE_TAGS = [
* Possible statuses of projects
*/
export const PROJECT_STATUSES = [
{ label: 'Draft', value: 'draft' },
{ label: 'Active', value: 'active' },
{ label: 'In Review', value: 'in_review' },
{ label: 'Reviewed', value: 'reviewed' },
Expand Down
25 changes: 15 additions & 10 deletions src/containers/Users/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ 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)
this.removeProjectNember = this.removeProjectNember.bind(this)
this.updateProjectMember = this.updateProjectMember.bind(this)
this.removeProjectMember = this.removeProjectMember.bind(this)
this.addNewProjectInvite = this.addNewProjectInvite.bind(this)
this.addNewProjectMember = this.addNewProjectMember.bind(this)
this.loadNextProjects = this.loadNextProjects.bind(this)
Expand Down Expand Up @@ -80,9 +81,10 @@ class Users extends Component {
}

loadProject (projectId) {
this.setState({ isLoadingProject: true })
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider handling potential errors from fetchProjectById to ensure isLoadingProject state is properly reset in case of failure.

fetchProjectById(projectId).then(async (project) => {
const projectMembers = _.get(project, 'members')
const invitedMembers = _.get(project, 'invites')
const invitedMembers = _.get(project, 'invites') || []
const invitedUserIds = _.filter(_.map(invitedMembers, 'userId'))
const invitedUsers = await fetchInviteMembers(invitedUserIds)

Expand All @@ -91,14 +93,15 @@ class Users extends Component {
invitedMembers: invitedMembers.map(m => ({
...m,
email: m.email || invitedUsers[m.userId].handle
}))
})),
isLoadingProject: false
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

})
const { loggedInUser } = this.props
this.updateLoginUserRoleInProject(projectMembers, loggedInUser)
})
}

updateProjectNember (newMemberInfo) {
updateProjectMember (newMemberInfo) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const { projectMembers } = this.state
const newProjectMembers = projectMembers.map(pm => pm.id === newMemberInfo.id ? ({
...pm,
Expand All @@ -111,7 +114,7 @@ class Users extends Component {
this.updateLoginUserRoleInProject(newProjectMembers, loggedInUser)
}

removeProjectNember (projectMember) {
removeProjectMember (projectMember) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const { projectMembers, invitedMembers } = this.state
const newProjectMembers = _.filter(projectMembers, pm => pm.id !== projectMember.id)
const newInvitedMembers = _.filter(invitedMembers, pm => pm.id !== projectMember.id)
Expand Down Expand Up @@ -156,19 +159,21 @@ class Users extends Component {
const {
projectMembers,
invitedMembers,
isAdmin
isAdmin,
isLoadingProject
} = this.state
return (
<UsersComponent
projects={projects}
loadProject={this.loadProject}
updateProjectNember={this.updateProjectNember}
removeProjectNember={this.removeProjectNember}
updateProjectMember={this.updateProjectMember}
removeProjectMember={this.removeProjectMember}
addNewProjectMember={this.addNewProjectMember}
addNewProjectInvite={this.addNewProjectInvite}
loadNextProjects={this.loadNextProjects}
projectMembers={projectMembers}
invitedMembers={invitedMembers}
isLoadingProject={isLoadingProject}
auth={auth}
isAdmin={isAdmin}
isEditable={this.isEditable()}
Expand Down