-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
fix(projects): Show join a team if person doesn't have a team #55499
Conversation
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.
Adding what changed after reverted PR for easier review
@@ -39,7 +40,10 @@ function NoProjectMessage({ | |||
? !!projects?.some(p => p.hasAccess) | |||
: !!projects?.some(p => p.isMember && p.hasAccess); | |||
|
|||
if (hasProjectAccess || !projectsLoaded || !teamsLoaded) { | |||
if ( | |||
(isTeamMember || isSuperuser) && |
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.
This is the only change from previous PR
@@ -96,4 +132,22 @@ describe('NoProjectMessage', function () { | |||
screen.getByText('You need at least one project to use this view') | |||
).toBeInTheDocument(); | |||
}); | |||
|
|||
it('shows projects to superusers if membership is not required', function () { |
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.
This is the test that would have failed on the previous PR
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.
Thanks for the test 👏
Did you link the correct PR in your description? |
|
||
it('shows projects to superusers if membership is not required', function () { | ||
ProjectsStore.loadInitialData([ | ||
TestStubs.Project({hasAccess: true, isMember: false}), |
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.
do we always return hasAccess: true
for projects when you're superuser?
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.
No. The component I'm modifying has a prop superuserNeedsToBeProjectMember
that if that is set they will only see projects that have access to. And if false they see all projects. This component is used in multiple places so that's why the prop exist. Let me remove the access field from the test to make it simpler since it doesn't matter.
@nhsiehgit you're right. I fixed it 😅 |
@@ -96,4 +132,22 @@ describe('NoProjectMessage', function () { | |||
screen.getByText('You need at least one project to use this view') | |||
).toBeInTheDocument(); | |||
}); | |||
|
|||
it('shows projects to superusers if membership is not required', function () { |
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.
Thanks for the test 👏
PR reverted: 8a10a85 |
This reverts commit e4f8f53. Co-authored-by: sentaur-athena <132939361+sentaur-athena@users.noreply.github.com>
This is breaking some other experience again. I'll go ahead and make another change only affecting fly people or something 😅 |
Exact copy of #55190 except this time considering that super users don't have teams and still might have project access.