Skip to content

Commit

Permalink
feat(core): fix infinite loading in releases and create addon dataset…
Browse files Browse the repository at this point in the history
… if it doesn't exist (#7255)

* fix(core): update bundle stores to avoid infinite loading if no addon client or no bundles exists

* feat(core): create addon dataset when creating first release if it doesn't exist

* fix(releases): fixing issue loading metadata and controlling aggregator loading state (#7259)

* fix(releases): fixing issue with loading metadata and controlling the loading staet on metadata aggregator

* fix(releases): fixing failing ReleaseOverview tests after showing empty screen

* fix(core): noop bundle store state is initialising

* fix(core): update createBundlesStore loading state to use addon dataset ready

---------

Co-authored-by: pedrobonamin <[email protected]>

---------
Co-authored-by: Jordan Lawrence <[email protected]>
  • Loading branch information
3 people authored and bjoerge committed Aug 20, 2024
1 parent 48ca5c3 commit d787883
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 49 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {ArrowRightIcon} from '@sanity/icons'
import {Box, Button, Dialog, Flex} from '@sanity/ui'
import {Box, Button, Dialog, Flex, useToast} from '@sanity/ui'
import {type FormEvent, useCallback, useState} from 'react'

import {type BundleDocument} from '../../../store/bundles/types'
Expand All @@ -15,6 +15,7 @@ interface BundleDetailsDialogProps {

export function BundleDetailsDialog(props: BundleDetailsDialogProps): JSX.Element {
const {onCancel, onSubmit, bundle} = props
const toast = useToast()
const {createBundle, updateBundle} = useBundleOperations()
const [hasErrors, setHasErrors] = useState(false)
const formAction = bundle ? 'edit' : 'create'
Expand Down Expand Up @@ -66,18 +67,23 @@ export function BundleDetailsDialog(props: BundleDetailsDialogProps): JSX.Elemen
setIsSubmitting(true)
await bundleOperation(value)
setValue(value)
if (formAction === 'create') {
setPerspective(value.slug)
}
} catch (err) {
console.error(err)
toast.push({
closable: true,
status: 'error',
title: `Failed to ${formAction} release`,
})
} finally {
setIsSubmitting(false)
if (formAction === 'create') {
setPerspective(value.slug)
}
onSubmit()
}
}
},
[bundleOperation, formAction, onSubmit, setPerspective, value],
[bundleOperation, formAction, onSubmit, setPerspective, value, toast],
)

const handleOnChange = useCallback((changedValue: Partial<BundleDocument>) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export function ReleasesOverview() {
const bundleSlugs = useMemo(() => bundles?.map((bundle) => bundle.slug) || [], [bundles])
const {data: bundlesMetadata, loading: loadingBundlesMetadata} = useBundlesMetadata(bundleSlugs)
const loading = loadingBundles || loadingBundlesMetadata
const loadingTableData = loading || (!bundlesMetadata && Boolean(bundleSlugs.length))

const hasBundles = bundles && containsBundles(bundles)
const loadingOrHasBundles = loading || hasBundles

Expand Down Expand Up @@ -163,7 +165,7 @@ export function ReleasesOverview() {
{!loading && !hasBundles && (
<Container style={{margin: 0}} width={0}>
<Stack space={5}>
<Text muted size={2}>
<Text data-testid="no-bundles-info-text" muted size={2}>
Releases are collections of document versions which can be managed and
published together.
</Text>
Expand All @@ -176,18 +178,20 @@ export function ReleasesOverview() {
</Flex>
{loadingOrHasBundles && createReleaseButton}
</Flex>
<Table<TableBundle>
// for resetting filter and sort on table when mode changed
key={bundleGroupMode}
defaultSort={DEFAULT_RELEASES_OVERVIEW_SORT}
loading={loading}
data={groupedBundles[bundleGroupMode]}
columnDefs={releasesOverviewColumnDefs}
searchFilter={applySearchTermToBundles}
emptyState="No Releases"
rowId="_id"
rowActions={renderRowActions}
/>
{(hasBundles || loadingTableData) && (
<Table<TableBundle>
// for resetting filter and sort on table when mode changed
key={bundleGroupMode}
defaultSort={DEFAULT_RELEASES_OVERVIEW_SORT}
loading={loadingTableData}
data={groupedBundles[bundleGroupMode]}
columnDefs={releasesOverviewColumnDefs}
searchFilter={applySearchTermToBundles}
emptyState="No Releases"
rowId="_id"
rowActions={renderRowActions}
/>
)}
</Stack>
</Container>
{renderCreateBundleDialog()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ describe('ReleasesOverview', () => {
})
mockUseBundlesMetadata.mockReturnValue({
loading: true,
fetching: true,
error: null,
data: null,
})
Expand Down Expand Up @@ -81,7 +80,6 @@ describe('ReleasesOverview', () => {
})
mockUseBundlesMetadata.mockReturnValue({
loading: false,
fetching: false,
error: null,
data: null,
})
Expand All @@ -90,12 +88,12 @@ describe('ReleasesOverview', () => {
return render(<ReleasesOverview />, {wrapper})
})

it('shows a message that no bundles are available', () => {
screen.getByText('No Releases')
it('shows a message about bundles', () => {
screen.getByTestId('no-bundles-info-text')
})

it('does not allow for bundle search', () => {
expect(screen.getByPlaceholderText('Search releases')).toBeDisabled()
it('does not show the bundles table', () => {
expect(screen.queryByRole('table')).toBeNull()
})

it('does not show bundle history mode switch', () => {
Expand Down
9 changes: 3 additions & 6 deletions packages/sanity/src/core/releases/tool/useBundlesMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ export const useBundlesMetadata = (bundleSlugs: string[]) => {
const [responseData, setResponseData] = useState<Record<string, BundlesMetadata> | null>(null)

useEffect(() => {
addBundleSlugsToListener([...new Set(bundleSlugs)])
if (bundleSlugs.length) addBundleSlugsToListener([...new Set(bundleSlugs)])

return () => removeBundleSlugsFromListener([...new Set(bundleSlugs)])
}, [addBundleSlugsToListener, bundleSlugs, removeBundleSlugsFromListener])

const {data} = state
const {data, loading} = state

useEffect(() => {
if (!data) return
Expand All @@ -43,10 +43,7 @@ export const useBundlesMetadata = (bundleSlugs: string[]) => {
error: state.error,
// loading is only for initial load
// changing listened to bundle slugs will not cause a re-load
loading: !responseData,
// fetching is true when performing initial load for a given set of bundle metadata
// changing listened to bundle slugs will cause a re-fetch
fetching: state.loading,
loading,
data: responseData,
}
}
10 changes: 5 additions & 5 deletions packages/sanity/src/core/store/_legacy/datastores.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,22 +280,22 @@ export function useKeyValueStore(): KeyValueStore {
export function useBundlesStore(): BundlesStore {
const resourceCache = useResourceCache()
const workspace = useWorkspace()
const {client: addOnClient} = useAddonDataset()
const {client: addonClient, ready} = useAddonDataset()
const studioClient = useClient(DEFAULT_STUDIO_CLIENT_OPTIONS)

return useMemo(() => {
const bundlesStore =
resourceCache.get<BundlesStore>({
dependencies: [workspace, addOnClient],
dependencies: [workspace, addonClient, {addonClientReady: ready}],
namespace: 'BundlesStore',
}) || createBundlesStore({addOnClient, studioClient})
}) || createBundlesStore({addonClient, studioClient, addonClientReady: ready})

resourceCache.set({
dependencies: [workspace, addOnClient],
dependencies: [workspace, addonClient, {addonClientReady: ready}],
namespace: 'BundlesStore',
value: bundlesStore,
})

return bundlesStore
}, [addOnClient, resourceCache, studioClient, workspace])
}, [addonClient, resourceCache, studioClient, workspace, ready])
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const createBundlesMetadataAggregator = (client: SanityClient | null) =>
bundleSlugs: string[],
isInitialLoad: boolean = false,
): Observable<MetadataWrapper> => {
if (!bundleSlugs?.length || !client) return EMPTY
if (!bundleSlugs?.length || !client) return of({data: null, error: null, loading: false})

const {subquery: queryAllDocumentsInBundleSlugs, projection: projectionToBundleMetadata} =
getFetchQuery(bundleSlugs)
Expand Down
32 changes: 24 additions & 8 deletions packages/sanity/src/core/store/bundles/createBundlesStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,20 @@ const INITIAL_STATE: bundlesReducerState = {
state: 'initialising',
}

const NOOP_BUNDLES_STORE: BundlesStore = {
const NOOP_BUNDLE_STORE: BundlesStore = {
state$: EMPTY.pipe(startWith(INITIAL_STATE)),
getMetadataStateForSlugs$: () => EMPTY,
getMetadataStateForSlugs$: () => of({data: null, error: null, loading: false}),
dispatch: () => undefined,
}

const LOADED_BUNDLE_STORE: BundlesStore = {
state$: EMPTY.pipe(
startWith({
bundles: new Map(),
state: 'loaded' as const,
}),
),
getMetadataStateForSlugs$: () => of({data: null, error: null, loading: false}),
dispatch: () => undefined,
}

Expand All @@ -70,19 +81,24 @@ const NOOP_BUNDLES_STORE: BundlesStore = {
* given the latest state upon subscription.
*/
export function createBundlesStore(context: {
addOnClient: SanityClient | null
addonClient: SanityClient | null
studioClient: SanityClient | null
addonClientReady: boolean
}): BundlesStore {
const {addOnClient: client, studioClient} = context
const {addonClient, studioClient, addonClientReady} = context

// While the comments dataset is initialising, this factory function will be called with an empty
// `client` value. Return a noop store while the client is unavailable.
//
// TODO: While the comments dataset is initialising, it incorrectly emits an empty object for the
// client instead of `null`, as the types suggest. Once this is fixed, we can remove the object
// keys length check.
if (!client || Object.keys(client).length === 0) {
return NOOP_BUNDLES_STORE
if (!addonClient || Object.keys(addonClient).length === 0) {
if (addonClientReady) {
// addon client has been fetched but it doesn't exists, nothing to load.
return LOADED_BUNDLE_STORE
}
return NOOP_BUNDLE_STORE
}

const dispatch$ = new Subject<bundlesReducerAction>()
Expand All @@ -105,7 +121,7 @@ export function createBundlesStore(context: {
filter(() => !fetchPending$.value),
tap(() => fetchPending$.next(true)),
concatWith(
client.observable.fetch<BundleDocument[]>(QUERY, {}, {tag: 'bundles.list'}).pipe(
addonClient.observable.fetch<BundleDocument[]>(QUERY, {}, {tag: 'bundles.list'}).pipe(
timeout(10_000), // 10s timeout
retry({
count: 2,
Expand Down Expand Up @@ -147,7 +163,7 @@ export function createBundlesStore(context: {
),
)

const listener$ = client.observable.listen<BundleDocument>(QUERY, {}, LISTEN_OPTIONS).pipe(
const listener$ = addonClient.observable.listen<BundleDocument>(QUERY, {}, LISTEN_OPTIONS).pipe(
map((event) => ({event})),
catchError((error) =>
of<ActionWrapper>({
Expand Down
22 changes: 17 additions & 5 deletions packages/sanity/src/core/store/bundles/useBundleOperations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,38 @@ import {DEFAULT_STUDIO_CLIENT_OPTIONS} from '../../studioClient'
import {type BundleDocument} from './types'

const useGuardedAddonClient = () => {
const {client: addOnClient} = useAddonDataset()
const {client: addOnClient, createAddonDataset} = useAddonDataset()

return function returnAddonClient() {
function getAddonClient() {
if (!addOnClient) {
throw new Error('Addon client is not available')
}

return addOnClient
}
async function getOrCreateAddonClient() {
if (!addOnClient) {
const client = await createAddonDataset()
if (!client) {
throw new Error('There was an error creating the addon client')
}
return client
}
return addOnClient
}

return {getAddonClient, getOrCreateAddonClient}
}

// WIP - Raw implementation for initial testing purposes
export function useBundleOperations() {
const getAddonClient = useGuardedAddonClient()
const {getAddonClient, getOrCreateAddonClient} = useGuardedAddonClient()
const studioClient = useClient(DEFAULT_STUDIO_CLIENT_OPTIONS)
const currentUser = useCurrentUser()

const handleCreateBundle = useCallback(
async (bundle: Partial<BundleDocument>) => {
const addonClient = getAddonClient()
const addonClient = await getOrCreateAddonClient()

const document = {
...bundle,
Expand All @@ -40,7 +52,7 @@ export function useBundleOperations() {
const res = await addonClient.createIfNotExists(document)
return res
},
[currentUser?.id, getAddonClient],
[currentUser?.id, getOrCreateAddonClient],
)

const handleDeleteBundle = useCallback(
Expand Down

0 comments on commit d787883

Please sign in to comment.