Skip to content

Commit ba3f062

Browse files
kanadguptakanadgupta
authored
feat(docs/migrate): migration stats (#1230)
| 🚥 Resolves RM-12506 | | :------------------- | ## 🧰 Changes adds migration stats reporting to the `docs:migrate` command. this was a big exercise in strong types to ensure that plugin authors have enough guardrails when writing these stats for themselves. as part of this: - [x] added a bunch of types and a tiny bit of logic (which is technically a breaking change) to the `pre_markdown_validation` hook return types so it includes both the transformed pages as well as the stats results - [x] added additional type safety to our syncing results (so we can use these more effectively in downstream plugins) - [x] added a hidden `--max-errors` flag (inspired in part by [eslint's `--max-warnings` flag](https://eslint.org/docs/latest/use/command-line-interface#--max-warnings)) to `docs:upload` so users/plugin authors have the ability to control how errors are returned ## 🧬 QA & Testing do tests + types pass?
1 parent 36b30b9 commit ba3f062

File tree

8 files changed

+407
-49
lines changed

8 files changed

+407
-49
lines changed

‎__tests__/commands/docs/__snapshots__/upload.test.ts.snap‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,72 @@ exports[`rdme docs upload > given that the file path is a directory > should han
206206
}
207207
`;
208208
209+
exports[`rdme docs upload > given that the file path is a directory > should handle a mix of creates and updates and failures and skipped files and not error out with \`max-errors\` flag 1`] = `
210+
{
211+
"result": {
212+
"created": [
213+
{
214+
"filePath": "__tests__/__fixtures__/docs/mixed-docs/invalid-attributes.md",
215+
"response": {},
216+
"result": "created",
217+
"slug": "invalid-attributes",
218+
},
219+
],
220+
"failed": [
221+
{
222+
"error": [APIv2Error: The ReadMe API responded with an unexpected error. Please try again and if this issue persists, get in touch with us at support@readme.io.],
223+
"filePath": "__tests__/__fixtures__/docs/mixed-docs/new-doc-slug.mdx",
224+
"result": "failed",
225+
"slug": "some-slug",
226+
},
227+
{
228+
"error": [APIv2Error: The ReadMe API responded with an unexpected error. Please try again and if this issue persists, get in touch with us at support@readme.io.],
229+
"filePath": "__tests__/__fixtures__/docs/mixed-docs/simple-doc.md",
230+
"result": "failed",
231+
"slug": "simple-doc",
232+
},
233+
],
234+
"skipped": [
235+
{
236+
"filePath": "__tests__/__fixtures__/docs/mixed-docs/doc-sans-attributes.md",
237+
"result": "skipped",
238+
"slug": "doc-sans-attributes",
239+
},
240+
],
241+
"updated": [
242+
{
243+
"filePath": "__tests__/__fixtures__/docs/mixed-docs/legacy-category.md",
244+
"response": {},
245+
"result": "updated",
246+
"slug": "legacy-category",
247+
},
248+
],
249+
},
250+
"stderr": " › Warning: This command is in an experimental alpha and is likely to change.
251+
› Use at your own risk!
252+
- 🔍 Looking for Markdown files in the \`__tests__/__fixtures__/docs/mixed-docs\` directory...
253+
✔ 🔍 Looking for Markdown files in the \`__tests__/__fixtures__/docs/mixed-docs\` directory... 5 file(s) found!
254+
- 🔬 Validating frontmatter data...
255+
⚠ 🔬 Validating frontmatter data... issues found in 2 file(s).
256+
› Warning: 1 file(s) have issues that cannot be fixed automatically. The
257+
› upload will proceed but we recommend addressing these issues. Please get
258+
› in touch with us at support@readme.io if you need a hand.
259+
- 🚀 Uploading files to ReadMe...
260+
✖ 🚀 Uploading files to ReadMe... 2 file(s) failed.
261+
",
262+
"stdout": "🌱 Successfully created 1 page(s) in ReadMe:
263+
- invalid-attributes (__tests__/__fixtures__/docs/mixed-docs/invalid-attributes.md)
264+
🔄 Successfully updated 1 page(s) in ReadMe:
265+
- legacy-category (__tests__/__fixtures__/docs/mixed-docs/legacy-category.md)
266+
⏭️ Skipped 1 page(s) in ReadMe due to no frontmatter data:
267+
- doc-sans-attributes (__tests__/__fixtures__/docs/mixed-docs/doc-sans-attributes.md)
268+
🚨 Received errors when attempting to upload 2 page(s):
269+
- __tests__/__fixtures__/docs/mixed-docs/new-doc-slug.mdx: The ReadMe API responded with an unexpected error. Please try again and if this issue persists, get in touch with us at support@readme.io.
270+
- __tests__/__fixtures__/docs/mixed-docs/simple-doc.md: The ReadMe API responded with an unexpected error. Please try again and if this issue persists, get in touch with us at support@readme.io.
271+
",
272+
}
273+
`;
274+
209275
exports[`rdme docs upload > given that the file path is a directory > should handle complex frontmatter 1`] = `
210276
{
211277
"result": {
@@ -718,3 +784,33 @@ exports[`rdme docs upload > given that the file path is a single file > should h
718784
",
719785
}
720786
`;
787+
788+
exports[`rdme docs upload > given that the file path is a single file > should not throw an error if \`max-errors\` flag is set to -1 1`] = `
789+
{
790+
"result": {
791+
"created": [],
792+
"failed": [
793+
{
794+
"error": [APIv2Error: ReadMe API error: bad request
795+
796+
something went so so wrong],
797+
"filePath": "__tests__/__fixtures__/docs/new-docs/new-doc.md",
798+
"result": "failed",
799+
"slug": "new-doc",
800+
},
801+
],
802+
"skipped": [],
803+
"updated": [],
804+
},
805+
"stderr": " › Warning: This command is in an experimental alpha and is likely to change.
806+
› Use at your own risk!
807+
- 🔬 Validating frontmatter data...
808+
✔ 🔬 Validating frontmatter data... no issues found!
809+
- 🚀 Uploading files to ReadMe...
810+
✖ 🚀 Uploading files to ReadMe... 1 file(s) failed.
811+
",
812+
"stdout": "🚨 Received errors when attempting to upload 1 page(s):
813+
- __tests__/__fixtures__/docs/new-docs/new-doc.md: bad request
814+
",
815+
}
816+
`;

‎__tests__/commands/docs/upload.test.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,21 @@ describe('rdme docs upload', () => {
389389
mock.done();
390390
});
391391

392+
it('should not throw an error if `max-errors` flag is set to -1', async () => {
393+
const mock = getAPIv2Mock({ authorization }).get('/versions/stable/guides/new-doc').reply(500, {
394+
title: 'bad request',
395+
detail: 'something went so so wrong',
396+
status: 500,
397+
});
398+
399+
const result = await run(['__tests__/__fixtures__/docs/new-docs/new-doc.md', '--key', key, '--max-errors', '-1']);
400+
401+
expect(result).toMatchSnapshot();
402+
expect(fs.writeFileSync).not.toHaveBeenCalled();
403+
404+
mock.done();
405+
});
406+
392407
it('should error out if the file does not exist', async () => {
393408
const result = await run(['non-existent-file', '--key', key]);
394409

@@ -592,6 +607,53 @@ describe('rdme docs upload', () => {
592607
mock.done();
593608
});
594609

610+
it('should handle a mix of creates and updates and failures and skipped files and not error out with `max-errors` flag', async () => {
611+
const mock = getAPIv2Mock({ authorization })
612+
.get('/versions/stable/guides/invalid-attributes')
613+
.reply(404)
614+
.post('/versions/stable/guides', {
615+
category: { uri: '/versions/stable/categories/guides/some-category-uri', 'is-this-a-valid-property': 'nope' },
616+
slug: 'invalid-attributes',
617+
title: 'This is the document title',
618+
content: { body: '\nBody\n' },
619+
})
620+
.reply(201, {})
621+
.get('/versions/stable/guides/legacy-category')
622+
.reply(200)
623+
.patch('/versions/stable/guides/legacy-category', {
624+
category: { uri: '/versions/stable/categories/guides/uri-that-does-not-map-to-5ae122e10fdf4e39bb34db6f' },
625+
title: 'This is the document title',
626+
content: { body: '\nBody\n' },
627+
})
628+
.reply(201, {})
629+
.get('/versions/stable/guides/some-slug')
630+
.reply(404)
631+
.post('/versions/stable/guides', {
632+
slug: 'some-slug',
633+
title: 'This is the document title',
634+
category: { uri: '/versions/stable/categories/guides/some-category-uri' },
635+
content: { body: '\nBody\n' },
636+
})
637+
.reply(500, {})
638+
.get('/versions/stable/guides/simple-doc')
639+
.reply(404)
640+
.post('/versions/stable/guides', {
641+
slug: 'simple-doc',
642+
title: 'This is the document title',
643+
content: { body: '\nBody\n' },
644+
})
645+
.reply(500, {});
646+
647+
prompts.inject([true]);
648+
649+
const result = await run(['__tests__/__fixtures__/docs/mixed-docs', '--key', key, '--max-errors', '10']);
650+
651+
expect(result).toMatchSnapshot();
652+
expect(fs.writeFileSync).toHaveBeenCalledTimes(5);
653+
654+
mock.done();
655+
});
656+
595657
it('should handle a mix of creates and updates and failures and skipped files (dry run)', async () => {
596658
const mock = getAPIv2Mock({ authorization })
597659
.get('/versions/stable/guides/invalid-attributes')

‎src/commands/docs/migrate.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { PluginHooks } from '../../lib/hooks/exported.js';
1+
import type { MigrationStats, PluginHooks } from '../../lib/hooks/exported.js';
22

33
import { Args, Flags, type Hook } from '@oclif/core';
44
import chalk from 'chalk';
@@ -46,7 +46,7 @@ export default class DocsMigrateCommand extends BaseCommand<typeof DocsMigrateCo
4646
}),
4747
};
4848

49-
async run() {
49+
async run(): Promise<{ outputDir: string; stats: MigrationStats }> {
5050
if (!this.flags['hide-experimental-warning']) {
5151
this.warn(alphaNotice);
5252
}
@@ -76,6 +76,16 @@ export default class DocsMigrateCommand extends BaseCommand<typeof DocsMigrateCo
7676
}
7777
});
7878

79+
const stats: MigrationStats = {
80+
migrationOutputDir: outputDir,
81+
results: {},
82+
timestamp: new Date().toISOString(),
83+
};
84+
85+
if (zipResults.zipped) {
86+
stats.unzippedAssetsDir = zipResults.unzippedDir;
87+
}
88+
7989
let unsortedFiles = await findPages.call(this, pathInput);
8090

8191
let transformedByHooks = false;
@@ -89,10 +99,11 @@ export default class DocsMigrateCommand extends BaseCommand<typeof DocsMigrateCo
8999
}
90100

91101
validationHookResults.successes.forEach(success => {
92-
if (success.result && success.result.length) {
102+
if (success.result && success.result.pages.length) {
93103
transformedByHooks = true;
94-
this.log(`🔌 ${success.result.length} Markdown files updated via the \`${success.plugin.name}\` plugin`);
95-
unsortedFiles = success.result;
104+
this.log(`🔌 ${success.result.pages.length} Markdown files updated via the \`${success.plugin.name}\` plugin`);
105+
stats.results[success.plugin.name] = success.result.stats;
106+
unsortedFiles = success.result.pages;
96107
}
97108
});
98109

@@ -193,6 +204,6 @@ export default class DocsMigrateCommand extends BaseCommand<typeof DocsMigrateCo
193204
}
194205
}
195206

196-
return { outputDir };
207+
return { outputDir, stats };
197208
}
198209
}

‎src/commands/docs/upload.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@ import { Args, Flags } from '@oclif/core';
22

33
import BaseCommand from '../../lib/baseCommand.js';
44
import { keyFlag } from '../../lib/flags.js';
5-
import syncPagePath, { type FailedPushResult, type PushResult } from '../../lib/syncPagePath.js';
5+
import syncPagePath, {
6+
type FailedPushResult,
7+
type SkippedPushResult,
8+
type CreatePushResult,
9+
type UpdatePushResult,
10+
} from '../../lib/syncPagePath.js';
611

712
const alphaNotice = 'This command is in an experimental alpha and is likely to change. Use at your own risk!';
813

@@ -54,6 +59,13 @@ export default class DocsUploadCommand extends BaseCommand<typeof DocsUploadComm
5459
description: 'Hides the warning message about this command being in an experimental alpha.',
5560
hidden: true,
5661
}),
62+
'max-errors': Flags.integer({
63+
summary: 'Maximum number of page uploading errors before the command throws an error.',
64+
description:
65+
'By default, this command will respond with a 1 exit code if any number of the Markdown files fail to upload. This flag allows you to set a maximum number of errors before the command fails. For example, if you set this flag to `5`, the command will respond with an error if 5 or more errors are encountered. If you do not want the command to fail under any circumstances (this could be useful for plugins where you want to handle the error handling yourself), set this flag to `-1`.',
66+
default: 0,
67+
hidden: true,
68+
}),
5769
'skip-validation': Flags.boolean({
5870
description:
5971
'Skips the pre-upload validation of the Markdown files. This flag can be a useful escape hatch but its usage is not recommended.',
@@ -67,10 +79,10 @@ export default class DocsUploadCommand extends BaseCommand<typeof DocsUploadComm
6779
};
6880

6981
async run(): Promise<{
70-
created: PushResult[];
82+
created: CreatePushResult[];
7183
failed: FailedPushResult[];
72-
skipped: PushResult[];
73-
updated: PushResult[];
84+
skipped: SkippedPushResult[];
85+
updated: UpdatePushResult[];
7486
}> {
7587
if (!this.flags['hide-experimental-warning']) {
7688
this.warn(alphaNotice);

‎src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export const COMMANDS = {
4141
'openapi:validate': OpenAPIValidateCommand,
4242

4343
whoami: WhoAmICommand,
44-
};
44+
} as const;
4545

4646
export type CommandClass = ValueOf<typeof COMMANDS>;
4747

0 commit comments

Comments
 (0)