Skip to content

feat(PM-1506): Overwrite role if the copilot is already a member of the project with "read"/"write" access #1658

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 3 commits into from
Jul 29, 2025
Merged

feat(PM-1506): Overwrite role if the copilot is already a member of the project with "read"/"write" access #1658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e551d12
feat: modifications on copilot addition to project
hentrymartin Jul 25, 2025
537a94b
fix: action string
hentrymartin Jul 28, 2025
59cb7d7
fix: action string
hentrymartin Jul 28, 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
11 changes: 6 additions & 5 deletions src/components/Users/Users.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@
}
}

.textContent {
font-size: 18px;
margin-top: 25px;
}


.row {
display: flex;
Expand Down Expand Up @@ -400,7 +405,7 @@
justify-content: center;
}

.addButtonContainer {
.addButtonContainer, .buttonWrapper {
display: flex;
justify-content: flex-start;
height: 30px;
Expand All @@ -412,10 +417,6 @@
}
}

.addUserContentContainer {

}

.tcRadioButton {
.tc-radioButton-label {
@include roboto-light();
Expand Down
3 changes: 3 additions & 0 deletions src/components/Users/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ class Users extends Component {
addNewProjectMember={this.props.addNewProjectMember}
onMemberInvited={this.props.addNewProjectInvite}
onClose={this.resetAddUserState}
projectOption={this.state.projectOption}
Copy link

Choose a reason for hiding this comment

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

Consider checking if projectOption is correctly initialized and used within the component. Ensure that it aligns with the intended functionality described in the pull request.

projectMembers={projectMembers}
Copy link

Choose a reason for hiding this comment

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

Verify that projectMembers is correctly populated and used within the component. Ensure it reflects the current state of project members accurately.

updateProjectMember={updateProjectMember}
Copy link

Choose a reason for hiding this comment

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

Ensure that updateProjectMember is properly defined and handles the logic for updating project members as intended. Consider edge cases where the update might fail or need additional validation.

/>
)
}
Expand Down
239 changes: 142 additions & 97 deletions src/components/Users/user-add.modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,30 @@ import Modal from '../Modal'
import SelectUserAutocomplete from '../SelectUserAutocomplete'
import { PROJECT_ROLES } from '../../config/constants'
import PrimaryButton from '../Buttons/PrimaryButton'
import { addUserToProject, inviteUserToProject } from '../../services/projects'
import { addUserToProject, inviteUserToProject, updateProjectMemberRole } from '../../services/projects'

import styles from './Users.module.scss'

const theme = {
container: styles.modalContainer
}

const UserAddModalContent = ({ projectId, addNewProjectMember, onMemberInvited, onClose }) => {
const UserAddModalContent = ({
projectMembers,
projectOption,
Copy link

Choose a reason for hiding this comment

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

The projectOption parameter is added but not used in the function. Consider removing it if it's unnecessary.

projectId,
addNewProjectMember,
onMemberInvited,
onClose,
updateProjectMember
}) => {
const [userToAdd, setUserToAdd] = useState(null)
const [userPermissionToAdd, setUserPermissionToAdd] = useState(PROJECT_ROLES.READ)
const [showSelectUserError, setShowSelectUserError] = useState(false)
const [addUserError, setAddUserError] = useState(null)
const [isAdding, setIsAdding] = useState(false)
const [isUserAddingFailed, setUserAddingFailed] = useState(false)
Copy link

Choose a reason for hiding this comment

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

The variable isUserAddingFailed is introduced but not used in the code. Ensure it is utilized or remove it if not needed.

const [existingRole, setExistingRole] = useState('')
Copy link

Choose a reason for hiding this comment

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

The variable existingRole is introduced but not used in the code. Ensure it is utilized or remove it if not needed.


const onUpdateUserToAdd = (option) => {
if (option && option.value) {
Expand Down Expand Up @@ -52,8 +62,12 @@ const UserAddModalContent = ({ projectId, addNewProjectMember, onMemberInvited,
})
if (failed) {
const error = get(failed, '0.message', 'User cannot be invited')
const errorCode = get(failed, '0.error')
Copy link

Choose a reason for hiding this comment

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

Consider checking if failed is not null or undefined before attempting to access its properties to avoid potential runtime errors.

const role = get(failed, '0.role')
Copy link

Choose a reason for hiding this comment

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

Consider checking if failed is not null or undefined before attempting to access its properties to avoid potential runtime errors.

setAddUserError(error)
setIsAdding(false)
setUserAddingFailed(errorCode === 'ALREADY_MEMBER')
Copy link

Choose a reason for hiding this comment

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

Consider using a constant or enum for the error code 'ALREADY_MEMBER' to avoid magic strings and improve maintainability.

setExistingRole(role)
} else if (rest.message) {
setAddUserError(rest.message)
setIsAdding(false)
Expand All @@ -74,121 +88,152 @@ const UserAddModalContent = ({ projectId, addNewProjectMember, onMemberInvited,
}
}

const onConfirmCopilotRoleChange = async () => {
const member = projectMembers.find(item => item.userId === userToAdd.userId)
const action = member.role === 'manager' ? 'complete-copilot-requests' : ''
Copy link

Choose a reason for hiding this comment

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

Consider checking if member is undefined before accessing its properties. This will prevent potential runtime errors if projectMembers.find does not find a matching member.

const response = await updateProjectMemberRole(projectId, member.id, 'copilot', action)
Copy link

Choose a reason for hiding this comment

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

The action variable is set to an empty string if the member's role is not 'manager'. Consider handling other roles explicitly or documenting why an empty string is appropriate for roles other than 'manager'.

updateProjectMember(response)
onClose()
}

const onCancelCopilotRoleChange = () => {
setUserAddingFailed(false)
setAddUserError('')
}

return (
<Modal theme={theme} onCancel={onClose}>
<div className={cn(styles.contentContainer, styles.confirm)}>
<div className={styles.title}>Add User</div>
<div className={styles.addUserContentContainer}>
<div className={styles.row}>
<div className={cn(styles.field, styles.col1, styles.addUserTitle)}>
Member<span className={styles.required}>*</span> :
</div>
<div className={cn(styles.field, styles.col2)}>
<SelectUserAutocomplete
value={userToAdd ? { label: userToAdd.handle, value: userToAdd.userId.toString() } : null}
onChange={onUpdateUserToAdd}
/>
{
isUserAddingFailed && (['observer', 'customer', 'copilot', 'manager'].includes(existingRole)) && (
Copy link

Choose a reason for hiding this comment

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

The condition (['observer', 'customer', 'copilot', 'manager'].includes(existingRole)) is repeated multiple times. Consider defining a constant array for these roles to improve readability and maintainability.

<div className={cn(styles.contentContainer, styles.confirm)}>
<div className={styles.textContent}>{`The copilot ${userToAdd.handle} is part of ${projectOption.label} project with ${existingRole} role.`}</div>
<div className={styles.buttonWrapper}>
<PrimaryButton onClick={onConfirmCopilotRoleChange} text={'Confirm'} type={'info'} />
<PrimaryButton onClick={onCancelCopilotRoleChange} text={'Cancel'} type={'disabled'} />
</div>
</div>
{showSelectUserError && (
<div className={styles.row}>
<div className={styles.errorMesssage}>Please select a member.</div>
</div>
)}
<div className={styles.row}>
<div className={cn(styles.field, styles.col1, styles.addUserTitle)}>
<label htmlFor='memberToAdd'>Role :</label>
</div>
<div className={cn(styles.col5)}>
<div className={styles.tcRadioButton}>
<input
name={`add-user-radio`}
type='radio'
id={`read-add-user`}
checked={userPermissionToAdd === PROJECT_ROLES.READ}
onChange={() => setUserPermissionToAdd(PROJECT_ROLES.READ)}
/>
<label htmlFor={`read-add-user`}>
<div>Read</div>
<input type='hidden' />
</label>
)
}
{
!isUserAddingFailed && (
<div className={cn(styles.contentContainer, styles.confirm)}>
<div className={styles.title}>Add User</div>
<div className={styles.addUserContentContainer}>
<div className={styles.row}>
<div className={cn(styles.field, styles.col1, styles.addUserTitle)}>
Member<span className={styles.required}>*</span> :
</div>
<div className={cn(styles.field, styles.col2)}>
<SelectUserAutocomplete
value={userToAdd ? { label: userToAdd.handle, value: userToAdd.userId.toString() } : null}
onChange={onUpdateUserToAdd}
/>
</div>
</div>
</div>
<div className={cn(styles.col5)}>
<div className={styles.tcRadioButton}>
<input
name={`add-user-radio`}
type='radio'
id={`write-add-user`}
checked={userPermissionToAdd === PROJECT_ROLES.WRITE}
onChange={() => setUserPermissionToAdd(PROJECT_ROLES.WRITE)}
/>
<label htmlFor={`write-add-user`}>
<div>Write</div>
<input type='hidden' />
</label>
{showSelectUserError && (
<div className={styles.row}>
<div className={styles.errorMesssage}>Please select a member.</div>
Copy link

Choose a reason for hiding this comment

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

There is a typo in the class name styles.errorMesssage. It should be styles.errorMessage.

</div>
)}
<div className={styles.row}>
<div className={cn(styles.field, styles.col1, styles.addUserTitle)}>
<label htmlFor='memberToAdd'>Role :</label>
</div>
<div className={cn(styles.col5)}>
<div className={styles.tcRadioButton}>
<input
name={`add-user-radio`}
type='radio'
id={`read-add-user`}
checked={userPermissionToAdd === PROJECT_ROLES.READ}
onChange={() => setUserPermissionToAdd(PROJECT_ROLES.READ)}
/>
<label htmlFor={`read-add-user`}>
<div>Read</div>
<input type='hidden' />
</label>
</div>
</div>
<div className={cn(styles.col5)}>
<div className={styles.tcRadioButton}>
<input
name={`add-user-radio`}
type='radio'
id={`write-add-user`}
checked={userPermissionToAdd === PROJECT_ROLES.WRITE}
onChange={() => setUserPermissionToAdd(PROJECT_ROLES.WRITE)}
/>
<label htmlFor={`write-add-user`}>
<div>Write</div>
<input type='hidden' />
</label>
</div>
</div>
<div className={cn(styles.col5)}>
<div className={styles.tcRadioButton}>
<input
name={`add-user-radio`}
type='radio'
id={`full-access-add-user`}
checked={userPermissionToAdd === PROJECT_ROLES.MANAGER}
onChange={() => setUserPermissionToAdd(PROJECT_ROLES.MANAGER)}
/>
<label htmlFor={`full-access-add-user`}>
<div>Full Access</div>
<input type='hidden' />
</label>
</div>
</div>
<div className={cn(styles.col5)}>
<div className={styles.tcRadioButton}>
<input
name={`add-user-radio`}
type='radio'
id={`copilot-add-user`}
checked={userPermissionToAdd === PROJECT_ROLES.COPILOT}
onChange={() => setUserPermissionToAdd(PROJECT_ROLES.COPILOT)}
/>
<label htmlFor={`copilot-add-user`}>
<div>Copilot</div>
<input type='hidden' />
</label>
</div>
</div>
</div>
{addUserError && (
<div className={styles.errorMesssage}>{addUserError}</div>
)}
</div>
<div className={cn(styles.col5)}>
<div className={styles.tcRadioButton}>
<input
name={`add-user-radio`}
type='radio'
id={`full-access-add-user`}
checked={userPermissionToAdd === PROJECT_ROLES.MANAGER}
onChange={() => setUserPermissionToAdd(PROJECT_ROLES.MANAGER)}
<div className={styles.buttonGroup}>
<div className={styles.buttonSizeA}>
<PrimaryButton
text={'Close'}
type={'info'}
onClick={onClose}
/>
<label htmlFor={`full-access-add-user`}>
<div>Full Access</div>
<input type='hidden' />
</label>
</div>
</div>
<div className={cn(styles.col5)}>
<div className={styles.tcRadioButton}>
<input
name={`add-user-radio`}
type='radio'
id={`copilot-add-user`}
checked={userPermissionToAdd === PROJECT_ROLES.COPILOT}
onChange={() => setUserPermissionToAdd(PROJECT_ROLES.COPILOT)}
<div className={styles.buttonSizeA}>
<PrimaryButton
text={isAdding ? 'Adding user...' : 'Add User'}
type={'info'}
onClick={onAddUserConfirmClick}
/>
<label htmlFor={`copilot-add-user`}>
<div>Copilot</div>
<input type='hidden' />
</label>
</div>
</div>
</div>
{addUserError && (
<div className={styles.errorMesssage}>{addUserError}</div>
)}
</div>
<div className={styles.buttonGroup}>
<div className={styles.buttonSizeA}>
<PrimaryButton
text={'Close'}
type={'info'}
onClick={onClose}
/>
</div>
<div className={styles.buttonSizeA}>
<PrimaryButton
text={isAdding ? 'Adding user...' : 'Add User'}
type={'info'}
onClick={onAddUserConfirmClick}
/>
</div>
</div>
</div>
)
}
</Modal>
)
}
UserAddModalContent.propTypes = {
projectId: PropTypes.number.isRequired,
addNewProjectMember: PropTypes.func.isRequired,
onMemberInvited: PropTypes.func.isRequired,
onClose: PropTypes.func.isRequired
onClose: PropTypes.func.isRequired,
projectOption: PropTypes.any.isRequired,
Copy link

Choose a reason for hiding this comment

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

Consider using a more specific PropType instead of PropTypes.any for projectOption to ensure type safety and clarity. If the type is not clear, consider defining it or using a more descriptive PropType.

projectMembers: PropTypes.array.isRequired,
updateProjectMember: PropTypes.func.isRequired
}

export default UserAddModalContent
5 changes: 3 additions & 2 deletions src/services/projects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,10 @@ export async function fetchProjectPhases (id) {
* @param newRole the new role
* @returns {Promise<*>}
*/
export async function updateProjectMemberRole (projectId, memberRecordId, newRole) {
export async function updateProjectMemberRole (projectId, memberRecordId, newRole, action) {
Copy link

Choose a reason for hiding this comment

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

The function updateProjectMemberRole now includes an additional parameter action. Ensure that all calls to this function throughout the codebase are updated to pass this new parameter, or provide a default value to maintain backward compatibility.

const response = await axiosInstance.patch(`${PROJECTS_API_URL}/${projectId}/members/${memberRecordId}`, {
role: newRole
role: newRole,
action
})
return _.get(response, 'data')
}
Expand Down