-
Notifications
You must be signed in to change notification settings - Fork 0
feat(push): handle deleting custom types that still have documents #187
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,10 @@ import { pascalCase } from "change-case"; | |
|
|
||
| import { getAdapter } from "../adapters"; | ||
| import { getHost, getToken } from "../auth"; | ||
| import { | ||
| deleteDocumentsByCustomType, | ||
| getDocumentTotalByCustomTypes, | ||
| } from "../clients/core"; | ||
| import { | ||
| getCustomTypes, | ||
| getSlices, | ||
|
|
@@ -16,10 +20,12 @@ import { | |
| completeOnboardingStepsSilently, | ||
| type OnboardingStep, | ||
| } from "../clients/repository"; | ||
| import { getWorkingDocumentsUrlForCustomType } from "../clients/wroom"; | ||
| import { resolveEnvironment } from "../environments"; | ||
| import { CommandError, createCommand, type CommandConfig } from "../lib/command"; | ||
| import { diffArrays } from "../lib/diff"; | ||
| import { getDirtyPaths, getGitRoot } from "../lib/git"; | ||
| import { BadRequestError } from "../lib/request"; | ||
| import { appendTrailingSlash, isDescendant, relativePathname } from "../lib/url"; | ||
| import { canonicalizeModel } from "../models"; | ||
| import { findProjectRoot, getRepositoryName } from "../project"; | ||
|
|
@@ -33,14 +39,24 @@ const config = { | |
| updated, or deleted to match. | ||
| `, | ||
| options: { | ||
| force: { type: "boolean", short: "f", description: "Skip safety checks" }, | ||
| force: { type: "boolean", short: "f", description: "Skip overwrite safety checks" }, | ||
| "delete-pages": { | ||
| type: "boolean", | ||
| description: | ||
| "Confirm the bulk-deletion of associated pages when removing a type", | ||
| }, | ||
| repo: { type: "string", short: "r", description: "Repository domain" }, | ||
| env: { type: "string", short: "e", description: "Environment domain" }, | ||
| }, | ||
| } satisfies CommandConfig; | ||
|
|
||
| export default createCommand(config, async ({ values }) => { | ||
| const { force = false, repo: parentRepo = await getRepositoryName(), env } = values; | ||
| const { | ||
| force = false, | ||
| "delete-pages": deletePages = false, | ||
| repo: parentRepo = await getRepositoryName(), | ||
| env, | ||
| } = values; | ||
|
|
||
| const token = await getToken(); | ||
| const host = await getHost(); | ||
|
|
@@ -134,8 +150,13 @@ export default createCommand(config, async ({ values }) => { | |
| for (const model of customTypeOps.update) { | ||
| await updateCustomType(model, { repo, token, host }); | ||
| } | ||
| for (const id of customTypeOps.delete.map((m) => m.id)) { | ||
| await removeCustomType(id, { repo, token, host }); | ||
| for (const model of customTypeOps.delete) { | ||
| await removeCustomTypeWithDocumentHandling(model.id, { | ||
| repo, | ||
| token, | ||
| host, | ||
| deletePages, | ||
| }); | ||
| } | ||
| for (const model of sliceOps.insert) { | ||
| await insertSlice(model, { repo, token, host }); | ||
|
|
@@ -173,3 +194,74 @@ export default createCommand(config, async ({ values }) => { | |
| if (totalDeletes > 0) console.info(`Deleted ${totalDeletes} model(s).`); | ||
| } | ||
| }); | ||
|
|
||
| const DELETE_PAGES_LIMIT = 200; // same hard limit from type builder and sm-api | ||
|
|
||
| async function removeCustomTypeWithDocumentHandling( | ||
| id: string, | ||
| config: { | ||
| repo: string; | ||
| token: string | undefined; | ||
| host: string; | ||
| deletePages: boolean; | ||
| }, | ||
| ): Promise<void> { | ||
| const { repo, token, host, deletePages: forceDeletePages } = config; | ||
| try { | ||
| await removeCustomType(id, { repo, token, host }); | ||
| } catch (error) { | ||
| if (!(await isDocumentsInUseError(error))) throw error; | ||
|
|
||
| let documentCount: number; | ||
| try { | ||
| documentCount = await getDocumentTotalByCustomTypes(id, { repo, token, host }); | ||
| } catch { | ||
| throw new CommandError( | ||
| `Failed to check whether type "${id}" has associated pages. ` + | ||
| "Please try pushing again, or manually delete any associated pages in Prismic: " + getWorkingDocumentsUrlForCustomType({ repo, host, customTypeId: id }), | ||
| ); | ||
| } | ||
|
|
||
| if (documentCount === 0) { | ||
| try { | ||
| await removeCustomType(id, { repo, token, host }); | ||
| return; | ||
| } catch (retryError) { | ||
| if (!(await isDocumentsInUseError(retryError))) throw retryError; | ||
| throw new CommandError( | ||
| `Unable to delete type "${id}". It may have associated pages. ` + | ||
| `Please try pushing again, or manually delete any associated pages in Prismic: ` + getWorkingDocumentsUrlForCustomType({ repo, host, customTypeId: id }), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (documentCount > DELETE_PAGES_LIMIT) { | ||
| const plural = documentCount === 1 ? "" : "s"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will always evaluate to "s" since |
||
| throw new CommandError( | ||
| `Cannot delete type "${id}": it has ${documentCount} associated page${plural}, ` + | ||
| `which exceeds the limit of ${DELETE_PAGES_LIMIT} that can be bulk-deleted. ` + | ||
| `Delete pages manually before pushing: ` + getWorkingDocumentsUrlForCustomType({ repo, host, customTypeId: id }), | ||
| ); | ||
| } | ||
|
|
||
| if (!forceDeletePages) { | ||
| const plural = documentCount === 1 ? "" : "s"; | ||
| throw new CommandError( | ||
| `Type "${id}" has ${documentCount} associated page${plural}. ` + | ||
| `Deleting it will also permanently delete all associated pages: \n` + getWorkingDocumentsUrlForCustomType({ repo, host, customTypeId: id }) + "\n\n" + | ||
| `Pass --delete-pages to confirm this cascading deletion.`, | ||
| ); | ||
|
|
||
| } | ||
|
|
||
| console.info(`Deleting pages associated with type "${id}"...`); | ||
| await deleteDocumentsByCustomType(id, { repo, token, host }); | ||
| await removeCustomType(id, { repo, token, host }); | ||
| } | ||
| } | ||
|
|
||
| async function isDocumentsInUseError(error: unknown): Promise<boolean> { | ||
| if (!(error instanceof BadRequestError)) return false; | ||
| const body = await error.text(); | ||
| return body.includes("associated documents") || body.includes("Delete all documents belonging"); | ||
| } | ||
|
jomifepe marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,6 +53,7 @@ export async function request<T>( | |
|
|
||
| return value; | ||
| } else { | ||
| if (response.status === 400) throw new BadRequestError(response, value); | ||
|
jomifepe marked this conversation as resolved.
|
||
| if (response.status === 401) throw new UnauthorizedRequestError(response); | ||
| if (response.status === 403) throw new ForbiddenRequestError(response); | ||
| if (response.status === 404) throw new NotFoundRequestError(response); | ||
|
|
@@ -89,6 +90,15 @@ export class RequestError extends Error { | |
| export class UnknownRequestError extends RequestError { | ||
| name = "UnknownRequestError"; | ||
| } | ||
| export class BadRequestError extends RequestError { | ||
| name = "BadRequestError"; | ||
| body: unknown; | ||
|
|
||
| constructor(response: Response, body: unknown) { | ||
| super(response); | ||
| this.body = body; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BadRequestError body property is set but never readLow Severity The Additional Locations (1)Reviewed by Cursor Bugbot for commit 86370a7. Configure here. |
||
| } | ||
| } | ||
| export class NotFoundRequestError extends RequestError { | ||
| name = "NotFoundRequestError"; | ||
|
|
||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this retry necessary? How often does it help to recover from the error state? Maybe we could just throw error and prompt user to retry?