feat: content releases #8454
feat: content releases #8454
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
|
No changes to documentation |
|
Component Testing Report Updated Jan 31, 2025 12:03 PM (UTC) ❌ Failed Tests (1) -- expand for details
|
⚡️ Editor Performance ReportUpdated Fri, 31 Jan 2025 12:06:20 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
bjoerge
force-pushed
the
feat/content-releases
branch
from
536e5f2 to
93d76ce
Compare
| @@ -35,39 +48,57 @@ export function getReferenceInfo( | |||
| documentPreviewStore: DocumentPreviewStore, | |||
| id: string, | |||
| referenceType: ReferenceSchemaType, | |||
| {version}: {version?: string} = {}, | |||
| perspective: {bundleIds: string[]; bundleStack: PerspectiveStack} = { | |||
There was a problem hiding this comment.
we should consider renaming this to perspective: {ids: string[], stack: PerspectiveStack}
| } else if (releaseId === 'published' || releaseId.startsWith('r')) { | ||
| perspectiveParam = releaseId | ||
| } else { | ||
| throw new Error(`Invalid releaseId: ${releaseId}`) |
There was a problem hiding this comment.
making a note for us to verify whether we can keep this hard validation of release ids, or whether we should rather consider it a convention
| /** | ||
| * An array of all existing bundle ids. | ||
| */ | ||
| bundleIds: string[] |
There was a problem hiding this comment.
should we rename this to ids?
| * An array of release ids ordered chronologically to represent the state of documents at the | ||
| * given point in time. | ||
| */ | ||
| bundleStack: PerspectiveStack |
There was a problem hiding this comment.
should we rename this to just stack? not sure if perspective.bundleStack makes sense anymore?
| * document readability for the version document | ||
| */ | ||
| version?: DocumentAvailability | ||
| // TODO: validate versions availability? |
|
|
||
| const QUERY_FILTER = `_type=="${RELEASE_DOCUMENT_TYPE}" && _id in path("${RELEASE_DOCUMENTS_PATH}.*")` | ||
|
|
||
| // TODO: Extend the projection with the fields needed |
| */ | ||
| export interface ReleaseStore { | ||
| state$: Observable<ReleasesReducerState> | ||
| getMetadataStateForSlugs$: (slugs: string[]) => Observable<MetadataWrapper> |
There was a problem hiding this comment.
it's probably fine to skip the $-suffix here and above (feels like a less valuable pattern combined with types)
| /** | ||
| * @internal | ||
| */ | ||
| export function useReleaseOperations() { |
There was a problem hiding this comment.
instead of creating a separate store per component it would be better if we moved this to a context and kept a single instance to it (the store is stateless, so it's probably not critical for merge)
| * Gets all the releases ids | ||
| * @internal | ||
| */ | ||
| export function useReleasesIds(releases: ReleaseDocument[]): { |
There was a problem hiding this comment.
The double plural reads a bit awkward, better to rename it to useReleaseIds?
There was a problem hiding this comment.
We could also consider whether we need it at all, seems to have somewhat limited value?
There was a problem hiding this comment.
The naming of this file seems a bit off to me. I think it's one of very few places we use "variant" as the common term for published, draft and versions. It might not be a 100% correct name, but could we consider renaming to getDocumentVersionType instead? At least that's closer to the name I would look for if I needed to know.
Co-Authored-By: Jordan Lawrence <[email protected]> Co-Authored-By: Bjørge Næss <[email protected]> Co-Authored-By: Rita <[email protected]> Co-Authored-By: pedrobonamin <[email protected]> Co-Authored-By: Ash <[email protected]> Co-Authored-By: Cody Olsen <[email protected]> Co-Authored-By: Robin Neatherway <[email protected]>
bjoerge
force-pushed
the
feat/content-releases
branch
from
93d76ce to
1b4edb7
Compare
| export * from '../store/_legacy' | ||
| export * from '../store/user' |
There was a problem hiding this comment.
I think we should remove this two exports from here
| @@ -30,6 +30,7 @@ export type ReferenceFilterSearchOptions = { | |||
| tag?: string | |||
| maxFieldDepth?: number | |||
| strategy?: SearchStrategy | |||
| perspective?: string | string[] | |||
There was a problem hiding this comment.
Could we use the ClientPerspective type from @sanity/client here?
| /** | ||
| * This is a temporary hack to get around the fact that the `drafts.versions.versionName.id` format is not supported | ||
| * `versions.` don't have draft documents. | ||
| * | ||
| * Where is the drafts. prefix coming from and why it's added to the `versions.` documents? | ||
| * */ | ||
| const cleanId = | ||
| data.documentId.includes('versions.') && isDraftId(data.documentId) | ||
| ? data.documentId.replace('drafts.', '') | ||
| : data.documentId | ||
| const snapshot = await client.getDocument(cleanId, { |
There was a problem hiding this comment.
This should no longer be necessary
| /** | |
| * This is a temporary hack to get around the fact that the `drafts.versions.versionName.id` format is not supported | |
| * `versions.` don't have draft documents. | |
| * | |
| * Where is the drafts. prefix coming from and why it's added to the `versions.` documents? | |
| * */ | |
| const cleanId = | |
| data.documentId.includes('versions.') && isDraftId(data.documentId) | |
| ? data.documentId.replace('drafts.', '') | |
| : data.documentId | |
| const snapshot = await client.getDocument(cleanId, { | |
| const snapshot = await client.getDocument(data.documentId, { |
There was a problem hiding this comment.
It would be ideal if we reused all of these in @sanity/client/csm so we have a single source of truth:
import {
type DraftId,
DRAFTS_FOLDER,
getDraftId,
getPublishedId,
getVersionFromId,
getVersionId,
isDraftId,
isPublishedId,
isVersionId,
type PublishedId,
VERSION_FOLDER,
} from '@sanity/client/csm'
Description
This PR marks the public beta of the Content Releases feature. The content releases feature itself will be opt-in (for now), but these changes also introduces a few key improvements to the Studio UI, most notably the ability to switch between the published document and the current draft, and the ability to browse the Studio by only seeing what's currently published.
The Studio now supports a
perspectivesearch param, which is "sticky", meaning that it is preserved when navigating around in the studio. This param can take a comma-separated list of values (referred to as a "perspecive stack", which will then be forwarded to theperspectiveparam used by list queries made by the Studio)Terminology
With these changes we are introducing three new terms into the codebase, I'll cover them in brief here (there's more details to be found in internal documentation for the curious minded):
Version- This is comparable to a draft document but with a different prefix: instead ofdrafts.<publishedId>it will haveversions.<releaseId>.<publishedId>. Theversions.prefix is fixed, and no documents can have that as a prefix without following theversions.<string>.<id>structure.Release– A release is represented by a system document that has an ID on the form_.release.<releaseId>. The<releaseId>-part here is what connects it with version documents. The<releaseId>part conventionally starts withr, and is followed by 8 random characters, e.g.rNe6H9RGZ.Bundle- This is an internal, technical name used to connect documents in the release together. You can think of thereleaseIdas a type of bundle, but there could also be other types of bundles in the future.There's no change to the semantics around
draftdocuments, other than you can now think of them as version documents with special sematics applied to them.What to review
This is a rather chonky one, when reviewing feel free to skp fast through things like tests and fixtures and focus on the more substantial stuff. Things in particular to watch out for are:
Note: we'll be using
vXfor various API calls if the feature is enabled, otherwise we'll stick with stable API versions. While it's not ideal to ship with vX, we have considered it to be an ok tradeoff in this case, given that the feature is opt-in and considered beta. Edit: Seems like this is not the case right now, we will be addressing this before merging, but adding a todo here:Testing
Most new code includes tests, but our coverage of existing code paths is still aspirational at best, so any manual testing is greatly appreciated.
Notes for release
tbd