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): hide versions.* documents from search, update getPublishedId to account for versions #7470

Merged
merged 4 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -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('.')
pedrobonamin marked this conversation as resolved.
Show resolved Hide resolved
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
Loading