Skip to content

refactor: Refactor auth provider #5782

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

Merged
merged 18 commits into from
Jan 20, 2023
Merged

refactor: Refactor auth provider #5782

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
244845f
Add paywall to the audit route
BrunoQuaresma Jan 18, 2023
b83f9c2
refactor auth provider
BrunoQuaresma Jan 18, 2023
528892f
Move router to be inside of AppRouter
BrunoQuaresma Jan 18, 2023
9f0e0ee
Create dashboard provider
BrunoQuaresma Jan 18, 2023
1714ca5
Minor refactoring
BrunoQuaresma Jan 18, 2023
884dac7
Fix tests
BrunoQuaresma Jan 18, 2023
970e1a5
Merge branch 'bq/refactor-audit-route' into bq/refactor-auth-provider-2
BrunoQuaresma Jan 18, 2023
6e572e3
Merge branch 'main' of github.com:coder/coder into bq/refactor-auth-p…
BrunoQuaresma Jan 19, 2023
6442b3a
Fix Audit Log page test
BrunoQuaresma Jan 19, 2023
5747590
Add long time running test
BrunoQuaresma Jan 19, 2023
7768e95
Fix stories
BrunoQuaresma Jan 19, 2023
1173d16
Increase timeout
BrunoQuaresma Jan 19, 2023
ea4300b
Increse timeout
BrunoQuaresma Jan 19, 2023
d8826c3
Update site/src/components/WorkspaceStats/WorkspaceStats.stories.tsx
BrunoQuaresma Jan 19, 2023
e12f437
Update site/src/components/AuthProvider/AuthProvider.tsx
BrunoQuaresma Jan 19, 2023
9e7a875
Fix format
BrunoQuaresma Jan 19, 2023
3e589f1
Improve name
BrunoQuaresma Jan 19, 2023
dce50e3
Extract audit log paywall
BrunoQuaresma Jan 19, 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
3 changes: 3 additions & 0 deletions site/src/pages/AuditPage/AuditPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
getPaginationContext,
nonInitialPage,
} from "components/PaginationWidget/utils"
import { useFeatureVisibility } from "hooks/useFeatureVisibility"
import { FC } from "react"
import { Helmet } from "react-helmet-async"
import { useSearchParams } from "react-router-dom"
Expand All @@ -27,6 +28,7 @@ const AuditPage: FC = () => {

const { auditLogs, count } = auditState.context
const paginationRef = auditState.context.paginationRef as PaginationMachineRef
const { audit_log: isAuditLogVisible } = useFeatureVisibility()

return (
<>
Expand All @@ -42,6 +44,7 @@ const AuditPage: FC = () => {
}}
paginationRef={paginationRef}
isNonInitialPage={nonInitialPage(searchParams)}
isAuditLogVisible={isAuditLogVisible}
/>
</>
)
Expand Down
121 changes: 81 additions & 40 deletions site/src/pages/AuditPage/AuditPageView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import Button from "@material-ui/core/Button"
import Link from "@material-ui/core/Link"
import Table from "@material-ui/core/Table"
import TableBody from "@material-ui/core/TableBody"
import TableCell from "@material-ui/core/TableCell"
Expand All @@ -8,12 +10,14 @@ import { AuditLogRow } from "components/AuditLogRow/AuditLogRow"
import { ChooseOne, Cond } from "components/Conditionals/ChooseOne"
import { EmptyState } from "components/EmptyState/EmptyState"
import { Margins } from "components/Margins/Margins"
import ArrowRightAltOutlined from "@material-ui/icons/ArrowRightAltOutlined"
import {
PageHeader,
PageHeaderSubtitle,
PageHeaderTitle,
} from "components/PageHeader/PageHeader"
import { PaginationWidget } from "components/PaginationWidget/PaginationWidget"
import { Paywall } from "components/Paywall/Paywall"
import { SearchBarWithFilter } from "components/SearchBarWithFilter/SearchBarWithFilter"
import { Stack } from "components/Stack/Stack"
import { TableLoader } from "components/TableLoader/TableLoader"
Expand Down Expand Up @@ -46,6 +50,7 @@ export interface AuditPageViewProps {
onFilter: (filter: string) => void
paginationRef: PaginationMachineRef
isNonInitialPage: boolean
isAuditLogVisible: boolean
}

export const AuditPageView: FC<AuditPageViewProps> = ({
Expand All @@ -55,6 +60,7 @@ export const AuditPageView: FC<AuditPageViewProps> = ({
onFilter,
paginationRef,
isNonInitialPage,
isAuditLogVisible,
}) => {
const { t } = useTranslation("auditLog")
const isLoading = auditLogs === undefined || count === undefined
Expand All @@ -72,53 +78,88 @@ export const AuditPageView: FC<AuditPageViewProps> = ({
<PageHeaderSubtitle>{Language.subtitle}</PageHeaderSubtitle>
</PageHeader>

<SearchBarWithFilter
docs="https://coder.com/docs/coder-oss/latest/admin/audit-logs#filtering-logs"
filter={filter}
onFilter={onFilter}
presetFilters={presetFilters}
/>
<ChooseOne>
Copy link
Member

Choose a reason for hiding this comment

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

I would break these two outcomes out into separate files: one for the case where audit logs should be hidden, and one for their visibility.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What benefits do you see on that?

Copy link
Member

Choose a reason for hiding this comment

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

Great question! A couple:

  1. less scrolling through long files
  2. easier to read conditionals, e.g:
<ChooseOne>
    <Cond condition={bool}>
        <OptionAComponent />
    </Cond>
   <Cond condition={bool}>
        <OptionBComponent />
   </Cond>
</ChooseOne>

I know we have a lot of longer files in our repo but this just isn't a pattern I've seen anywhere else. Many companies turn on lint rules to limit file length.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thx, it definitely makes easier to read conditionals. About long files I personally like them haha it sounds a tooling thing since you can collapse the root functions in the editor tho. What I usually think about it is, if the component or code is only used in that file, it should be on the file so it is easier to find, modify, delete and I know it only affects that scope. When it has your own file, I assume it has been used in more than one place so I have to be more carefully.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But no strong opinion, I know a lot of people don't like it 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure thing!

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I also like shorter files because I feel less likely to overlook something. I think the concern about stuff that's reused is important but that we can put everything related in one folder, like Kira says. I won't be heartbroken either way though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lets go with shorter files!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another thought, is it ok just to move the Paywall? I would avoid to have one more level of prop drilling

Copy link
Member

Choose a reason for hiding this comment

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

Ya, definitely!

<Cond condition={isAuditLogVisible}>
<SearchBarWithFilter
docs="https://coder.com/docs/coder-oss/latest/admin/audit-logs#filtering-logs"
filter={filter}
onFilter={onFilter}
presetFilters={presetFilters}
/>

<TableContainer>
<Table>
<TableBody>
<ChooseOne>
<Cond condition={isLoading}>
<TableLoader />
</Cond>
<Cond condition={isEmpty}>
<TableContainer>
<Table>
<TableBody>
<ChooseOne>
<Cond condition={isNonInitialPage}>
<TableRow>
<TableCell colSpan={999}>
<EmptyState message={t("table.emptyPage")} />
</TableCell>
</TableRow>
<Cond condition={isLoading}>
<TableLoader />
</Cond>
<Cond condition={isEmpty}>
<ChooseOne>
<Cond condition={isNonInitialPage}>
<TableRow>
<TableCell colSpan={999}>
<EmptyState message={t("table.emptyPage")} />
</TableCell>
</TableRow>
</Cond>
<Cond>
<TableRow>
<TableCell colSpan={999}>
<EmptyState message={t("table.noLogs")} />
</TableCell>
</TableRow>
</Cond>
</ChooseOne>
</Cond>
<Cond>
<TableRow>
<TableCell colSpan={999}>
<EmptyState message={t("table.noLogs")} />
</TableCell>
</TableRow>
{auditLogs && (
<Timeline
items={auditLogs}
getDate={(log) => new Date(log.time)}
row={(log) => (
<AuditLogRow key={log.id} auditLog={log} />
)}
/>
)}
</Cond>
</ChooseOne>
</Cond>
<Cond>
{auditLogs && (
<Timeline
items={auditLogs}
getDate={(log) => new Date(log.time)}
row={(log) => <AuditLogRow key={log.id} auditLog={log} />}
/>
)}
</Cond>
</ChooseOne>
</TableBody>
</Table>
</TableContainer>
</TableBody>
</Table>
</TableContainer>

<PaginationWidget numRecords={count} paginationRef={paginationRef} />
</Cond>

<PaginationWidget numRecords={count} paginationRef={paginationRef} />
<Cond>
<Paywall
message="Audit logs"
description="Audit Logs allows Auditors to monitor user operations in their deployment. To use this feature, you have to upgrade your account."
cta={
<Stack direction="row" alignItems="center">
<Link
underline="none"
href="https://coder.com/docs/coder-oss/latest/admin/upgrade"
target="_blank"
rel="noreferrer"
>
<Button size="small" startIcon={<ArrowRightAltOutlined />}>
See how to upgrade
</Button>
</Link>
<Link
underline="none"
href="https://coder.com/docs/coder-oss/latest/admin/audit-logs"
target="_blank"
rel="noreferrer"
>
Read the docs
</Link>
</Stack>
}
/>
</Cond>
</ChooseOne>
</Margins>
)
}