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 5 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
9 changes: 7 additions & 2 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 @@ -160,7 +161,8 @@ class Users extends Component {
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

@github-actions github-actions bot May 12, 2025

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 Down Expand Up @@ -305,6 +307,8 @@ class Users extends Component {
)
}

{isLoadingProject && <Loader />}
Copy link

@github-actions github-actions bot May 12, 2025

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>
)
}
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

@github-actions github-actions bot May 12, 2025

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
13 changes: 9 additions & 4 deletions src/containers/Users/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link

@github-actions github-actions bot May 12, 2025

Choose a reason for hiding this comment

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

Typo in method name updateProjectNember. It should likely be updateProjectMember.

Copy link
Contributor Author

@kkartunov kkartunov May 12, 2025

Choose a reason for hiding this comment

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

@hentrymartin can you please commit a fix for this in develop?

Copy link
Collaborator

@hentrymartin hentrymartin May 12, 2025

Choose a reason for hiding this comment

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

@kkartunov I've added a fix for this!

Expand Down Expand Up @@ -80,9 +81,10 @@ class Users extends Component {
}

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

@github-actions github-actions bot May 12, 2025

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,7 +93,8 @@ class Users extends Component {
invitedMembers: invitedMembers.map(m => ({
...m,
email: m.email || invitedUsers[m.userId].handle
}))
})),
isLoadingProject: false
Copy link

@github-actions github-actions bot May 12, 2025

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)
Expand Down Expand Up @@ -156,7 +159,8 @@ class Users extends Component {
const {
projectMembers,
invitedMembers,
isAdmin
isAdmin,
isLoadingProject
} = this.state
return (
<UsersComponent
Expand All @@ -169,6 +173,7 @@ class Users extends Component {
loadNextProjects={this.loadNextProjects}
projectMembers={projectMembers}
invitedMembers={invitedMembers}
isLoadingProject={isLoadingProject}
auth={auth}
isAdmin={isAdmin}
isEditable={this.isEditable()}
Expand Down