-
Notifications
You must be signed in to change notification settings - Fork 52
[PROD RELEASE] - WorkManager Changes - Connect Decommission #1633
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
Conversation
…ression Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
PM-971 Allow hyphen in url - asset library
fix(PM-974) Allow project managers to view all projects
feat(PM-974): allow PM to view users and delete users from project
fix(PM-974): projects list in challenges tab
PM-973 invite by mail
PM-973 - fetch project invites sepparately
@@ -20,7 +20,8 @@ import { | |||
UPDATE_PROJECT_FAILURE, | |||
ADD_PROJECT_ATTACHMENT_SUCCESS, | |||
UPDATE_PROJECT_ATTACHMENT_SUCCESS, | |||
REMOVE_PROJECT_ATTACHMENT_SUCCESS | |||
REMOVE_PROJECT_ATTACHMENT_SUCCESS, | |||
LOAD_PROJECT_INVITES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a new constant LOAD_PROJECT_INVITES
has been added. Ensure that this constant is defined in the ../config/constants
file and is being used appropriately in the codebase.
@@ -30,9 +31,10 @@ import { | |||
createProjectApi, | |||
fetchBillingAccounts, | |||
fetchMemberProjects, | |||
updateProjectApi | |||
updateProjectApi, | |||
getProjectInvites |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ensure that getProjectInvites
is actually used in the code. If it's not used, consider removing it to keep the imports clean and avoid unnecessary dependencies.
@@ -8,11 +8,11 @@ import { PROJECT_STATUSES } from '../../config/constants' | |||
|
|||
import styles from './ProjectCard.module.scss' | |||
|
|||
const ProjectCard = ({ projectName, projectStatus, projectId, selected }) => { | |||
const ProjectCard = ({ projectName, projectStatus, projectId, selected, isInvited }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a default value for the isInvited
prop to ensure it is always a boolean. This can prevent potential issues if the prop is undefined.
const { auth } = this.props | ||
|
||
if (checkIsUserInvited(auth.token, this.props.projectDetail)) { | ||
this.props.history.push(`/projects/${this.props.projectId}/invitation`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change from this.props.projectDetail.id
to this.props.projectId
may affect the logic if projectId
is not equivalent to projectDetail.id
. Ensure that projectId
is correctly set and used throughout the component to avoid potential mismatches.
import { toastr } from 'react-redux-toastr' | ||
import { checkIsUserInvited } from '../../util/tc' | ||
import { isEmpty } from 'lodash' | ||
import { loadProjectInvites } from '../../actions/projects' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement has been changed from loadProject
to loadProjectInvites
. Ensure that this change is intentional and that the loadProjectInvites
function is defined and used correctly in the code. If loadProject
is still needed elsewhere in the file, consider importing both functions.
container: styles.modalContainer | ||
} | ||
|
||
const ProjectInvitations = ({ match, auth, isProjectLoading, history, projectDetail, loadProjectInvites }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The prop loadProject
has been renamed to loadProjectInvites
. Ensure that all instances where loadProject
is used within this component are updated accordingly to prevent potential errors.
|
||
if (isProjectLoading || isEmpty(projectDetail)) { | ||
if (!isProjectLoading) { | ||
loadProjectInvites(projectId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function loadProjectInvites
is being called instead of loadProject
. Ensure that this change is intentional and that loadProjectInvites
is defined and behaves as expected for the given context.
|
||
// await for the project details to propagate | ||
await delay(1000) | ||
await loadProjectInvites(projectId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function loadProjectInvites
is used here, replacing loadProject
. Ensure that loadProjectInvites
is defined and correctly implemented to handle the logic previously managed by loadProject
. If loadProjectInvites
is a new function, verify that it is imported and available in this context.
// await for the project details to fetch | ||
await delay(1000) | ||
history.push(status === PROJECT_MEMBER_INVITE_STATUS_ACCEPTED ? `/projects/${projectId}/challenges` : '/projects') | ||
}, [projectId, invitation, loadProjectInvites, history]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency array for the useEffect hook has been updated to include loadProjectInvites
instead of loadProject
. Ensure that loadProjectInvites
is a stable reference (e.g., memoized or defined outside of the component) to prevent unnecessary re-renders.
auth: PropTypes.object.isRequired, | ||
isProjectLoading: PropTypes.bool, | ||
history: PropTypes.object, | ||
loadProjectInvites: PropTypes.func.isRequired, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name loadProjectInvites
suggests it only loads project invitations. Ensure that this change aligns with the intended functionality and that other parts of the codebase are updated accordingly to reflect this change.
const mapStateToProps = ({ projects, auth }) => { | ||
return { | ||
projectDetail: projects.projectDetail, | ||
isProjectLoading: projects.isLoading || projects.isProjectInvitationsLoading, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition projects.isLoading || projects.isProjectInvitationsLoading
may lead to unexpected behavior if both flags are true. Consider if this logic accurately reflects the loading state you want to convey.
src/containers/Projects/index.js
Outdated
@@ -5,7 +5,7 @@ import { withRouter, Link } from 'react-router-dom' | |||
import { connect } from 'react-redux' | |||
import PropTypes from 'prop-types' | |||
import Loader from '../../components/Loader' | |||
import { checkAdminOrCopilot } from '../../util/tc' | |||
import { checkAdminOrCopilot, checkIsUserInvited, checkManager } from '../../util/tc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider alphabetizing the import statements for better readability and maintainability. This will help in quickly locating specific imports.
src/containers/Projects/index.js
Outdated
</div> | ||
{projects.length > 0 ? ( | ||
<> | ||
<ul> | ||
{projects.map(p => ( | ||
<li key={p.id}> | ||
<ProjectCard | ||
isInvited={!!checkIsUserInvited(auth.token, p)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function checkIsUserInvited
is used here, but it's not clear from the diff if this function is defined and imported correctly. Ensure that checkIsUserInvited
is defined and imported in this file to avoid runtime errors.
@@ -30,7 +30,10 @@ import { | |||
UPDATE_PROJECT_DETAILS_SUCCESS, | |||
ADD_PROJECT_ATTACHMENT_SUCCESS, | |||
UPDATE_PROJECT_ATTACHMENT_SUCCESS, | |||
REMOVE_PROJECT_ATTACHMENT_SUCCESS | |||
REMOVE_PROJECT_ATTACHMENT_SUCCESS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a missing trailing comma in the import statement. Consider adding a comma after REMOVE_PROJECT_ATTACHMENT_SUCCESS
to maintain consistency with the added lines.
...state, | ||
projectDetail: { | ||
...state.projectDetail, | ||
invites: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider checking if state.projectDetail
is defined before spreading it to prevent potential runtime errors if state.projectDetail
is undefined.
...state, | ||
projectDetail: { | ||
...state.projectDetail, | ||
invites: action.payload |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider checking if action.payload
is an array before assigning it to invites
to ensure that the data structure is as expected.
* @returns {Promise<*>} | ||
*/ | ||
export async function getProjectInvites (projectId) { | ||
const response = await axiosInstance.get(`${PROJECTS_API_URL}/${projectId}/invites`) |
There was a problem hiding this comment.
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 issues with the network or server are gracefully managed.
PM-973 - fix checkIsUserInvitedToProject
import { | ||
loadNextProjects, | ||
setActiveProject, | ||
resetSidebarActiveParams | ||
} from '../../actions/sidebar' | ||
import { checkAdmin } from '../../util/tc' | ||
import { checkAdmin, checkIsUserInvitedToProject } from '../../util/tc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name checkIsUserInvitedToProject
suggests a more specific purpose than the previous checkIsUserInvited
. Ensure that this change is intentional and that the function is correctly implemented to handle the specific context of checking if a user is invited to a project.
componentDidUpdate () { | ||
const { auth } = this.props | ||
|
||
if (checkIsUserInvitedToProject(auth.token, this.props.projectDetail)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name has been changed from checkIsUserInvited
to checkIsUserInvitedToProject
. Ensure that this function is defined and correctly implemented elsewhere in the codebase, and that all references to the old function name have been updated accordingly.
@@ -15,7 +15,7 @@ import { | |||
updateProject | |||
} from '../../actions/projects' | |||
import { setActiveProject } from '../../actions/sidebar' | |||
import { checkAdminOrCopilot, checkAdmin } from '../../util/tc' | |||
import { checkAdminOrCopilot, checkAdmin, checkIsUserInvitedToProject } from '../../util/tc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function checkIsUserInvited
has been renamed to checkIsUserInvitedToProject
. Ensure that all instances where this function is called are updated accordingly throughout the codebase to prevent any potential errors.
@@ -37,6 +37,11 @@ class ProjectEditor extends Component { | |||
|
|||
componentDidUpdate () { | |||
const { auth } = this.props | |||
|
|||
if (checkIsUserInvitedToProject(auth.token, this.props.projectDetail)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name has been changed from checkIsUserInvited
to checkIsUserInvitedToProject
. Ensure that this function is defined and correctly implemented elsewhere in the codebase to avoid runtime errors.
import { connect } from 'react-redux' | ||
import { withRouter } from 'react-router-dom' | ||
import { toastr } from 'react-redux-toastr' | ||
import { checkIsUserInvitedToProject } from '../../util/tc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name has been changed from checkIsUserInvited
to checkIsUserInvitedToProject
. Ensure that all references to this function throughout the codebase are updated accordingly to prevent any potential errors.
const ProjectInvitations = ({ match, auth, isProjectLoading, history, projectDetail, loadProjectInvites }) => { | ||
const automaticAction = useMemo(() => [PROJECT_MEMBER_INVITE_STATUS_ACCEPTED, PROJECT_MEMBER_INVITE_STATUS_REFUSED].includes(match.params.action) ? match.params.action : undefined, [match.params]) | ||
const projectId = useMemo(() => parseInt(match.params.projectId), [match.params]) | ||
const invitation = useMemo(() => checkIsUserInvitedToProject(auth.token, projectDetail), [auth.token, projectDetail]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function checkIsUserInvitedToProject
is used here, replacing checkIsUserInvited
. Ensure that this function is defined and correctly implemented elsewhere in the codebase, as this change might affect the logic of invitation checking.
@@ -5,7 +5,7 @@ import { withRouter, Link } from 'react-router-dom' | |||
import { connect } from 'react-redux' | |||
import PropTypes from 'prop-types' | |||
import Loader from '../../components/Loader' | |||
import { checkAdminOrCopilot } from '../../util/tc' | |||
import { checkAdminOrCopilot, checkIsUserInvitedToProject, checkManager } from '../../util/tc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function checkIsUserInvited
has been renamed to checkIsUserInvitedToProject
. Ensure that all instances of this function call throughout the codebase are updated to reflect this change to avoid any potential errors.
</div> | ||
{projects.length > 0 ? ( | ||
<> | ||
<ul> | ||
{projects.map(p => ( | ||
<li key={p.id}> | ||
<ProjectCard | ||
isInvited={!!checkIsUserInvitedToProject(auth.token, p)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name has been changed from checkIsUserInvited
to checkIsUserInvitedToProject
. Ensure that this function is defined and correctly implemented elsewhere in the codebase to avoid any runtime errors.
@@ -227,6 +233,15 @@ export const checkAdminOrCopilot = (token, project) => { | |||
return isAdmin || (isCopilot && canManageProject) | |||
} | |||
|
|||
export const checkIsUserInvitedToProject = (token, project) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider renaming the function to isUserInvitedToProject
to follow the convention of starting boolean-returning function names with 'is'.
} | ||
|
||
const tokenData = decodeToken(token) | ||
return project && !_.isEmpty(project) && (_.find(project.invites, d => d.userId === tokenData.userId || d.email === tokenData.email)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for checking if a user is invited now includes checking by email. Ensure that tokenData.email
is always available and correctly populated to avoid potential undefined errors.
PM-973 - Update label for "cancel" on invitation modal
Changes to be done in Work Manager while decommissioning Connect.
Updates: