Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): update intent link for releases tool #7468

Merged
merged 3 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ export const BundleMenuButton = ({disabled, bundle, documentCount}: BundleMenuBu
),
})
setDiscardStatus('idle')
if (router.state.bundleId) {
if (router.state.releaseId) {
// navigate back to bundle overview
router.navigate({bundleId: undefined})
router.navigate({releaseId: undefined})
}
} catch (e) {
setDiscardStatus('error')
Expand Down
2 changes: 1 addition & 1 deletion packages/sanity/src/core/releases/i18n/resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ const releasesLocaleStrings = {
/** Text for when no releases are found */
'no-releases': 'No Releases',
/** Text for when a release is not found */
'not-found': 'Release not found: {{bundleId}}',
'not-found': 'Release not found: {{releaseId}}',

/** Description for the release tool */
'overview.description':
Expand Down
9 changes: 5 additions & 4 deletions packages/sanity/src/core/releases/plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ export const releases = definePlugin({
name: 'releases',
title: 'Releases',
component: ReleasesTool,
router: route.create('/', [route.create('/:bundleId')]),
canHandleIntent: (intent, params) => {
return Boolean(intent === 'release' && params.slug)
router: route.create('/', [route.create('/:releaseId')]),
canHandleIntent: (intent) => {
// If intent is release, open the releases tool.
return Boolean(intent === 'release')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of not resolving the intent if the params are missing, resolve to the tool at root level.

},
getIntentState(intent, params) {
if (intent === 'release') {
return {bundleId: params.slug}
return {releaseId: params.id}
}
return null
},
Expand Down
4 changes: 2 additions & 2 deletions packages/sanity/src/core/releases/tool/ReleasesTool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import {ReleasesOverview} from './overview/ReleasesOverview'
export function ReleasesTool() {
const router = useRouter()

const {bundleId} = router.state
if (bundleId) return <ReleaseDetail />
const {releaseId} = router.state
if (releaseId) return <ReleaseDetail />

return <ReleasesOverview />
}
14 changes: 7 additions & 7 deletions packages/sanity/src/core/releases/tool/detail/ReleaseDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,18 @@ export const ReleaseDetail = () => {

const activeScreen = getActiveScreen(router)

const {bundleId}: ReleasesRouterState = router.state
const {releaseId: releaseIdRaw}: ReleasesRouterState = router.state

const parsedBundleId = decodeURIComponent(bundleId || '')
const releaseId = decodeURIComponent(releaseIdRaw || '')
const {data, loading, deletedBundles} = useBundles()
const deletedBundle = deletedBundles[parsedBundleId]
const deletedBundle = deletedBundles[releaseId]

const {loading: documentsLoading, results} = useBundleDocuments(parsedBundleId)
const {loading: documentsLoading, results} = useBundleDocuments(releaseId)

const documentIds = results.map((result) => result.document?._id)
const history = useReleaseHistory(documentIds, parsedBundleId)
const history = useReleaseHistory(documentIds, releaseId)

const bundle = data?.find((storeBundle) => storeBundle._id === parsedBundleId)
const bundle = data?.find((storeBundle) => storeBundle._id === releaseId)
const bundleHasDocuments = !!results.length
const showPublishButton = loading || !bundle?.publishedAt
const isPublishButtonDisabled = loading || !bundle || !bundleHasDocuments
Expand Down Expand Up @@ -222,7 +222,7 @@ export const ReleaseDetail = () => {
<Card flex={1} tone="critical">
<Container width={0}>
<Stack paddingX={4} paddingY={6} space={1}>
<Heading>{t('not-found', {bundleId})}</Heading>
<Heading>{t('not-found', {releaseId})}</Heading>
</Stack>
</Container>
</Card>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ const renderTest = async () => {
return render(
<RouterProvider
state={{
bundleId: 'test-bundle-id',
releaseId: 'test-bundle-id',
}}
onNavigate={mockRouterNavigate}
router={route.create('/', [route.create('/:bundleId')])}
router={route.create('/', [route.create('/:releaseId')])}
>
<ReleaseDetail />
</RouterProvider>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ jest.mock('sanity/router', () => ({
...(jest.requireActual('sanity/router') || {}),
IntentLink: jest.fn().mockImplementation((props: any) => <a> {props.children}</a>),
useRouter: jest.fn().mockReturnValue({
state: {bundleId: 'differences'},
state: {releaseId: 'differences'},
navigate: jest.fn(),
}),
}))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const ReleaseNameCell: Column<TableBundle>['cell'] = ({cellProps, datum: bundle}
: {
as: 'a',
// navigate to bundle detail
onClick: () => router.navigate({bundleId: bundle._id}),
onClick: () => router.navigate({releaseId: bundle._id}),
}

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ describe('ReleasesOverview', () => {
const bundleRow = screen.getAllByTestId('table-row')[1]
fireEvent.click(within(bundleRow).getByText('Bundle 1'))

expect(useRouter().navigate).toHaveBeenCalledWith({bundleId: 'b1abcdefg'})
expect(useRouter().navigate).toHaveBeenCalledWith({releaseId: 'b1abcdefg'})
})
})
})
2 changes: 1 addition & 1 deletion packages/sanity/src/core/releases/types/router.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {type RouterState} from 'sanity/router'

export interface ReleasesRouterState extends RouterState {
bundleId?: string
releaseId?: string
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export const DocumentHeaderTitle = memo(function DocumentHeaderTitle(): ReactEle
</span>
)}
</TitleContainer>
<Flex flex="none" gap={1}>
<Flex flex="none" align="center" gap={1}>
<DocumentPerspectiveMenu />
</Flex>
</Flex>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,61 @@
import {ChevronDownIcon} from '@sanity/icons'
// eslint-disable-next-line no-restricted-imports -- Bundle Button requires more fine-grained styling than studio button
import {Box, Button} from '@sanity/ui'
import {memo, useCallback, useMemo} from 'react'
import {BundleBadge, BundlesMenu, usePerspective, useTranslation} from 'sanity'
import {useRouter} from 'sanity/router'
import {styled} from 'styled-components'
// eslint-disable-next-line camelcase
import {getTheme_v2} from '@sanity/ui/theme'
import {memo, useMemo} from 'react'
import {BundleBadge, type BundleDocument, BundlesMenu, usePerspective, useTranslation} from 'sanity'
import {IntentLink} from 'sanity/router'
import {css, styled} from 'styled-components'

import {Button as StudioButton} from '../../../../../../ui-components'
import {usePaneRouter} from '../../../../../components'
import {useDocumentPane} from '../../../useDocumentPane'

const BadgeButton = styled(Button)({
cursor: 'pointer',
const BadgeButton = styled(Button)((props) => {
const theme = getTheme_v2(props.theme)
const mode = props.mode || 'default'
const tone = props.tone || 'default'
const color = theme.color.button[mode][tone]

return css`
cursor: pointer;
border-radius: 999px !important;
@media (hover: hover) {
&:hover {
text-decoration: none !important;
background-color: ${color.hovered.bg};
}
}
`
})

const ReleaseLink = ({release}: {release: Partial<BundleDocument>}) => {
const {hue, title, icon, _id: releaseId} = release

return (
<BadgeButton
mode="bleed"
padding={0}
radius="full"
data-testid="button-document-release"
intent="release"
params={{id: releaseId}}
target="_blank"
rel="noopener noreferrer"
as={IntentLink}
>
<BundleBadge hue={hue} title={title} icon={icon} padding={2} />
</BadgeButton>
)
}

export const DocumentPerspectiveMenu = memo(function DocumentPerspectiveMenu() {
const paneRouter = usePaneRouter()
const {t} = useTranslation()
const {currentGlobalBundle} = usePerspective(paneRouter.perspective)

const {documentVersions, existsInBundle} = useDocumentPane()
const {title, hue, icon, _id: bundleId} = currentGlobalBundle

const router = useRouter()

const handleBundleClick = useCallback(() => {
router.navigateIntent('release', {id: bundleId})
}, [router, bundleId])

const bundlesMenuButton = useMemo(
() => (
Expand All @@ -41,17 +70,7 @@ export const DocumentPerspectiveMenu = memo(function DocumentPerspectiveMenu() {

return (
<>
{currentGlobalBundle && existsInBundle && (
<BadgeButton
onClick={handleBundleClick}
mode="bleed"
padding={0}
radius="full"
data-testid="button-document-release"
>
<BundleBadge hue={hue} title={title} icon={icon} padding={2} />
</BadgeButton>
)}
{currentGlobalBundle && existsInBundle && <ReleaseLink release={currentGlobalBundle} />}

{/** TODO IS THIS STILL NEEDED? VS THE PICKER IN STUDIO NAVBAR? */}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {beforeEach, describe, expect, it, jest} from '@jest/globals'
import {fireEvent, render, screen} from '@testing-library/react'
import {render, screen} from '@testing-library/react'
import {type HTMLProps} from 'react'
import {type BundleDocument, usePerspective} from 'sanity'
import {useRouter} from 'sanity/router'
import {type IntentLinkProps} from 'sanity/router'

import {createTestProvider} from '../../../../../../../../test/testUtils/TestProvider'
import {type DocumentPaneContextValue} from '../../../../DocumentPaneContext'
Expand All @@ -22,24 +23,34 @@ jest.mock('sanity', () => {
}
})

jest.mock('sanity/router', () => ({
useRouter: jest.fn().mockReturnValue({
navigateIntent: jest.fn(),
stickyParams: {},
}),
route: {
create: jest.fn(),
},
IntentLink: jest.fn(),
}))
const IntentLinkMock = (props: IntentLinkProps & HTMLProps<HTMLAnchorElement>) => {
const {params = {}, intent, ...rest} = props
const stringParams = params
? Object.entries(params)
.map(([key, value]) => `${key}=${value}`)
.join('&')
: ''

return <a {...rest} href={`/intent/${intent}/${stringParams}`} />
}

jest.mock('sanity/router', () => {
return {
useRouter: jest.fn().mockReturnValue({
stickyParams: {},
}),
route: {
create: jest.fn(),
},
IntentLink: IntentLinkMock,
}
})

jest.mock('../../../../useDocumentPane')

const mockUseDocumentPane = useDocumentPane as jest.MockedFunction<
() => Partial<DocumentPaneContextValue>
>
const mockUseRouter = useRouter as jest.MockedFunction<typeof useRouter>
const navigateIntent = mockUseRouter().navigateIntent as jest.Mock

const mockUsePerspective = usePerspective as jest.Mock

Expand Down Expand Up @@ -90,45 +101,33 @@ describe('DocumentPerspectiveMenu', () => {
const wrapper = await createTestProvider()
render(<DocumentPerspectiveMenu />, {wrapper})

expect(screen.getByTestId('button-document-release')).toBeInTheDocument()
const linkButton = screen.getByRole('link', {name: 'Spring Drop'})
expect(linkButton).toBeInTheDocument()
expect(linkButton).toHaveAttribute('href', '/intent/release/id=spring-drop')
expect(linkButton).toHaveTextContent('Spring Drop')
})

it('should not render the bundle badge if the document does not exist in the bundle', async () => {
mockUseDocumentPane.mockReturnValue({
existsInBundle: false,
})

const wrapper = await createTestProvider()
render(<DocumentPerspectiveMenu />, {wrapper})

expect(screen.queryByTestId('button-document-release')).toBeNull()
})

it('should navigate to the release intent when the bundle badge is clicked', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test doesn't apply anymore, because we are not rendering a button, we are now rendering a <a>
That's also the reason why instead of using a getByTestId getter for the button we are now using a getByRole("link") getter in this tests

mockUseDocumentPane.mockReturnValue({
documentVersions: [
{
_id: 'spring-drop',
title: 'Spring Drop',
hue: 'magenta',
icon: 'heart-filled',
_type: 'release',
authorId: '',
_id: 'spring-drop',
_createdAt: '',
_updatedAt: '',
_rev: '',
},
],
existsInBundle: true,
existsInBundle: false,
})

const wrapper = await createTestProvider()
render(<DocumentPerspectiveMenu />, {wrapper})

expect(screen.queryByTestId('button-document-release')).toBeInTheDocument()
fireEvent.click(screen.getByTestId('button-document-release'))

expect(navigateIntent).toHaveBeenCalledTimes(1)
expect(navigateIntent).toHaveBeenCalledWith('release', {id: 'spring-drop'})
expect(screen.queryByRole('link', {name: 'Spring Drop'})).toBeNull()
})
})
Loading