From 7c814a88e4d0629650b57d4705892ade2b7d1220 Mon Sep 17 00:00:00 2001 From: Pedro Bonamin <46196328+pedrobonamin@users.noreply.github.com> Date: Fri, 20 Sep 2024 17:07:59 +0200 Subject: [PATCH] fix(core): hide `versions.*` documents from search, update `getPublishedId` 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 --------- Co-authored-by: Ash --- .../search/text-search/createTextSearch.ts | 1 + .../search/weighted/createSearchQuery.test.ts | 18 +++---- .../core/search/weighted/createSearchQuery.ts | 1 + .../store/_legacy/document/document-store.ts | 5 +- .../sanity/src/core/util/draftUtils.test.ts | 48 ++++++++++++++++- packages/sanity/src/core/util/draftUtils.ts | 51 +++++++++++++++++-- 6 files changed, 109 insertions(+), 15 deletions(-) diff --git a/packages/sanity/src/core/search/text-search/createTextSearch.ts b/packages/sanity/src/core/search/text-search/createTextSearch.ts index 1afbd3a20c2..a005d7eefb3 100644 --- a/packages/sanity/src/core/search/text-search/createTextSearch.ts +++ b/packages/sanity/src/core/search/text-search/createTextSearch.ts @@ -142,6 +142,7 @@ export const createTextSearch: SearchStrategyFactory = ( 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 = { diff --git a/packages/sanity/src/core/search/weighted/createSearchQuery.test.ts b/packages/sanity/src/core/search/weighted/createSearchQuery.test.ts index 7cd847d75dc..93b56c5f38e 100644 --- a/packages/sanity/src/core/search/weighted/createSearchQuery.test.ts +++ b/packages/sanity/src/core/search/weighted/createSearchQuery.test.ts @@ -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 })}', @@ -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.**"))]', ) }) @@ -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*') @@ -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('') @@ -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') }) @@ -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 })}', @@ -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('') @@ -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 })}', @@ -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. diff --git a/packages/sanity/src/core/search/weighted/createSearchQuery.ts b/packages/sanity/src/core/search/weighted/createSearchQuery.ts index 605660351ae..15b924da74d 100644 --- a/packages/sanity/src/core/search/weighted/createSearchQuery.ts +++ b/packages/sanity/src/core/search/weighted/createSearchQuery.ts @@ -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) => { diff --git a/packages/sanity/src/core/store/_legacy/document/document-store.ts b/packages/sanity/src/core/store/_legacy/document/document-store.ts index d40fba36ce4..7fcc203c829 100644 --- a/packages/sanity/src/core/store/_legacy/document/document-store.ts +++ b/packages/sanity/src/core/store/_legacy/document/document-store.ts @@ -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' @@ -34,6 +34,9 @@ import {type IdPair} from './types' export type QueryParams = Record 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.') } diff --git a/packages/sanity/src/core/util/draftUtils.test.ts b/packages/sanity/src/core/util/draftUtils.test.ts index ce813abf206..d87d8f1de33 100644 --- a/packages/sanity/src/core/util/draftUtils.test.ts +++ b/packages/sanity/src/core/util/draftUtils.test.ts @@ -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'} @@ -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) + }) +}) diff --git a/packages/sanity/src/core/util/draftUtils.ts b/packages/sanity/src/core/util/draftUtils.ts index c87031b4f86..763fe9c9d1b 100644 --- a/packages/sanity/src/core/util/draftUtils.ts +++ b/packages/sanity/src/core/util/draftUtils.ts @@ -14,7 +14,11 @@ export type PublishedId = Opaque /** @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}` /** * @@ -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 { @@ -65,7 +74,7 @@ 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 */ @@ -73,9 +82,45 @@ 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 */