Skip to content

Commit 3fb7892

Browse files
Kira-PilotKira-Pilot
authored
fix: template permissions page never loads (#5014)
* removed redundant permissions loader * fixed tests
1 parent fefacc5 commit 3fb7892

File tree

3 files changed

+59
-81
lines changed

3 files changed

+59
-81
lines changed

‎site/src/components/TemplateLayout/TemplateLayout.tsx

+56-74
Original file line numberDiff line numberDiff line change
@@ -106,35 +106,23 @@ export const TemplateLayout: FC<PropsWithChildren> = ({ children }) => {
106106
organizationId,
107107
},
108108
})
109-
const {
110-
template,
111-
activeTemplateVersion,
112-
templateResources,
113-
templateDAUs,
114-
permissions: templatePermissions,
115-
} = templateState.context
109+
const { template, permissions: templatePermissions } = templateState.context
116110
const xServices = useContext(XServiceContext)
117111
const permissions = useSelector(xServices.authXService, selectPermissions)
118-
const isLoading =
119-
!template ||
120-
!activeTemplateVersion ||
121-
!templateResources ||
122-
!permissions ||
123-
!templateDAUs ||
124-
!templatePermissions
125-
126112
const hasIcon = template && template.icon && template.icon !== ""
127113

114+
if (!template) {
115+
return <Loader />
116+
}
117+
128118
const generatePageHeaderActions = (): JSX.Element[] => {
129119
const pageActions: JSX.Element[] = []
130120

131-
if (!isLoading && templatePermissions.canUpdateTemplate) {
132-
pageActions.push(<TemplateSettingsButton templateName={templateName} />)
121+
if (templatePermissions?.canUpdateTemplate) {
122+
pageActions.push(<TemplateSettingsButton templateName={template.name} />)
133123
}
134124

135-
if (!isLoading) {
136-
pageActions.push(<CreateWorkspaceButton templateName={templateName} />)
137-
}
125+
pageActions.push(<CreateWorkspaceButton templateName={template.name} />)
138126

139127
return pageActions
140128
}
@@ -152,65 +140,59 @@ export const TemplateLayout: FC<PropsWithChildren> = ({ children }) => {
152140
}
153141
>
154142
<Stack direction="row" spacing={3} className={styles.pageTitle}>
155-
{!isLoading && (
156-
<div>
157-
{hasIcon ? (
158-
<div className={styles.iconWrapper}>
159-
<img src={template.icon} alt="" />
160-
</div>
161-
) : (
162-
<Avatar className={styles.avatar}>
163-
{firstLetter(templateName)}
164-
</Avatar>
165-
)}
166-
</div>
167-
)}
143+
<div>
144+
{hasIcon ? (
145+
<div className={styles.iconWrapper}>
146+
<img src={template.icon} alt="" />
147+
</div>
148+
) : (
149+
<Avatar className={styles.avatar}>
150+
{firstLetter(template.name)}
151+
</Avatar>
152+
)}
153+
</div>
168154

169-
{!isLoading && (
170-
<div>
171-
<PageHeaderTitle>{templateName}</PageHeaderTitle>
172-
<PageHeaderSubtitle condensed>
173-
{template.description === ""
174-
? Language.noDescription
175-
: template.description}
176-
</PageHeaderSubtitle>
177-
</div>
178-
)}
155+
<div>
156+
<PageHeaderTitle>{template.name}</PageHeaderTitle>
157+
<PageHeaderSubtitle condensed>
158+
{template.description === ""
159+
? Language.noDescription
160+
: template.description}
161+
</PageHeaderSubtitle>
162+
</div>
179163
</Stack>
180164
</PageHeader>
181165
</Margins>
182166

183-
{!isLoading && (
184-
<div className={styles.tabs}>
185-
<Margins>
186-
<Stack direction="row" spacing={0.25}>
187-
<NavLink
188-
end
189-
to={`/templates/${template.name}`}
190-
className={({ isActive }) =>
191-
combineClasses([
192-
styles.tabItem,
193-
isActive ? styles.tabItemActive : undefined,
194-
])
195-
}
196-
>
197-
Summary
198-
</NavLink>
199-
<NavLink
200-
to={`/templates/${template.name}/permissions`}
201-
className={({ isActive }) =>
202-
combineClasses([
203-
styles.tabItem,
204-
isActive ? styles.tabItemActive : undefined,
205-
])
206-
}
207-
>
208-
Permissions
209-
</NavLink>
210-
</Stack>
211-
</Margins>
212-
</div>
213-
)}
167+
<div className={styles.tabs}>
168+
<Margins>
169+
<Stack direction="row" spacing={0.25}>
170+
<NavLink
171+
end
172+
to={`/templates/${template.name}`}
173+
className={({ isActive }) =>
174+
combineClasses([
175+
styles.tabItem,
176+
isActive ? styles.tabItemActive : undefined,
177+
])
178+
}
179+
>
180+
Summary
181+
</NavLink>
182+
<NavLink
183+
to={`/templates/${template.name}/permissions`}
184+
className={({ isActive }) =>
185+
combineClasses([
186+
styles.tabItem,
187+
isActive ? styles.tabItemActive : undefined,
188+
])
189+
}
190+
>
191+
Permissions
192+
</NavLink>
193+
</Stack>
194+
</Margins>
195+
</div>
214196

215197
<Margins>
216198
<TemplateLayoutContext.Provider

‎site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPage.tsx

+2-6
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import { Helmet } from "react-helmet-async"
1313
import { pageTitle } from "util/page"
1414
import { templateACLMachine } from "xServices/template/templateACLXService"
1515
import { TemplatePermissionsPageView } from "./TemplatePermissionsPageView"
16-
import { Loader } from "components/Loader/Loader"
1716

1817
export const TemplatePermissionsPage: FC<
1918
React.PropsWithChildren<unknown>
@@ -26,14 +25,11 @@ export const TemplatePermissionsPage: FC<
2625
context: { templateId: template?.id },
2726
})
2827
const { templateACL, userToBeUpdated, groupToBeUpdated } = state.context
29-
if (!template || !permissions) {
30-
return <Loader />
31-
}
3228

3329
return (
3430
<>
3531
<Helmet>
36-
<title>{pageTitle(`${template.name} · Permissions`)}</title>
32+
<title>{pageTitle(`${template?.name} · Permissions`)}</title>
3733
</Helmet>
3834
<ChooseOne>
3935
<Cond condition={!isTemplateRBACEnabled}>
@@ -68,7 +64,7 @@ export const TemplatePermissionsPage: FC<
6864
<TemplatePermissionsPageView
6965
organizationId={organizationId}
7066
templateACL={templateACL}
71-
canUpdatePermissions={permissions.canUpdateTemplate}
67+
canUpdatePermissions={Boolean(permissions?.canUpdateTemplate)}
7268
onAddUser={(user, role, reset) => {
7369
send("ADD_USER", { user, role, onDone: reset })
7470
}}

‎site/src/pages/TemplatePage/TemplateSummaryPage/TemplateSummaryPage.test.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe("TemplateSummaryPage", () => {
3838

3939
renderPage()
4040
await screen.findByText(MockTemplate.name)
41-
screen.getByTestId("markdown")
41+
await screen.findByTestId("markdown")
4242
screen.getByText(MockWorkspaceResource.name)
4343
screen.queryAllByText(`${MockTemplateVersion.name}`).length
4444
})

0 commit comments

Comments
 (0)