Skip to content

[PROD RELEASE] - BA selection UI exposed to PMs #1641

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.

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 11 commits into from
Apr 24, 2025
1 change: 1 addition & 0 deletions config/constants/development.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ module.exports = {
PROJECT_API_URL: `${DEV_API_HOSTNAME}/v5/projects`,
GROUPS_API_URL: `${DEV_API_HOSTNAME}/v5/groups`,
TERMS_API_URL: `${DEV_API_HOSTNAME}/v5/terms`,
MEMBERS_API_URL: `${DEV_API_HOSTNAME}/v5/members`,
RESOURCES_API_URL: `${DEV_API_HOSTNAME}/v5/resources`,
RESOURCE_ROLES_API_URL: `${DEV_API_HOSTNAME}/v5/resource-roles`,
SUBMISSIONS_API_URL: `${DEV_API_HOSTNAME}/v5/submissions`,
Expand Down
1 change: 1 addition & 0 deletions config/constants/production.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module.exports = {
PROJECT_API_URL: `${PROD_API_HOSTNAME}/v5/projects`,
GROUPS_API_URL: `${PROD_API_HOSTNAME}/v5/groups`,
TERMS_API_URL: `${PROD_API_HOSTNAME}/v5/terms`,
MEMBERS_API_URL: `${PROD_API_HOSTNAME}/v5/members`,
RESOURCES_API_URL: `${PROD_API_HOSTNAME}/v5/resources`,
RESOURCE_ROLES_API_URL: `${PROD_API_HOSTNAME}/v5/resource-roles`,
SUBMISSIONS_API_URL: `${PROD_API_HOSTNAME}/v5/submissions`,
Expand Down
2 changes: 1 addition & 1 deletion src/components/UserCard/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class UserCard extends Component {
)}
<div className={styles.item}>
<div className={cn(styles.col5)}>
{isInvite ? user.email : user.handle}
{isInvite ? (user.email || user.handle) : user.handle}

Choose a reason for hiding this comment

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

The change introduces a fallback to user.handle if user.email is not available when isInvite is true. Consider checking if both user.email and user.handle are defined before using them to prevent potential issues with undefined values. For example, you could use user.email ? user.email : user.handle to ensure that user.handle is only used when user.email is not available.

</div>
{!isInvite && (
<>
Expand Down
1 change: 1 addition & 0 deletions src/components/Users/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ class Users extends Component {
<UserAddModalContent
projectId={this.state.projectOption.value}
addNewProjectMember={this.props.addNewProjectMember}
onMemberInvited={this.props.addNewProjectInvite}
onClose={this.resetAddUserState}
/>
)
Expand Down
5 changes: 4 additions & 1 deletion src/components/Users/invite-user.modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ const InviteUserModalContent = ({ projectId, onClose, onMemberInvited, projectMe

try {
// api restriction: ONLY "customer" role can be invited via email
const { success: invitations = [], failed } = await inviteUserToProject(projectId, emailToInvite, PROJECT_ROLES.CUSTOMER)
const { success: invitations = [], failed } = await inviteUserToProject(projectId, {
emails: [emailToInvite],
role: PROJECT_ROLES.CUSTOMER
})

if (failed) {
const error = get(failed, '0.message', 'Unable to invite user')
Expand Down
31 changes: 25 additions & 6 deletions src/components/Users/user-add.modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ import Modal from '../Modal'
import SelectUserAutocomplete from '../SelectUserAutocomplete'
import { PROJECT_ROLES } from '../../config/constants'
import PrimaryButton from '../Buttons/PrimaryButton'
import { addUserToProject } from '../../services/projects'
import { addUserToProject, inviteUserToProject } from '../../services/projects'

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

const theme = {
container: styles.modalContainer
}

const UserAddModalContent = ({ projectId, addNewProjectMember, onClose }) => {
const UserAddModalContent = ({ projectId, addNewProjectMember, onMemberInvited, onClose }) => {
const [userToAdd, setUserToAdd] = useState(null)
const [userPermissionToAdd, setUserPermissionToAdd] = useState(PROJECT_ROLES.READ)
const [showSelectUserError, setShowSelectUserError] = useState(false)
Expand Down Expand Up @@ -45,10 +45,28 @@ const UserAddModalContent = ({ projectId, addNewProjectMember, onClose }) => {
setAddUserError(null)

try {
const newUserInfo = await addUserToProject(projectId, userToAdd.userId, userPermissionToAdd)
newUserInfo.handle = userToAdd.handle
addNewProjectMember(newUserInfo)
onClose()
if (userPermissionToAdd === PROJECT_ROLES.COPILOT) {
const { success: invitations = [], failed, ...rest } = await inviteUserToProject(projectId, {

Choose a reason for hiding this comment

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

Consider checking if rest.message is a string before using it in the condition on line 57 to ensure that it doesn't cause unexpected behavior if rest.message is not a string.

handles: [userToAdd.handle],
role: userPermissionToAdd
})
if (failed) {
const error = get(failed, '0.message', 'User cannot be invited')

Choose a reason for hiding this comment

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

The error message has been changed from 'Unable to invite user' to 'User cannot be invited'. Ensure that this change aligns with the user experience and error messaging guidelines.

setAddUserError(error)
setIsAdding(false)
} else if (rest.message) {
setAddUserError(rest.message)
setIsAdding(false)
} else {
onMemberInvited(invitations[0] || {})

Choose a reason for hiding this comment

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

Ensure that invitations[0] is not undefined before accessing it to avoid potential runtime errors. Consider adding a check or default value.

onClose()
}
} else {
const newUserInfo = await addUserToProject(projectId, userToAdd.userId, userPermissionToAdd)
newUserInfo.handle = userToAdd.handle
addNewProjectMember(newUserInfo)
onClose()
}
} catch (e) {
const error = get(e, 'response.data.message', 'Unable to add user')
setAddUserError(error)
Expand Down Expand Up @@ -169,6 +187,7 @@ const UserAddModalContent = ({ projectId, addNewProjectMember, onClose }) => {
UserAddModalContent.propTypes = {
projectId: PropTypes.number.isRequired,
addNewProjectMember: PropTypes.func.isRequired,
onMemberInvited: PropTypes.func.isRequired,

Choose a reason for hiding this comment

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

The new prop onMemberInvited is marked as required, but there is no context provided in this diff about its usage or necessity. Ensure that this prop is indeed required and used appropriately within the component. If it's not used or optional, consider updating the prop type accordingly.

onClose: PropTypes.func.isRequired
}

Expand Down
1 change: 1 addition & 0 deletions src/config/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export const FILE_PICKER_PROGRESS_INTERVAL = 100
export const FILE_PICKER_UPLOAD_RETRY = 2
export const FILE_PICKER_UPLOAD_TIMEOUT = 30 * 60 * 1000 // 30 minutes
export const SPECIFICATION_ATTACHMENTS_FOLDER = 'SPECIFICATION_ATTACHMENTS'
export const MEMBERS_API_URL = process.env.MEMBERS_API_URL

export const getAWSContainerFileURL = (key) => `https://${FILE_PICKER_CONTAINER_NAME}.s3.amazonaws.com/${key}`

Expand Down
12 changes: 9 additions & 3 deletions src/containers/Users/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import _ from 'lodash'
import PT from 'prop-types'
import UsersComponent from '../../components/Users'
import { PROJECT_ROLES } from '../../config/constants'
import { fetchProjectById } from '../../services/projects'
import { fetchInviteMembers, fetchProjectById } from '../../services/projects'
import { checkAdmin, checkManager } from '../../util/tc'

import {
Expand Down Expand Up @@ -80,12 +80,18 @@ class Users extends Component {
}

loadProject (projectId) {
fetchProjectById(projectId).then((project) => {
fetchProjectById(projectId).then(async (project) => {
const projectMembers = _.get(project, 'members')
const invitedMembers = _.get(project, 'invites')
const invitedUserIds = _.filter(_.map(invitedMembers, 'userId'))

Choose a reason for hiding this comment

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

Consider checking if invitedMembers is not null or undefined before mapping over it to avoid potential runtime errors.

const invitedUsers = await fetchInviteMembers(invitedUserIds)

Choose a reason for hiding this comment

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

Ensure that fetchInviteMembers handles cases where invitedUserIds might be an empty array, as this could lead to unnecessary API calls or errors.


this.setState({
projectMembers,
invitedMembers
invitedMembers: invitedMembers.map(m => ({
...m,
email: m.email || invitedUsers[m.userId].handle

Choose a reason for hiding this comment

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

The change from email to handle might affect how invited members are identified or displayed. Ensure that this change aligns with the intended functionality and that handle provides the necessary information for the 'Select Billing Account' feature.

}))
})
const { loggedInUser } = this.props
this.updateLoginUserRoleInProject(projectMembers, loggedInUser)
Expand Down
23 changes: 17 additions & 6 deletions src/services/projects.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
GENERIC_PROJECT_MILESTONE_PRODUCT_TYPE,
PHASE_PRODUCT_CHALLENGE_ID_FIELD,
PHASE_PRODUCT_TEMPLATE_ID,
PROJECTS_API_URL
PROJECTS_API_URL,
MEMBERS_API_URL
} from '../config/constants'
import { paginationHeaders } from '../util/pagination'
import { createProjectMemberInvite } from './projectMemberInvites'
Expand Down Expand Up @@ -68,6 +69,19 @@ export async function fetchProjectById (id) {
return _.get(response, 'data')
}

/**
* This fetches the user corresponding to the given userIds
* @param {*} userIds

Choose a reason for hiding this comment

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

The parameter userIds is typed as *, which is not specific. Consider specifying the expected type, such as Array or Array<number>, to improve code readability and maintainability.

*/
export async function fetchInviteMembers (userIds) {
const url = `${MEMBERS_API_URL}?${userIds.map(id => `userIds[]=${id}`).join('&')}`

Choose a reason for hiding this comment

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

Consider adding error handling for the API request to ensure that any network or server errors are properly managed and do not cause the application to crash.

const { data = [] } = await axiosInstance.get(url)
return data.reduce((acc, member) => {
acc[member.userId] = member
return acc
}, {})
}

/**
* Api request for fetching project phases
* @param id Project id
Expand Down Expand Up @@ -118,11 +132,8 @@ export async function addUserToProject (projectId, userId, role) {
* @param role
* @returns {Promise<*>}
*/
export async function inviteUserToProject (projectId, email, role) {
return createProjectMemberInvite(projectId, {
emails: [email],
role: role
})
export async function inviteUserToProject (projectId, params) {

Choose a reason for hiding this comment

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

The function inviteUserToProject now accepts params instead of email and role separately. Ensure that the params object is validated properly before being passed to createProjectMemberInvite to prevent potential issues with missing or malformed data.

return createProjectMemberInvite(projectId, params)
}

/**
Expand Down