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.

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 11 commits into from
Apr 24, 2025
Merged

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

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3b7fbe2
feat: copilot invitation with email
hentrymartin Apr 10, 2025
bbe6312
Merge pull request #1638 from topcoder-platform/pm-972
hentrymartin Apr 14, 2025
997d8f0
fix: show update billing address for project manager who belongs to org
hentrymartin Apr 16, 2025
3c464c0
Merge pull request #1640 from topcoder-platform/pm-574
hentrymartin Apr 16, 2025
c8c1075
fix: handled errors for certain use cases
hentrymartin Apr 16, 2025
70f514e
Merge pull request #1643 from topcoder-platform/pm-972_1
hentrymartin Apr 22, 2025
e04fae1
fix: show handle for invit
hentrymartin Apr 22, 2025
50d6f0a
fix: show handle for invit
hentrymartin Apr 22, 2025
b0d0f48
removed circle ci changes
hentrymartin Apr 22, 2025
9f1728f
Merge pull request #1644 from topcoder-platform/pm-1077
hentrymartin Apr 22, 2025
00f7089
Merge pull request #1645 from topcoder-platform/master
kkartunov Apr 24, 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
1 change: 1 addition & 0 deletions config/constants/development.js
View file
Open in desktop
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
View file
Open in desktop
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
7 changes: 6 additions & 1 deletion src/components/ChallengesComponent/ChallengeList/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import Loader from '../../Loader'
import UpdateBillingAccount from '../../UpdateBillingAccount'

import { CHALLENGE_STATUS, PAGE_SIZE, PAGINATION_PER_PAGE_OPTIONS, PROJECT_ROLES } from '../../../config/constants'
import { checkAdmin, checkReadOnlyRoles } from '../../../util/tc'
import { checkAdmin, checkManager, checkReadOnlyRoles } from '../../../util/tc'

require('bootstrap/scss/bootstrap.scss')

Expand Down Expand Up @@ -406,6 +406,9 @@ class ChallengeList extends Component {
} = this.props
const isReadOnly = checkReadOnlyRoles(this.props.auth.token) || loginUserRoleInProject === PROJECT_ROLES.READ
const isAdmin = checkAdmin(this.props.auth.token)
const isManager = checkManager(this.props.auth.token)
Copy link

Choose a reason for hiding this comment

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

Consider renaming isManager to isProjectManager for clarity, as it specifically checks for the Project Manager role.

const loginUserId = this.props.auth.user.userId
const isMemberOfActiveProject = activeProject && activeProject.members && activeProject.members.some(member => member.userId === loginUserId)
Copy link

Choose a reason for hiding this comment

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

The variable isMemberOfActiveProject could be more descriptive. Consider renaming it to userIsMemberOfActiveProject to clearly indicate that it refers to the current user.


if (warnMessage) {
return <Message warnMessage={warnMessage} />
Expand Down Expand Up @@ -496,6 +499,8 @@ class ChallengeList extends Component {
currentBillingAccount={currentBillingAccount}
updateProject={updateProject}
projectId={activeProject.id}
isMemberOfActiveProject={isMemberOfActiveProject}
isManager={isManager}
/>
</div>
) : (
Expand Down
12 changes: 8 additions & 4 deletions src/components/UpdateBillingAccount/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ const UpdateBillingAccount = ({
isAdmin,
currentBillingAccount,
projectId,
updateProject
updateProject,
isMemberOfActiveProject,
isManager
}) => {
const [isEditing, setIsEditing] = useState(false)
const [selectedBillingAccount, setSelectedBillingAccount] = useState(null)
Expand Down Expand Up @@ -129,7 +131,7 @@ const UpdateBillingAccount = ({
!currentBillingAccount && (
<Fragment>
<span className={styles.error}>No Billing Account set</span>
{isAdmin && (
{(isAdmin || (isManager && isMemberOfActiveProject)) && (
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 descriptive variable name for isManager to clarify its purpose, especially if it refers specifically to a Project Manager role. This can improve code readability and maintainability.

<span>
{' '}
({' '}
Expand All @@ -153,7 +155,7 @@ const UpdateBillingAccount = ({
>
{isBillingAccountExpired ? 'INACTIVE' : 'ACTIVE'}
</span>{' '}
{isAdmin && (
{(isAdmin || (isManager && isMemberOfActiveProject)) && (
Copy link

Choose a reason for hiding this comment

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

Consider extracting the condition (isAdmin || (isManager && isMemberOfActiveProject)) into a well-named variable or function to improve readability and maintainability of the code.

<span>
{' '}
({' '}
Expand Down Expand Up @@ -187,7 +189,9 @@ UpdateBillingAccount.propTypes = {
isBillingAccountExpired: PropTypes.bool,
isAdmin: PropTypes.bool,
projectId: PropTypes.number,
updateProject: PropTypes.func.isRequired
updateProject: PropTypes.func.isRequired,
isMemberOfActiveProject: 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 prop type isMemberOfActiveProject is defined as PropTypes.bool.isRequired, but it's not clear from the context if this prop is being used in the component. Please ensure that this prop is utilized within the component logic.

isManager: 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 prop type isManager is defined as PropTypes.bool.isRequired, but it's not clear from the context if this prop is being used in the component. Please ensure that this prop is utilized within the component logic.

}

export default UpdateBillingAccount
1 change: 1 addition & 0 deletions src/components/Users/index.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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, {
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 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')
Copy link

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] || {})
Copy link

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,
Copy link

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
View file
Open in desktop
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
View file
Open in desktop
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'))
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 invitedMembers is not null or undefined before mapping over it to avoid potential runtime errors.

const invitedUsers = await fetchInviteMembers(invitedUserIds)
Copy link

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
Copy link

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
View file
Open in desktop
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
Copy link

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('&')}`
Copy link

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) {
Copy link

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