Skip to content
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

Jump to bottom

fix(projects): Show join a team if person doesn't have a team #55499

Merged
merged 5 commits into from
Aug 30, 2023
Merged

fix(projects): Show join a team if person doesn't have a team #55499

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8b56252
fix(projects): Show join a team if person doesn't have a team
sentaur-athena Aug 23, 2023
deb2711
fix test
sentaur-athena Aug 23, 2023
0ef863e
more broken tests
sentaur-athena Aug 24, 2023
efdc82a
fix(projects): Show join a team if user doesn't have a team
sentaur-athena Aug 30, 2023
2e4bf3b
super user doesnt need access
sentaur-athena Aug 30, 2023
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
18 changes: 18 additions & 0 deletions static/app/components/noProjectMessage.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,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 () {
Copy link
Member Author

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the test 👏

ProjectsStore.loadInitialData([
TestStubs.Project({hasAccess: true, isMember: false}),
Copy link
Member

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?

Copy link
Member Author

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.

]);

ConfigStore.set('user', {...ConfigStore.get('user'), isSuperuser: true});

render(
<NoProjectMessage organization={org}>
<Label data-test-id="project-row" isDisabled>
Some Project
</Label>
</NoProjectMessage>
);

expect(screen.getByTestId('project-row')).toBeInTheDocument();
});
});
5 changes: 4 additions & 1 deletion static/app/components/noProjectMessage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ function NoProjectMessage({
? !!projects?.some(p => p.hasAccess)
: !!projects?.some(p => p.isMember && p.hasAccess);

if (isTeamMember && (hasProjectAccess || !projectsLoaded || !teamsLoaded)) {
if (
(isTeamMember || isSuperuser) &&
Copy link
Member Author

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

(hasProjectAccess || !projectsLoaded || !teamsLoaded)
) {
return <Fragment>{children}</Fragment>;
}

Expand Down