ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

introduce safelyFetch; add some polish to /accounts 

test plan:
  - ensure that /accounts still lists accounts

Change-Id: Ib8dee9d83afb1bbde7631b4da6240442759dc585
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/354990
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Aaron Shafovaloff <ashafovaloff@instructure.com>
Product-Review: Aaron Shafovaloff <ashafovaloff@instructure.com>
This commit is contained in:
Aaron Shafovaloff 2024-08-14 12:12:59 -06:00
parent 9a29bc546b
commit 2d03324bd4
18 changed files with 319 additions and 176 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/views/accounts/index.html.erb
View File

@ -19,10 +19,4 @@
<% provide :page_title do %><%= t(:page_title, "Accounts") %><% end %>
<% add_crumb t(:accounts_crumb, "Accounts") %>
<% provide :right_side do %>
<% end %>
<% js_bundle :account_manage %>
<h1><%= t(:title, "Accounts I Manage") %></h1>
<div id="context_list"></div>

4
ui/boot/initializers/router.tsx
View File

@ -41,6 +41,10 @@ const portalRouter = createBrowserRouter(
path="/users/:userId/masquerade"
lazy={() => import('../../features/act_as_modal/react/ActAsModalRoute')}
/>
<Route
path="/accounts"
lazy={() => import('../../features/account_manage/react/AccountListRoute')}
/>
{accountGradingSettingsRoutes}

1
ui/engine/engine.d.ts vendored
View File

@ -17,7 +17,6 @@
*/
declare module '@canvas/i18n'
declare module '@canvas/do-fetch-api-effect'
// a little disappointed this has to be done by hand
declare namespace Intl {

1
ui/featureBundles.ts
View File

@ -24,7 +24,6 @@ const featureBundles: {
account_calendar_settings: () => import('./features/account_calendar_settings/index'),
account_course_user_search: () => import('./features/account_course_user_search/index'),
account_grading_standards: () => import('./features/account_grading_standards/index'),
account_manage: () => import('./features/account_manage/index'),
account_notification_settings: () => import('./features/account_notification_settings/index'),
account_search: () => import('./features/account_search/index'),
account_settings: () => import('./features/account_settings/index'),

154
ui/features/account_manage/react/AccountList.tsx
View File

@ -16,79 +16,113 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React, {useState, useEffect} from 'react'
import React, {useState} from 'react'
import errorShipUrl from '@canvas/images/ErrorShip.svg'
import {Spinner} from '@instructure/ui-spinner'
import {AccountNavigation} from './AccountNavigation'
import {showFlashError} from '@canvas/alerts/react/FlashAlert'
import {useScope as useI18nScope} from '@canvas/i18n'
import doFetchApi from '@canvas/do-fetch-api-effect'
import {View} from '@instructure/ui-view'
import getAccounts from '@canvas/api/accounts/getAccounts'
import {useQuery} from '@canvas/query'
import {IconSettingsLine} from '@instructure/ui-icons'
import GenericErrorPage from '@canvas/generic-error-page/react'
import {Table} from '@instructure/ui-table'
import {IconButton} from '@instructure/ui-buttons'
const I18n = useI18nScope('account_manage')
const ACC_PER_PAGE = 50
interface Props {
readonly pageIndex: number
readonly onPageClick: (page: number) => void
const ErrorPage = ({error}: {error?: unknown}) => {
return (
<GenericErrorPage
imageUrl={errorShipUrl}
errorSubject={I18n.t('Accounts initial query error')}
errorCategory={I18n.t('Accounts Error Page')}
errorMessage={error instanceof Error ? error?.message : ''}
/>
)
}
export function AccountList(props: Props) {
const [accounts, setAccounts] = useState([])
const [load, setLoad] = useState(false)
const [last, setLast] = useState(0)
const [error, setError] = useState(false)
export function AccountList() {
const [pageIndex, setPageIndex] = useState(1)
useEffect(() => {
const loadAccounts = async () => {
setLoad(false)
try {
const {json, link} = await doFetchApi({
path: '/api/v1/accounts?per_page=' + ACC_PER_PAGE + '&page=' + props.pageIndex,
method: 'GET',
})
if (json !== undefined && link !== undefined) {
setLoad(true)
setError(false)
setAccounts(json)
const lastPage = parseInt(link?.last?.page, 10)
setLast(lastPage)
}
} catch (err) {
showFlashError(I18n.t('Accounts could not be loaded'))
setLoad(true)
setError(true)
}
}
loadAccounts()
}, [props.pageIndex])
const {data, error, isLoading, isError} = useQuery({
queryKey: ['accounts', {pageIndex}],
queryFn: getAccounts,
fetchAtLeastOnce: true,
})
if (!load) {
return (
<div>
<Spinner renderTitle={I18n.t('Loading')} margin="large auto 0 auto" />
</div>
)
const last = parseInt(String(data?.link?.last?.page || ''), 10)
if (isError) {
return <ErrorPage error={error} />
}
if (error) {
return <div />
} else {
if (isLoading) {
return (
<div>
<ul>
{accounts.length > 0
? accounts.map((item: any) => (
<li key={item.id}>
<a href={'/accounts/' + item.id}>{item.name}</a>
</li>
))
: null}
</ul>
<AccountNavigation
currentPage={props.pageIndex}
onPageClick={props.onPageClick}
pageCount={last}
<View>
<Spinner
renderTitle={I18n.t('Loading')}
size="small"
delay={500}
margin="large auto 0 auto"
/>
</div>
</View>
)
}
const accounts = data?.json
return (
<View>
<Table caption={I18n.t('Accounts')}>
<Table.Head>
<Table.Row>
<Table.ColHeader id="name-header" width="65%">
{I18n.t('Name')}
</Table.ColHeader>
<Table.ColHeader id="sub_account_count-header" width="15%">
{I18n.t('Subaccounts')}
</Table.ColHeader>
<Table.ColHeader id="course_count-header" width="15%">
{I18n.t('Courses')}
</Table.ColHeader>
<Table.ColHeader id="settings-header" width="5%">
{I18n.t('Settings')}
</Table.ColHeader>
</Table.Row>
</Table.Head>
<Table.Body>
{accounts?.map(account => {
const settingsTip = I18n.t('Settings for %{name}', {name: account.name})
return (
<Table.Row key={account.id}>
<Table.Cell>
<a href={`/accounts/${account.id}`}>{account.name}</a>
</Table.Cell>
<Table.Cell>
<a href={`/accounts/${account.id}/sub_accounts`}>{account.sub_account_count}</a>
</Table.Cell>
<Table.Cell>{account.course_count}</Table.Cell>
<Table.Cell textAlign="end">
<IconButton
withBorder={false}
withBackground={false}
size="small"
href={`/accounts/${account.id}/settings#tab-settings`}
screenReaderLabel={settingsTip}
>
<IconSettingsLine title={settingsTip} />
</IconButton>
</Table.Cell>
</Table.Row>
)
})}
</Table.Body>
</Table>
{last > 1 && (
<AccountNavigation currentPage={pageIndex} onPageClick={setPageIndex} pageCount={last} />
)}
</View>
)
}

20
ui/features/account_manage/react/AccountContainer.tsx → ui/features/account_manage/react/AccountListRoute.tsx
View File

@ -16,18 +16,18 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React, {useState} from 'react'
import React from 'react'
import {Portal} from '@instructure/ui-portal'
import {AccountList} from './AccountList'
export function AccountContainer() {
const [pageIndex, setPageIndex] = useState(1)
export function Component() {
const mountPoint: HTMLElement | null = document.querySelector('#context_list')
if (!mountPoint) {
return null
}
return (
<AccountList
pageIndex={pageIndex}
onPageClick={page => {
setPageIndex(page)
}}
/>
<Portal open={true} mountNode={mountPoint}>
<AccountList />
</Portal>
)
}

17
ui/features/account_manage/react/AccountNavigation.tsx
View File

@ -19,6 +19,7 @@
import React from 'react'
import {useScope as useI18nScope} from '@canvas/i18n'
import {Pagination} from '@instructure/ui-pagination'
import {View} from '@instructure/ui-view'
const I18n = useI18nScope('account_manage')
const {Page} = Pagination as any
@ -47,12 +48,14 @@ export function AccountNavigation(props: Props) {
}
return (
<Pagination
labelNext={I18n.t('Next Page')}
labelPrev={I18n.t('Previous Page')}
variant="compact"
>
{pageButtons}
</Pagination>
<View padding="0 x-small">
<Pagination
labelNext={I18n.t('Next Page')}
labelPrev={I18n.t('Previous Page')}
variant="compact"
>
{pageButtons}
</Pagination>
</View>
)
}

57
ui/features/account_manage/react/__tests__/AccountList.test.tsx
View File

@ -19,37 +19,68 @@
import React from 'react'
import {render, waitFor} from '@testing-library/react'
import {AccountList} from '../AccountList'
import {QueryProvider, queryClient} from '@canvas/query'
import fetchMock from 'fetch-mock'
describe('AccountLists', () => {
const props = {onPageClick: jest.fn()}
const accountFixture = {
id: '1',
name: 'acc1',
course_count: 1,
sub_account_count: 1,
workflow_state: 'active',
parent_account_id: null,
root_account_id: null,
uuid: '2675186350fe410fb1247a4b911deef4',
default_storage_quota_mb: 500,
default_user_storage_quota_mb: 50,
default_group_storage_quota_mb: 50,
default_time_zone: 'America/Denver',
}
afterEach(() => {
describe('AccountLists', () => {
beforeEach(() => {
fetchMock.restore()
})
it('makes an API call when page loads', async () => {
fetchMock.get('/api/v1/accounts?per_page=30&page=1', [{id: '1', name: 'acc1'}])
const {queryAllByText} = render(<AccountList {...props} pageIndex={1} />)
fetchMock.get('/api/v1/accounts?include=course_count,sub_account_count&per_page=50&page=1', [
accountFixture,
])
const {queryAllByText} = render(
<QueryProvider>
<AccountList />
</QueryProvider>
)
await waitFor(() => expect(queryAllByText('acc1')).toBeTruthy())
})
it('renders an error message when loading accounts fails', async () => {
fetchMock.get('/api/v1/accounts?per_page=30&page=1', () => {
throw Object.assign(new Error('error'), {code: 402})
})
const {queryAllByText} = render(<AccountList {...props} pageIndex={1} />)
fetchMock.get(
'/api/v1/accounts?include=course_count,sub_account_count&per_page=30&page=1',
() => {
throw Object.assign(new Error('error'), {code: 402})
}
)
const {queryAllByText} = render(
<QueryProvider>
<AccountList />
</QueryProvider>
)
await waitFor(() => expect(queryAllByText('Accounts could not be found')).toBeTruthy())
})
it('renders when the API does not return the last page', async () => {
fetchMock.get('/api/v1/accounts?per_page=100&page=1', {
body: [{id: '1', name: 'acc1'}],
fetchMock.get('/api/v1/accounts?include=course_count,sub_account_count&per_page=100&page=1', {
body: [accountFixture],
headers: {
link: '</api/v1/accounts?page=1&per_page=100>; rel="current"',
link: '</api/v1/accounts?include=course_count,sub_account_countpage=1&per_page=100>; rel="current"',
},
})
const {queryAllByText} = render(<AccountList {...props} pageIndex={1} />)
const {queryAllByText} = render(
<QueryProvider>
<AccountList />
</QueryProvider>
)
await waitFor(() => expect(queryAllByText('acc1')).toBeTruthy())
})
})

4
ui/features/navigation_header/react/OldSideNav.tsx
View File

@ -27,11 +27,11 @@ import tourPubSub from '@canvas/tour-pubsub'
import {getTrayLabel, getTrayPortal} from './utils'
import useHoverIntent from './hooks/useHoverIntent'
import coursesQuery from './queries/coursesQuery'
import accountsQuery from './queries/accountsQuery'
import groupsQuery from './queries/groupsQuery'
import NavigationBadges from './NavigationBadges'
import {prefetchQuery} from '@canvas/query'
import profileQuery from './queries/profileQuery'
import getAccounts from '@canvas/api/accounts/getAccounts'
const I18n = useI18nScope('Navigation')
@ -100,7 +100,7 @@ const Navigation = () => {
useHoverIntent(accountsNavLink, () => {
import('./trays/AccountsTray')
prefetchQuery(['accounts'], accountsQuery)
prefetchQuery(['accounts', {pageIndex: 1}], getAccounts)
})
useHoverIntent(groupsNavLink, () => {

12
ui/features/navigation_header/react/lists/AccountsList.tsx
View File

@ -23,17 +23,19 @@ import {Link} from '@instructure/ui-link'
import {useQuery} from '@canvas/query'
import {Spinner} from '@instructure/ui-spinner'
import {ActiveText} from './utils'
import accountsQuery from '../queries/accountsQuery'
import getAccounts from '@canvas/api/accounts/getAccounts'
const I18n = useI18nScope('CoursesTray')
export default function CoursesList() {
export default function AccountsList() {
const {data, isLoading, isSuccess} = useQuery({
queryKey: ['accounts'],
queryFn: accountsQuery,
queryKey: ['accounts', {pageIndex: 1}],
queryFn: getAccounts,
fetchAtLeastOnce: true,
})
const accounts = data?.json || []
return (
<List isUnstyled={true} itemSpacing="small" margin="0 0 0 x-large">
{isLoading && (
@ -49,7 +51,7 @@ export default function CoursesList() {
</Link>
</List.Item>,
].concat(
data.map(account => (
accounts.map(account => (
<List.Item key={account.id}>
<Link href={`/accounts/${account.id}`} isWithinText={false} display="block">
<ActiveText url={`/accounts/${account.id}`}>{account.name}</ActiveText>

38
ui/features/navigation_header/react/queries/accountsQuery.ts
View File

@ -1,38 +0,0 @@
/*
* Copyright (C) 2023 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import doFetchApi from '@canvas/do-fetch-api-effect'
import type {QueryFunctionContext} from '@tanstack/react-query'
import type {Account} from '../../../../api.d'
const ACCOUNTS_PATH = '/api/v1/accounts'
export default async function accountsQuery({signal}: QueryFunctionContext): Promise<Account[]> {
const data: Array<Account> = []
const fetchOpts = {signal}
let path = ACCOUNTS_PATH
while (path) {
// eslint-disable-next-line no-await-in-loop
const {json, link} = await doFetchApi<Account[]>({path, fetchOpts})
if (json) data.push(...json)
path = link?.next?.url || null
}
return data
}

11
ui/features/navigation_header/react/trays/AccountsTray.tsx
View File

@ -24,14 +24,13 @@ import {Heading} from '@instructure/ui-heading'
import {Spinner} from '@instructure/ui-spinner'
import {Link} from '@instructure/ui-link'
import {useQuery} from '@canvas/query'
import accountsQuery from '../queries/accountsQuery'
import type {Account} from '../../../../api.d'
import getAccounts from '@canvas/api/accounts/getAccounts'
const I18n = useI18nScope('AccountsTray')
export default function AccountsTray() {
const {data, isLoading, isSuccess} = useQuery<Account[], Error>({
queryKey: ['accounts'],
queryFn: accountsQuery,
const {data, isLoading, isSuccess} = useQuery({
queryKey: ['accounts', {pageIndex: 1}],
queryFn: getAccounts,
fetchAtLeastOnce: true,
})
@ -56,7 +55,7 @@ export default function AccountsTray() {
</List.Item>
)}
{isSuccess &&
data.map(account => (
data?.json?.map(account => (
<List.Item key={account.id}>
<Link isWithinText={false} href={`/accounts/${account.id}`}>
{account.name}

24
ui/features/navigation_header/react/trays/__tests__/AccountsTray.test.tsx
View File

@ -25,19 +25,21 @@ const render = (children: unknown) =>
testingLibraryRender(<QueryProvider>{children}</QueryProvider>)
describe('AccountsTray', () => {
const accounts = [
{
id: '1',
name: 'Account1',
},
{
id: '2',
name: 'Account2',
},
]
const data = {
json: [
{
id: '1',
name: 'Account1',
},
{
id: '2',
name: 'Account2',
},
],
}
beforeEach(() => {
queryClient.setQueryData(['accounts'], accounts)
queryClient.setQueryData(['accounts', {pageIndex: 1}], data)
})
afterEach(() => {

51
ui/shared/api/accounts/getAccounts.ts Normal file
View File

@ -0,0 +1,51 @@
/*
* Copyright (C) 2024 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import {safelyFetch} from '@canvas/do-fetch-api-effect'
import {ZAccount} from '@canvas/schemas'
import type {QueryFunctionContext} from '@tanstack/react-query'
import {z} from 'zod'
const ZAccountWithCounts = ZAccount.extend({
course_count: z.number(),
sub_account_count: z.number(),
}).strict()
const ZAccounts = z.array(ZAccountWithCounts)
export type Accounts = z.infer<typeof ZAccounts>
const ACCOUNTS_PATH = '/api/v1/accounts'
const ACC_PER_PAGE = 50
export default async function getAccounts({queryKey, signal}: QueryFunctionContext) {
if (!Array.isArray(queryKey) || typeof queryKey[1] !== 'object') {
throw new Error('Invalid query key')
}
const pageIndex = queryKey[1].pageIndex
const {json, link} = await safelyFetch(
{
path: `${ACCOUNTS_PATH}?include=course_count,sub_account_count&per_page=${ACC_PER_PAGE}&page=${pageIndex}`,
method: 'GET',
signal,
},
ZAccounts
)
return {json, link}
}

7
ui/shared/api/package.json Normal file
View File

@ -0,0 +1,7 @@
{
"name": "@canvas/api",
"private": true,
"version": "1.0.0",
"author": "neme",
"owner": "Instructure"
}

50
ui/shared/do-fetch-api-effect/index.ts
View File

@ -1,4 +1,4 @@
// @ts-nocheck
/* eslint-disable no-console */
/*
* Copyright (C) 2019 - present Instructure, Inc.
*
@ -22,6 +22,9 @@ import parseLinkHeader, {type Links} from '@canvas/parse-link-header'
import {defaultFetchOptions} from '@canvas/util/xhr'
import {toQueryString} from '@canvas/query-string-encoding'
import type {QueryParameterRecord} from '@canvas/query-string-encoding'
import z from 'zod'
type RequestCredentials = 'include' | 'omit' | 'same-origin'
function constructRelativeUrl({
path,
@ -32,7 +35,7 @@ function constructRelativeUrl({
}): string {
const queryString = toQueryString(params)
if (queryString.length === 0) return path
return path + '?' + queryString
return `${path}?${queryString}`
}
// https://fetch.spec.whatwg.org/#requestinit
@ -48,6 +51,7 @@ export type DoFetchApiOpts = {
// eslint-disable-next-line no-undef
body?: BodyInit
fetchOpts?: RequestInit
signal?: AbortSignal
}
export type DoFetchApiResults<T> = {
@ -56,15 +60,14 @@ export type DoFetchApiResults<T> = {
link?: Links
}
// NOTE: we do NOT deep-merge customFetchOptions.headers, they should be passed
// in the headers arg instead.
export default async function doFetchApi<T = unknown>({
path,
method = 'GET',
headers = {},
params = {},
body,
fetchOpts = {},
signal,
fetchOpts = {}, // we do not deep-merge fetchOptions.headers
}: DoFetchApiOpts): Promise<DoFetchApiResults<T>> {
const finalFetchOptions = {...defaultFetchOptions()}
finalFetchOptions.headers['X-CSRF-Token'] = getCookie('_csrf_token')
@ -81,7 +84,7 @@ export default async function doFetchApi<T = unknown>({
body,
method,
...finalFetchOptions,
// eslint-disable-next-line no-undef
signal,
credentials: finalFetchOptions.credentials as RequestCredentials,
})
if (!response.ok) {
@ -97,3 +100,38 @@ export default async function doFetchApi<T = unknown>({
const json = text.length > 0 ? (JSON.parse(text) as T) : undefined
return {json, response, link}
}
export type SafelyFetchResults<T> = {
json?: T
response: Response
link?: Links
}
export async function safelyFetch<T = unknown>(
{path, method = 'GET', headers = {}, params = {}, signal, body}: DoFetchApiOpts,
schema: z.Schema<T>
): Promise<SafelyFetchResults<T>> {
if (!schema) {
throw new Error('safelyFetch requires a schema')
}
const {json, response, link} = await doFetchApi<T>({path, method, headers, params, signal, body})
if (process.env.NODE_ENV !== 'production') {
try {
schema.parse(json)
} catch (err) {
if (err instanceof z.ZodError) {
console.group(`Zod parsing error for ${path}`)
for (const issue of err.issues) {
console.error(`Error at ${issue.path.join('.')} - ${issue.message}`)
}
console.groupEnd()
throw err
}
}
}
return {json, response, link}
}

32
ui/features/account_manage/index.tsx → ui/shared/schemas/index.ts
View File

@ -1,5 +1,5 @@
/*
* Copyright (C) 2023 - present Instructure, Inc.
* Copyright (C) 2024 - present Instructure, Inc.
*
* This file is part of Canvas.
*
@ -16,13 +16,25 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React from 'react'
import ReactDOM from 'react-dom'
import {AccountContainer} from './react/AccountContainer'
import ready from '@instructure/ready'
import {z} from 'zod'
ready(() => {
if (document.getElementById('context_list') !== null) {
ReactDOM.render(<AccountContainer />, document.getElementById('context_list'))
}
})
export const ZAccount = z
.object({
id: z.string(),
name: z.string(),
workflow_state: z.string(),
parent_account_id: z.string().nullable(),
root_account_id: z.string().nullable(),
uuid: z.string(),
default_storage_quota_mb: z.number(),
default_user_storage_quota_mb: z.number(),
default_group_storage_quota_mb: z.number(),
default_time_zone: z.string(),
})
.strict()
export type Account = z.infer<typeof ZAccount>
export const ZAccounts = z.array(ZAccount)
export type Accounts = z.infer<typeof ZAccounts>

6
ui/shared/schemas/package.json Normal file
View File

@ -0,0 +1,6 @@
{
"name": "@canvas/schemas",
"private": true,
"version": "1.0.0",
"author": "Instructure, Inc."
}