Add subscription migration list command - #8478
Conversation
Assisted-By: devx/0b604c20-c9ef-42f2-a0b6-f4cb3118b82f
Assisted-By: devx/0b604c20-c9ef-42f2-a0b6-f4cb3118b82f
Assisted-By: devx/0b604c20-c9ef-42f2-a0b6-f4cb3118b82f
Assisted-By: devx/0b604c20-c9ef-42f2-a0b6-f4cb3118b82f
Assisted-By: devx/0b604c20-c9ef-42f2-a0b6-f4cb3118b82f
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/fs.d.ts@@ -86,6 +86,7 @@ export declare function appendFile(path: string, content: string): Promise<void>
export declare function appendFileSync(path: string, data: string): void;
export interface WriteOptions {
encoding: BufferEncoding;
+ flag?: 'w' | 'wx';
}
/**
* Writes content to file at path.
|
| ...sharedFlags, | ||
| output: Flags.string({ | ||
| description: 'Path to write the subscription export.', | ||
| env: 'SHOPIFY_FLAG_OUTPUT', |
There was a problem hiding this comment.
UNIXey nit: Do we need --output / --force? Shouldn't folks just be able to run subscription migrations list > file.csv?
There was a problem hiding this comment.
I'm going to remove --output and --force, but for CSV output specifically I'm also going to make it stream each page of results. If we're writing CSV data to stdout then we can stream each page without risking a malformed csv output, but with --json we can't safely do that. So CSV output can stream, JSON output won't for now just to avoid a failed invocation leaving behind a malformed JSON file.
Assisted-By: devx/0b604c20-c9ef-42f2-a0b6-f4cb3118b82f
| // Pages must be requested sequentially because each request depends on the previous opaque cursor. | ||
| // eslint-disable-next-line no-await-in-loop | ||
| const page = await getPage({clientId, first: PAGE_SIZE, after, status}) | ||
| if (page === null) throw new AbortError('App not found') |
There was a problem hiding this comment.
Could this throw a domain-specific error and let List.run() translate it to AbortError? Throwing AbortError makes three CLI-specific choices: how to show the error, which exit status to use, and how to encode the JSON error. Ideally those choices belong at the command boundary rather than in the internal execution.
dmerand
left a comment
There was a problem hiding this comment.
This LGTM from a code standpoint. My comments about output format are non-blocking, though my opinion is that we don't need so much code to handle output formatting when that's already handled well-enough by the OS.
What this changes
Adds the subscription migration command:
The command:
migratableAppSubscriptionsPartners CLI GraphQL query;UNSCHEDULED,SCHEDULED, orMIGRATED;--json;The read model can preserve historical
NONEnotification values returned by the API without re-enablingNONEas a schedule-input option.Examples
Verification