Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 0 additions & 4 deletions static/app/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,6 @@ function buildRoutes(): RouteObject[] {
component: make(
() => import('sentry/views/settings/organizationTeams/teamDetails')
),
deprecatedRouteProps: true,
children: [
{
index: true,
Expand All @@ -1001,7 +1000,6 @@ function buildRoutes(): RouteObject[] {
component: make(
() => import('sentry/views/settings/organizationTeams/teamMembers')
),
deprecatedRouteProps: true,
},
{
path: 'notifications/',
Copy link
Member Author

Choose a reason for hiding this comment

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

TeamNotificationSettings makes its own API request to fetch the team - I believe this is unnecessary and it can be folded into the same pattern as the other children with useOutletContext. @scttcper mind confirming?

Expand All @@ -1016,15 +1014,13 @@ function buildRoutes(): RouteObject[] {
component: make(
() => import('sentry/views/settings/organizationTeams/teamProjects')
),
deprecatedRouteProps: true,
},
{
path: 'settings/',
name: t('Settings'),
component: make(
() => import('sentry/views/settings/organizationTeams/teamSettings')
),
deprecatedRouteProps: true,
},
],
},
Expand Down
68 changes: 34 additions & 34 deletions static/app/views/settings/organizationTeams/teamDetails.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
import TeamStore from 'sentry/stores/teamStore';
import TeamDetails from 'sentry/views/settings/organizationTeams/teamDetails';

describe('TeamMembers', () => {
describe('TeamDetails', () => {
let joinMock: any;

const organization = OrganizationFixture();
const team = TeamFixture({hasAccess: false});
const teamNoAccess = TeamFixture({slug: 'flask', hasAccess: false});
const teamHasAccess = TeamFixture({id: '1337', slug: 'django', hasAccess: true});

beforeEach(() => {
TeamStore.init();
TeamStore.loadInitialData([team, teamHasAccess]);
TeamStore.loadInitialData([teamNoAccess, teamHasAccess]);
joinMock = MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/members/me/teams/${team.slug}/`,
url: `/organizations/${organization.slug}/members/me/teams/${teamNoAccess.slug}/`,
method: 'POST',
});
});
Expand All @@ -28,43 +28,43 @@ describe('TeamMembers', () => {
});

it('can request membership', async () => {
render(
<TeamDetails>
<div data-test-id="test" />
</TeamDetails>,
{
organization,
initialRouterConfig: {
location: {
pathname: `/settings/${organization.slug}/teams/${team.slug}/`,
},
route: '/settings/:orgId/teams/:teamId/',
render(<TeamDetails />, {
organization,
initialRouterConfig: {
location: {
pathname: `/settings/${organization.slug}/teams/${teamNoAccess.slug}/`,
},
}
);
route: '/settings/:orgId/teams/:teamId/',
},
});

// No access - tabs are not shown
expect(await screen.findByText('#flask')).toBeInTheDocument();
expect(screen.queryByText('Members')).not.toBeInTheDocument();
expect(screen.queryByText('Projects')).not.toBeInTheDocument();
expect(screen.queryByText('Notifications')).not.toBeInTheDocument();
expect(screen.queryByText('Settings')).not.toBeInTheDocument();

await userEvent.click(screen.getByRole('button', {name: 'Request Access'}));
expect(joinMock).toHaveBeenCalled();

expect(screen.queryByTestId('test')).not.toBeInTheDocument();
});

it('displays children', async () => {
render(
<TeamDetails>
<div data-test-id="test" />
</TeamDetails>,
{
organization,
initialRouterConfig: {
location: {
pathname: `/settings/${organization.slug}/teams/${teamHasAccess.slug}/`,
},
route: '/settings/:orgId/teams/:teamId/',
it('renders tabs when team has access', async () => {
render(<TeamDetails />, {
organization,
initialRouterConfig: {
location: {
pathname: `/settings/${organization.slug}/teams/${teamHasAccess.slug}/`,
},
}
);
route: '/settings/:orgId/teams/:teamId/',
},
});

expect(await screen.findByTestId('test')).toBeInTheDocument();
// Has access - team name & tabs are shown
expect(await screen.findByText('#django')).toBeInTheDocument();
expect(screen.getByText('Members')).toBeInTheDocument();
expect(screen.getByText('Projects')).toBeInTheDocument();
expect(screen.getByText('Notifications')).toBeInTheDocument();
expect(screen.getByText('Settings')).toBeInTheDocument();
});
});
16 changes: 8 additions & 8 deletions static/app/views/settings/organizationTeams/teamDetails.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {cloneElement, isValidElement, useState} from 'react';
import {useState} from 'react';
import {Outlet} from 'react-router-dom';
import styled from '@emotion/styled';

import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
Expand All @@ -11,22 +12,23 @@ import LoadingIndicator from 'sentry/components/loadingIndicator';
import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
import {t, tct} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import type {Team} from 'sentry/types/organization';
import useApi from 'sentry/utils/useApi';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import {useParams} from 'sentry/utils/useParams';
import {useTeamsById} from 'sentry/utils/useTeamsById';

type Props = {
children: React.ReactNode;
export type TeamDetailsOutletContext = {
team: Team;
};

function TeamDetails({children}: Props) {
export default function TeamDetails() {
const api = useApi();
const params = useParams<{teamId: string}>();
const location = useLocation();
const orgSlug = useOrganization().slug;
const [requesting, setRequesting] = useState(false);
const params = useParams<{teamId: string}>();
const {teams, isLoading, isError} = useTeamsById({slugs: [params.teamId]});
const team = teams.find(({slug}) => slug === params.teamId);

Expand Down Expand Up @@ -110,7 +112,7 @@ function TeamDetails({children}: Props) {
</Tabs>
</TabsContainer>

{isValidElement(children) ? cloneElement<any>(children, {team}) : null}
<Outlet context={{team} satisfies TeamDetailsOutletContext} />
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this the pattern we want? i.e. define a type for the outlet context and then assert that type on the object given to Outlet as context. Is there a better way to enforce this type?

</div>
) : (
<Alert.Container>
Expand Down Expand Up @@ -140,8 +142,6 @@ const TabsContainer = styled('div')`
margin-bottom: ${space(2)};
`;

export default TeamDetails;

const RequestAccessWrapper = styled('div')`
display: flex;
justify-content: space-between;
Expand Down
48 changes: 32 additions & 16 deletions static/app/views/settings/organizationTeams/teamMembers.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ describe('TeamMembers', () => {

it('can add member to team with open membership', async () => {
const org = OrganizationFixture({access: [], openMembership: true});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a way we can enforce the types match when outletContext is provided to render? (other than putting satisfies everywhere)

initialRouterConfig,
organization: org,
});
Expand All @@ -83,7 +84,8 @@ describe('TeamMembers', () => {

it('can add multiple members with one click on dropdown', async () => {
const org = OrganizationFixture({access: [], openMembership: true});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: org,
});
Expand All @@ -99,7 +101,8 @@ describe('TeamMembers', () => {

it('can add member to team with team:admin permission', async () => {
const org = OrganizationFixture({access: ['team:admin'], openMembership: false});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: org,
});
Expand All @@ -114,7 +117,8 @@ describe('TeamMembers', () => {

it('can add member to team with org:write permission', async () => {
const org = OrganizationFixture({access: ['org:write'], openMembership: false});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: org,
});
Expand All @@ -129,7 +133,8 @@ describe('TeamMembers', () => {

it('can request access to add member to team without permission', async () => {
const org = OrganizationFixture({access: [], openMembership: false});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: org,
});
Expand All @@ -149,7 +154,8 @@ describe('TeamMembers', () => {
openMembership: false,
}),
});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: org,
});
Expand All @@ -169,7 +175,8 @@ describe('TeamMembers', () => {
openMembership: true,
}),
});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: org,
});
Expand All @@ -186,7 +193,8 @@ describe('TeamMembers', () => {
const {organization: org} = initializeOrg({
organization: OrganizationFixture({access: [], openMembership: true}),
});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: org,
});
Expand All @@ -203,7 +211,8 @@ describe('TeamMembers', () => {
const {organization: org} = initializeOrg({
organization: OrganizationFixture({access: [], openMembership: false}),
});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: org,
});
Expand All @@ -221,7 +230,8 @@ describe('TeamMembers', () => {
url: `/organizations/${organization.slug}/members/${members[0]!.id}/teams/${team.slug}/`,
method: 'DELETE',
});
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization,
});
Expand Down Expand Up @@ -251,7 +261,8 @@ describe('TeamMembers', () => {
});
const organizationMember = OrganizationFixture({access: []});

render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: organizationMember,
});
Expand Down Expand Up @@ -283,7 +294,8 @@ describe('TeamMembers', () => {
body: [...members, owner],
});

render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization,
});
Expand All @@ -309,7 +321,8 @@ describe('TeamMembers', () => {

const orgWithTeamRoles = OrganizationFixture({features: ['team-roles']});

render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization: orgWithTeamRoles,
});
Expand Down Expand Up @@ -354,7 +367,8 @@ describe('TeamMembers', () => {
body: team2,
});

render(<TeamMembers team={team2} />, {
render(<TeamMembers />, {
outletContext: {team: team2},
initialRouterConfig,
organization,
});
Expand Down Expand Up @@ -399,7 +413,8 @@ describe('TeamMembers', () => {
body: team2,
});

render(<TeamMembers team={team2} />, {
render(<TeamMembers />, {
outletContext: {team: team2},
initialRouterConfig,
organization,
});
Expand All @@ -411,7 +426,8 @@ describe('TeamMembers', () => {
});

it('renders a "Pending" tag for pending team members', async () => {
render(<TeamMembers team={team} />, {
render(<TeamMembers />, {
outletContext: {team},
initialRouterConfig,
organization,
});
Expand Down
Loading
Loading