- Notifications
You must be signed in to change notification settings - Fork1k
feat: display specific errors if templates page fails#4023
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
36cb15ee1dbcd97c788ab380d6ad302b8532d24806883ddc1d01a45de31e658f845af1File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,13 @@ | ||
| importauditLogfrom"./auditLog.json" | ||
| importcommonfrom"./common.json" | ||
| importtemplatePagefrom"./templatePage.json" | ||
| importtemplatesPagefrom"./templatesPage.json" | ||
| importworkspacePagefrom"./workspacePage.json" | ||
| exportconsten={ | ||
| common, | ||
| workspacePage, | ||
| auditLog, | ||
| templatePage, | ||
| templatesPage, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "errors": { | ||
| "getOrganizationError": "Something went wrong fetching organizations.", | ||
| "getTemplatesError": "Something went wrong fetching templates." | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -6,23 +6,24 @@ import { XServiceContext } from "../../xServices/StateContext" | ||
| import{templatesMachine}from"../../xServices/templates/templatesXService" | ||
| import{TemplatesPageView}from"./TemplatesPageView" | ||
| exportconstTemplatesPage:React.FC=()=>{ | ||
Member
| ||
| constxServices=useContext(XServiceContext) | ||
| const[authState]=useActor(xServices.authXService) | ||
| const[templatesState]=useMachine(templatesMachine) | ||
| const{ templates, getOrganizationsError, getTemplatesError}=templatesState.context | ||
| return( | ||
| <> | ||
| <Helmet> | ||
| <title>{pageTitle("Templates")}</title> | ||
| </Helmet> | ||
| <TemplatesPageView | ||
| templates={templates} | ||
| canCreateTemplate={authState.context.permissions?.createTemplates} | ||
| loading={templatesState.hasTag("loading")} | ||
| getOrganizationsError={getOrganizationsError} | ||
| getTemplatesError={getTemplatesError} | ||
| /> | ||
| </> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -8,7 +8,9 @@ import TableHead from "@material-ui/core/TableHead" | ||
| import TableRow from "@material-ui/core/TableRow" | ||
| import KeyboardArrowRight from "@material-ui/icons/KeyboardArrowRight" | ||
| import useTheme from "@material-ui/styles/useTheme" | ||
| import { ErrorSummary } from "components/ErrorSummary/ErrorSummary" | ||
| import { FC } from "react" | ||
| import { useTranslation } from "react-i18next" | ||
| import { useNavigate } from "react-router-dom" | ||
| import { createDayString } from "util/createDayString" | ||
| import { formatTemplateActiveDevelopers } from "util/templates" | ||
| @@ -77,12 +79,20 @@ export interface TemplatesPageViewProps { | ||
| loading?: boolean | ||
| canCreateTemplate?: boolean | ||
| templates?: TypesGen.Template[] | ||
| getOrganizationsError?: Error | unknown | ||
| getTemplatesError?: Error | unknown | ||
| } | ||
| export const TemplatesPageView: FC<React.PropsWithChildren<TemplatesPageViewProps>> = (props) => { | ||
| const styles = useStyles() | ||
| const navigate = useNavigate() | ||
| const { t } = useTranslation("templatesPage") | ||
| const theme: Theme = useTheme() | ||
| const empty = | ||
| !props.loading && | ||
| !props.getOrganizationsError && | ||
| !props.getTemplatesError && | ||
| !props.templates?.length | ||
| return ( | ||
| <Margins> | ||
| @@ -114,94 +124,110 @@ export const TemplatesPageView: FC<React.PropsWithChildren<TemplatesPageViewProp | ||
| )} | ||
| </PageHeader> | ||
| {props.getOrganizationsError ? ( | ||
| <ErrorSummary | ||
| error={props.getOrganizationsError} | ||
| defaultMessage={t("errors.getOrganizationsError")} | ||
| /> | ||
| ) : props.getTemplatesError ? ( | ||
| <ErrorSummary | ||
| error={props.getTemplatesError} | ||
| defaultMessage={t("errors.getTemplatesError")} | ||
| /> | ||
| ) : ( | ||
| <TableContainer> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I find this triple, JSX ternary a little confusing and personally think this is a good candidate for early return. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. How would you do the early return? I'm not seeing a good way of doing it since the margins and page header need to be there in each case. I could put those in a const but at that point I'd rather put the content (error or data) in a const and still use the conditional so you can easily see what's definitely there vs what's conditional. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I might move the margins and page header into I think the benefit here is we are writing state in a declarative way - it is apparent at a glance what states the developer should be aware of. Let me know what you think! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Yeah, this looks nice and readable, but the scaffolding is in the view for storybook reasons. I'm open to a refactor but I think I'll leave it for future work so I can get the error handling in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. sounds good! | ||
| <Table> | ||
| <TableHead> | ||
| <TableRow> | ||
| <TableCell width="50%">{Language.nameLabel}</TableCell> | ||
| <TableCell width="16%">{Language.usedByLabel}</TableCell> | ||
| <TableCell width="16%">{Language.lastUpdatedLabel}</TableCell> | ||
| <TableCell width="16%">{Language.createdByLabel}</TableCell> | ||
| <TableCell width="1%"></TableCell> | ||
| </TableRow> | ||
| </TableHead> | ||
| <TableBody> | ||
| {props.loading && <TableLoader />} | ||
| {empty ? ( | ||
| <TableRow> | ||
| <TableCell colSpan={999}> | ||
| <EmptyState | ||
| message={Language.emptyMessage} | ||
| description={ | ||
| props.canCreateTemplate | ||
| ? Language.emptyDescription | ||
| : Language.emptyViewNoPerms | ||
| } | ||
| descriptionClassName={styles.emptyDescription} | ||
| cta={<CodeExample code="coder templates init" />} | ||
| /> | ||
| </TableCell> | ||
| </TableRow> | ||
| ) : ( | ||
| props.templates?.map((template) => { | ||
| const templatePageLink = `/templates/${template.name}` | ||
| const hasIcon = template.icon && template.icon !== "" | ||
| return ( | ||
| <TableRow | ||
| key={template.id} | ||
| hover | ||
| data-testid={`template-${template.id}`} | ||
| tabIndex={0} | ||
| onKeyDown={(event) => { | ||
| if (event.key === "Enter") { | ||
| navigate(templatePageLink) | ||
| } | ||
| }} | ||
| className={styles.clickableTableRow} | ||
| > | ||
| <TableCellLink to={templatePageLink}> | ||
| <AvatarData | ||
| title={template.name} | ||
| subtitle={template.description} | ||
| highlightTitle | ||
| avatar={ | ||
| hasIcon && ( | ||
| <div className={styles.templateIconWrapper}> | ||
| <img alt="" src={template.icon} /> | ||
| </div> | ||
| ) | ||
| } | ||
| /> | ||
| </TableCellLink> | ||
| <TableCellLink to={templatePageLink}> | ||
| <span style={{ color: theme.palette.text.secondary }}> | ||
| {Language.developerCount(template.active_user_count)} | ||
| </span> | ||
| </TableCellLink> | ||
| <TableCellLink data-chromatic="ignore" to={templatePageLink}> | ||
| <span style={{ color: theme.palette.text.secondary }}> | ||
| {createDayString(template.updated_at)} | ||
| </span> | ||
| </TableCellLink> | ||
| <TableCellLink to={templatePageLink}> | ||
| <span style={{ color: theme.palette.text.secondary }}> | ||
| {template.created_by_name} | ||
| </span> | ||
| </TableCellLink> | ||
| <TableCellLink to={templatePageLink}> | ||
| <div className={styles.arrowCell}> | ||
| <KeyboardArrowRight className={styles.arrowRight} /> | ||
| </div> | ||
| </TableCellLink> | ||
| </TableRow> | ||
| ) | ||
| }) | ||
| )} | ||
| </TableBody> | ||
| </Table> | ||
| </TableContainer> | ||
| )} | ||
| </Margins> | ||
| ) | ||
| } | ||
Uh oh!
There was an error while loading.Please reload this page.