Skip to content

Commit

Permalink
fix(core): hide versions.* documents from search, update `getPublis…
Browse files Browse the repository at this point in the history
…hedId` to account for versions (#7470)

* fix(core): hide versions.* documents from search and lists, show error in document panel

* chore(core): update draftUtils to account for version ids

* fix(structure): revert structureTitle and DocumentPane changes for version checks

* fix: use path separator in getVersionId split

Co-authored-by: Ash <ash@sanity.io>

---------

Co-authored-by: Ash <ash@sanity.io>
  • Loading branch information
pedrobonamin and juice49 authored Sep 20, 2024
1 parent 4729e84 commit 7c814a8
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export const createTextSearch: SearchStrategyFactory<TextSearchResults> = (
searchOptions.includeDrafts === false && "!(_id in path('drafts.**'))",
factoryOptions.filter ? `(${factoryOptions.filter})` : false,
searchTerms.filter ? `(${searchTerms.filter})` : false,
'!(_id in path("versions.**"))',
].filter((baseFilter): baseFilter is string => Boolean(baseFilter))

const textSearchParams: TextSearchParams = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('createSearchQuery', () => {

expect(query).toEqual(
`// findability-mvi:${FINDABILITY_MVI}\n` +
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0)]' +
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0) && !(_id in path("versions.**"))]' +
'| order(_id asc)' +
'[0...$__limit]' +
'{_type, _id, ...select(_type == "basic-schema-test" => { "w0": _id,"w1": _type,"w2": title })}',
Expand Down Expand Up @@ -106,7 +106,7 @@ describe('createSearchQuery', () => {
})

expect(query).toContain(
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0 || object.field match $t0)]',
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0 || object.field match $t0) && !(_id in path("versions.**"))]',
)
})

Expand All @@ -117,7 +117,7 @@ describe('createSearchQuery', () => {
})

expect(query).toContain(
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0) && (_id match $t1 || _type match $t1 || title match $t1)]',
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0) && (_id match $t1 || _type match $t1 || title match $t1) && !(_id in path("versions.**"))]',
)
expect(params.t0).toEqual('term0*')
expect(params.t1).toEqual('term1*')
Expand Down Expand Up @@ -147,7 +147,7 @@ describe('createSearchQuery', () => {

const result = [
`// findability-mvi:${FINDABILITY_MVI}\n` +
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0)]{_type, _id, object{field}}',
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0) && !(_id in path("versions.**"))]{_type, _id, object{field}}',
'|order(_id asc)[0...$__limit]',
'{_type, _id, ...select(_type == "basic-schema-test" => { "w0": _id,"w1": _type,"w2": title })}',
].join('')
Expand Down Expand Up @@ -193,7 +193,7 @@ describe('createSearchQuery', () => {
)

expect(query).toContain(
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0) && (randomCondition == $customParam)]',
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0) && (randomCondition == $customParam) && !(_id in path("versions.**"))]',
)
expect(params.customParam).toEqual('custom')
})
Expand Down Expand Up @@ -241,7 +241,7 @@ describe('createSearchQuery', () => {

expect(query).toEqual(
`// findability-mvi:${FINDABILITY_MVI}\n` +
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0)]' +
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0) && !(_id in path("versions.**"))]' +
'| order(exampleField desc)' +
'[0...$__limit]' +
'{_type, _id, ...select(_type == "basic-schema-test" => { "w0": _id,"w1": _type,"w2": title })}',
Expand Down Expand Up @@ -275,7 +275,7 @@ describe('createSearchQuery', () => {

const result = [
`// findability-mvi:${FINDABILITY_MVI}\n`,
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0)]| ',
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0) && !(_id in path("versions.**"))]| ',
'order(exampleField desc,anotherExampleField asc,lower(mapWithField) asc)',
'[0...$__limit]{_type, _id, ...select(_type == "basic-schema-test" => { "w0": _id,"w1": _type,"w2": title })}',
].join('')
Expand All @@ -291,7 +291,7 @@ describe('createSearchQuery', () => {

expect(query).toEqual(
`// findability-mvi:${FINDABILITY_MVI}\n` +
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0)]' +
'*[_type in $__types && (_id match $t0 || _type match $t0 || title match $t0) && !(_id in path("versions.**"))]' +
'| order(_id asc)' +
'[0...$__limit]' +
'{_type, _id, ...select(_type == "basic-schema-test" => { "w0": _id,"w1": _type,"w2": title })}',
Expand Down Expand Up @@ -403,7 +403,7 @@ describe('createSearchQuery', () => {
* This is an improvement over before, where an illegal term was used (number-as-string, ala ["0"]),
* which lead to no hits at all. */
`// findability-mvi:${FINDABILITY_MVI}\n` +
'*[_type in $__types && (_id match $t0 || _type match $t0 || cover[].cards[].title match $t0) && (_id match $t1 || _type match $t1 || cover[].cards[].title match $t1)]' +
'*[_type in $__types && (_id match $t0 || _type match $t0 || cover[].cards[].title match $t0) && (_id match $t1 || _type match $t1 || cover[].cards[].title match $t1) && !(_id in path("versions.**"))]' +
'| order(_id asc)' +
'[0...$__limit]' +
// at this point we could refilter using cover[0].cards[0].title.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export function createSearchQuery(
...createConstraints(terms, specs),
filter ? `(${filter})` : '',
searchTerms.filter ? `(${searchTerms.filter})` : '',
'!(_id in path("versions.**"))',
].filter(Boolean)

const selections = specs.map((spec) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {type LocaleSource} from '../../../i18n'
import {type DocumentPreviewStore} from '../../../preview'
import {DEFAULT_STUDIO_CLIENT_OPTIONS} from '../../../studioClient'
import {type Template} from '../../../templates'
import {getDraftId, isDraftId} from '../../../util'
import {getDraftId, isDraftId, isVersionId} from '../../../util'
import {type ValidationStatus} from '../../../validation'
import {type HistoryStore} from '../history'
import {checkoutPair, type DocumentVersionEvent, type Pair} from './document-pair/checkoutPair'
Expand All @@ -34,6 +34,9 @@ import {type IdPair} from './types'
export type QueryParams = Record<string, string | number | boolean | string[]>

function getIdPairFromPublished(publishedId: string): IdPair {
if (isVersionId(publishedId)) {
throw new Error('editOpsOf does not expect a version id.')
}
if (isDraftId(publishedId)) {
throw new Error('editOpsOf does not expect a draft id.')
}
Expand Down
48 changes: 46 additions & 2 deletions packages/sanity/src/core/util/draftUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import {expect, test} from '@jest/globals'
import {describe, expect, it, test} from '@jest/globals'
import {type SanityDocument} from '@sanity/types'

import {collate, documentIdEquals, removeDupes} from './draftUtils'
import {
collate,
documentIdEquals,
getPublishedId,
getVersionFromId,
getVersionId,
removeDupes,
} from './draftUtils'

test('collate()', () => {
const foo = {_type: 'foo', _id: 'foo'}
Expand Down Expand Up @@ -38,3 +45,40 @@ test.each([
])('documentIdEquals(): %s', (_, documentId, equalsDocumentId, shouldEqual) => {
expect(documentIdEquals(documentId, equalsDocumentId)).toEqual(shouldEqual)
})

test.each([
['From published id', 'agot', 'summer-drop', 'versions.summer-drop.agot'],
['From draft id', 'drafts.agot', 'summer-drop', 'versions.summer-drop.agot'],
['From same version id', 'versions.summer-drop.agot', 'summer-drop', 'versions.summer-drop.agot'],
[
'From other version id',
'versions.winter-drop.agot',
'summer-drop',
'versions.summer-drop.agot',
],
])('getVersionId(): %s', (_, documentId, equalsDocumentId, shouldEqual) => {
expect(getVersionId(documentId, equalsDocumentId)).toEqual(shouldEqual)
})

test.each([
['from published id', 'agot', 'agot'],
['from draft id', 'drafts.agot', 'agot'],
['from version id', 'versions.summer-drop.agot', 'agot'],
['from complex id with version', 'versions.summer-drop.foo.agot', 'foo.agot'],
])('getPublishedId(): %s', (_, documentId, shouldEqual) => {
expect(getPublishedId(documentId)).toEqual(shouldEqual)
})

describe('getVersionFromId', () => {
it('should return the bundle slug', () => {
expect(getVersionFromId('versions.summer.my-document-id')).toBe('summer')
})

it('should return the undefined if no bundle slug is found and document is a draft', () => {
expect(getVersionFromId('drafts.my-document-id')).toBe(undefined)
})

it('should return the undefined if no bundle slug is found and document is published', () => {
expect(getVersionFromId('my-document-id')).toBe(undefined)
})
})
51 changes: 48 additions & 3 deletions packages/sanity/src/core/util/draftUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ export type PublishedId = Opaque<string, 'publishedId'>

/** @internal */
export const DRAFTS_FOLDER = 'drafts'
const DRAFTS_PREFIX = `${DRAFTS_FOLDER}.`
/** @internal */
export const VERSION_FOLDER = 'versions'
const PATH_SEPARATOR = '.'
const DRAFTS_PREFIX = `${DRAFTS_FOLDER}${PATH_SEPARATOR}`
const VERSION_PREFIX = `${VERSION_FOLDER}${PATH_SEPARATOR}`

/**
*
Expand Down Expand Up @@ -55,6 +59,11 @@ export function isDraftId(id: string): id is DraftId {
return id.startsWith(DRAFTS_PREFIX)
}

/** @internal */
export function isVersionId(id: string): boolean {
return id.startsWith(VERSION_PREFIX)
}

/** @internal */
export function getIdPair(id: string): {draftId: DraftId; publishedId: PublishedId} {
return {
Expand All @@ -65,17 +74,53 @@ export function getIdPair(id: string): {draftId: DraftId; publishedId: Published

/** @internal */
export function isPublishedId(id: string): id is PublishedId {
return !isDraftId(id)
return !isDraftId(id) && !isVersionId(id)
}

/** @internal */
export function getDraftId(id: string): DraftId {
return isDraftId(id) ? id : ((DRAFTS_PREFIX + id) as DraftId)
}

/** @internal */
export function getVersionId(id: string, bundle: string): string {
if (isVersionId(id)) {
const [_versionPrefix, versionId, ...publishedId] = id.split(PATH_SEPARATOR)
if (versionId === bundle) return id
return `${VERSION_PREFIX}${bundle}${PATH_SEPARATOR}${publishedId}`
}

const publishedId = getPublishedId(id)

return `${VERSION_PREFIX}${bundle}${PATH_SEPARATOR}${publishedId}`
}

/**
* @internal
* Given an id, returns the versionId if it exists.
* e.g. `versions.summer-drop.foo` = `summer-drop`
* e.g. `drafts.foo` = `undefined`
* e.g. `foo` = `undefined`
*/
export function getVersionFromId(id: string): string | undefined {
if (!isVersionId(id)) return undefined
const [_versionPrefix, versionId, ..._publishedId] = id.split(PATH_SEPARATOR)

return versionId
}

/** @internal */
export function getPublishedId(id: string): PublishedId {
return (isDraftId(id) ? id.slice(DRAFTS_PREFIX.length) : id) as PublishedId
if (isVersionId(id)) {
// make sure to only remove the versions prefix and the bundle name
return id.split(PATH_SEPARATOR).slice(2).join(PATH_SEPARATOR) as PublishedId as PublishedId
}

if (isDraftId(id)) {
return id.slice(DRAFTS_PREFIX.length) as PublishedId
}

return id as PublishedId
}

/** @internal */
Expand Down

0 comments on commit 7c814a8

Please sign in to comment.