Skip to content

Commit 30d4ed5

Browse files
cs-rajcs-raj
authored
Merge pull request #31 from contentstack/fix/DX-5326
fix: Audit fix progress bar and overwrite confirmation UX
2 parents f926ee9 + 9d88829 commit 30d4ed5

File tree

20 files changed

+2375
-71
lines changed

20 files changed

+2375
-71
lines changed

‎packages/contentstack-audit/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@
7373
"test": "mocha --forbid-only \"test/**/*.test.ts\"",
7474
"version": "oclif readme && git add README.md",
7575
"clean": "rm -rf ./lib ./node_modules tsconfig.tsbuildinfo oclif.manifest.json",
76-
"test:unit:report": "nyc --extension .ts mocha --forbid-only \"test/unit/**/*.test.ts\"",
77-
"test:unit": "mocha --timeout 10000 --forbid-only \"test/unit/**/*.test.ts\""
76+
"test:unit:report": "nyc --extension .ts mocha --forbid-only --file test/unit/logger-config.js \"test/unit/**/*.test.ts\"",
77+
"test:unit": "mocha --timeout 10000 --forbid-only --file test/unit/logger-config.js \"test/unit/**/*.test.ts\""
7878
},
7979
"engines": {
8080
"node": ">=16"

‎packages/contentstack-audit/src/modules/assets.ts‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export default class Assets extends BaseClass {
2727
protected schema: ContentTypeStruct[] = [];
2828
protected missingEnvLocales: Record<string, any> = {};
2929
public moduleName: keyof typeof auditConfig.moduleConfig;
30+
private fixOverwriteConfirmed: boolean | null = null;
3031

3132
constructor({ fix, config, moduleName }: ModuleConstructorParam & CtConstructorParam) {
3233
super({ config });
@@ -161,11 +162,17 @@ export default class Assets extends BaseClass {
161162

162163
if (this.fix) {
163164
log.debug('Fix mode enabled, checking write permissions', this.config.auditContext);
164-
if (!this.config.flags['copy-dir'] && !this.config.flags['external-config']?.skipConfirm) {
165-
log.debug(`Asking user for confirmation to write fix content (--yes flag: ${this.config.flags.yes})`, this.config.auditContext);
166-
canWrite = this.config.flags.yes || (await cliux.confirm(commonMsg.FIX_CONFIRMATION));
165+
if (this.config.flags['copy-dir'] || this.config.flags['external-config']?.skipConfirm || this.config.flags.yes) {
166+
this.fixOverwriteConfirmed = true;
167+
log.debug('Skipping confirmation due to copy-dir, external-config, or yes flags', this.config.auditContext);
168+
} else if (this.fixOverwriteConfirmed !== null) {
169+
canWrite = this.fixOverwriteConfirmed;
170+
log.debug(`Using cached overwrite confirmation: ${canWrite}`, this.config.auditContext);
167171
} else {
168-
log.debug('Skipping confirmation due to copy-dir or external-config flags', this.config.auditContext);
172+
log.debug(`Asking user for confirmation to write fix content (--yes flag: ${this.config.flags.yes})`, this.config.auditContext);
173+
this.completeProgress(true);
174+
canWrite = await cliux.confirm(commonMsg.FIX_CONFIRMATION);
175+
this.fixOverwriteConfirmed = canWrite;
169176
}
170177

171178
if (canWrite) {
@@ -248,13 +255,16 @@ export default class Assets extends BaseClass {
248255
if (this.progressManager) {
249256
this.progressManager.tick(true, `asset: ${assetUid}`, null);
250257
}
251-
258+
252259
if (this.fix) {
253260
log.debug(`Fixing asset ${assetUid}`, this.config.auditContext);
254261
log.info($t(auditFixMsg.ASSET_FIX, { uid: assetUid }), this.config.auditContext);
255-
await this.writeFixContent(`${basePath}/${indexer[fileIndex]}`, this.assets);
256262
}
257263
}
264+
265+
if (this.fix) {
266+
await this.writeFixContent(`${basePath}/${indexer[fileIndex]}`, this.assets);
267+
}
258268
}
259269

260270
log.debug(`Asset reference validation completed. Processed ${Object.keys(this.missingEnvLocales).length} assets with issues`, this.config.auditContext);

‎packages/contentstack-audit/src/modules/content-types.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,15 @@ export default class ContentType extends BaseClass {
222222
let canWrite = true;
223223

224224
if (!this.inMemoryFix && this.fix) {
225+
if (Array.isArray(this.schema) && this.schema.length === 0) {
226+
log.debug('No schemas to write, skipping writeFixContent', this.config.auditContext);
227+
return;
228+
}
229+
225230
log.debug('Fix mode enabled, checking write permissions', this.config.auditContext);
226231
if (!this.config.flags['copy-dir'] && !this.config.flags['external-config']?.skipConfirm) {
227232
log.debug('Asking user for confirmation to write fix content', this.config.auditContext);
233+
this.completeProgress(true);
228234
canWrite = this.config.flags.yes ?? (await cliux.confirm(commonMsg.FIX_CONFIRMATION));
229235
} else {
230236
log.debug('Skipping confirmation due to copy-dir or external-config flags', this.config.auditContext);

‎packages/contentstack-audit/src/modules/custom-roles.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ export default class CustomRoles extends BaseClass {
249249
log.debug('Skipping confirmation due to copy-dir, external-config, or yes flags', this.config.auditContext);
250250
} else {
251251
log.debug('Asking user for confirmation to write fix content', this.config.auditContext);
252+
this.completeProgress(true);
252253
}
253254

254255
const canWrite = skipConfirm || (await cliux.confirm(commonMsg.FIX_CONFIRMATION));

‎packages/contentstack-audit/src/modules/entries.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export default class Entries extends BaseClass {
6060
public environments: string[] = [];
6161
public entryMetaData: Record<string, any>[] = [];
6262
public moduleName: keyof typeof auditConfig.moduleConfig = 'entries';
63+
private fixOverwriteConfirmed: boolean | null = null;
6364

6465
constructor({ fix, config, moduleName, ctSchema, gfSchema }: ModuleConstructorParam & CtConstructorParam) {
6566
super({ config });
@@ -541,14 +542,21 @@ export default class Entries extends BaseClass {
541542

542543
const skipConfirm = this.config.flags['copy-dir'] || this.config.flags['external-config']?.skipConfirm;
543544

544-
if (skipConfirm) {
545-
log.debug('Skipping confirmation due to copy-dir or external-config flags', this.config.auditContext);
545+
let canWrite: boolean;
546+
if (skipConfirm || this.config.flags.yes) {
547+
canWrite = true;
548+
this.fixOverwriteConfirmed = true;
549+
log.debug('Skipping confirmation due to copy-dir, external-config, or yes flags', this.config.auditContext);
550+
} else if (this.fixOverwriteConfirmed !== null) {
551+
canWrite = this.fixOverwriteConfirmed;
552+
log.debug(`Using cached overwrite confirmation: ${canWrite}`, this.config.auditContext);
546553
} else {
547554
log.debug('Asking user for confirmation to write fix content', this.config.auditContext);
555+
this.completeProgress(true);
556+
canWrite = await cliux.confirm(commonMsg.FIX_CONFIRMATION);
557+
this.fixOverwriteConfirmed = canWrite;
548558
}
549559

550-
const canWrite = skipConfirm || this.config.flags.yes || (await cliux.confirm(commonMsg.FIX_CONFIRMATION));
551-
552560
if (canWrite) {
553561
log.debug(`Writing fixed entries to: ${filePath}`, this.config.auditContext);
554562
writeFileSync(filePath, JSON.stringify(schema));

‎packages/contentstack-audit/src/modules/extensions.ts‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,19 @@ export default class Extensions extends BaseClass {
172172
? JSON.parse(readFileSync(this.extensionsPath, 'utf8'))
173173
: {};
174174
log.debug(`Loaded ${Object.keys(newExtensionSchema).length} existing extensions`, this.config.auditContext);
175-
175+
176+
let userConfirm: boolean;
177+
if (
178+
this.config.flags['copy-dir'] ||
179+
this.config.flags['external-config']?.skipConfirm ||
180+
this.config.flags.yes
181+
) {
182+
userConfirm = true;
183+
} else {
184+
this.completeProgress(true);
185+
userConfirm = await cliux.confirm(commonMsg.FIX_CONFIRMATION);
186+
}
187+
176188
for (const ext of missingCtInExtensions) {
177189
const { uid, title } = ext;
178190
log.debug(`Fixing extension: ${title} (${uid})`, this.config.auditContext);
@@ -187,8 +199,7 @@ export default class Extensions extends BaseClass {
187199
} else {
188200
log.debug(`Extension ${title} has no valid content types or scope not found`, this.config.auditContext);
189201
cliux.print($t(commonMsg.EXTENSION_FIX_WARN, { title: title, uid }), { color: 'yellow' });
190-
const shouldDelete = this.config.flags.yes || (await cliux.confirm(commonMsg.EXTENSION_FIX_CONFIRMATION));
191-
if (shouldDelete) {
202+
if (userConfirm) {
192203
log.debug(`Deleting extension: ${title} (${uid})`, this.config.auditContext);
193204
delete newExtensionSchema[uid];
194205
} else {
@@ -198,23 +209,33 @@ export default class Extensions extends BaseClass {
198209
}
199210

200211
log.debug(`Extensions scope fix completed, writing updated schema`, this.config.auditContext);
201-
await this.writeFixContent(newExtensionSchema);
212+
await this.writeFixContent(newExtensionSchema, userConfirm);
202213
}
203214

204-
async writeFixContent(fixedExtensions: Record<string, Extension>) {
215+
async writeFixContent(fixedExtensions: Record<string, Extension>, preConfirmed?: boolean) {
205216
log.debug(`Writing fix content for ${Object.keys(fixedExtensions).length} extensions`, this.config.auditContext);
206217
log.debug(`Fix mode: ${this.fix}`, this.config.auditContext);
207218
log.debug(`Copy directory flag: ${this.config.flags['copy-dir']}`, this.config.auditContext);
208219
log.debug(`External config skip confirm: ${this.config.flags['external-config']?.skipConfirm}`, this.config.auditContext);
209220
log.debug(`Yes flag: ${this.config.flags.yes}`, this.config.auditContext);
210221

211-
if (
212-
this.fix &&
213-
(this.config.flags['copy-dir'] ||
214-
this.config.flags['external-config']?.skipConfirm ||
215-
this.config.flags.yes ||
216-
(await cliux.confirm(commonMsg.FIX_CONFIRMATION)))
222+
let shouldWrite: boolean;
223+
if (!this.fix) {
224+
shouldWrite = false;
225+
} else if (preConfirmed !== undefined) {
226+
shouldWrite = preConfirmed;
227+
} else if (
228+
this.config.flags['copy-dir'] ||
229+
this.config.flags['external-config']?.skipConfirm ||
230+
this.config.flags.yes
217231
) {
232+
shouldWrite = true;
233+
} else {
234+
this.completeProgress(true);
235+
shouldWrite = await cliux.confirm(commonMsg.FIX_CONFIRMATION);
236+
}
237+
238+
if (shouldWrite) {
218239
const outputPath = join(this.folderPath, this.config.moduleConfig[this.moduleName].fileName);
219240
log.debug(`Writing fixed extensions to: ${outputPath}`, this.config.auditContext);
220241
log.debug(`Extensions to write: ${Object.keys(fixedExtensions).join(', ')}`, this.config.auditContext);

‎packages/contentstack-audit/src/modules/field_rules.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,7 @@ export default class FieldRule extends BaseClass {
474474
if (this.fix) {
475475
if (!this.config.flags['copy-dir'] && !this.config.flags['external-config']?.skipConfirm) {
476476
log.debug(`Asking user for confirmation to write fix content`, this.config.auditContext);
477+
this.completeProgress(true);
477478
canWrite = this.config.flags.yes ?? (await cliux.confirm(commonMsg.FIX_CONFIRMATION));
478479
log.debug(`User confirmation: ${canWrite}`, this.config.auditContext);
479480
} else {

‎packages/contentstack-audit/src/modules/workflows.ts‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,22 @@ export default class Workflows extends BaseClass {
194194

195195
log.debug(`Loaded ${Object.keys(newWorkflowSchema).length} workflows for fixing`, this.config.auditContext);
196196

197-
if (Object.keys(newWorkflowSchema).length !== 0) {
197+
const hasWorkflowsToFix = Object.keys(newWorkflowSchema).length !== 0;
198+
let userConfirm: boolean;
199+
if (!hasWorkflowsToFix) {
200+
userConfirm = true;
201+
} else if (
202+
this.config.flags['copy-dir'] ||
203+
this.config.flags['external-config']?.skipConfirm ||
204+
this.config.flags.yes
205+
) {
206+
userConfirm = true;
207+
} else {
208+
this.completeProgress(true);
209+
userConfirm = await cliux.confirm(commonMsg.FIX_CONFIRMATION);
210+
}
211+
212+
if (hasWorkflowsToFix) {
198213
log.debug(`Processing ${this.workflowSchema.length} workflows for fixes`, this.config.auditContext);
199214

200215
for (const workflow of this.workflowSchema) {
@@ -237,7 +252,7 @@ export default class Workflows extends BaseClass {
237252

238253
cliux.print(warningMessage, { color: 'yellow' });
239254

240-
if (this.config.flags.yes || (await cliux.confirm(commonMsg.WORKFLOW_FIX_CONFIRMATION))) {
255+
if (userConfirm) {
241256
log.debug(`Deleting workflow ${name} (${uid})`, this.config.auditContext);
242257
delete newWorkflowSchema[workflow.uid];
243258
} else {
@@ -250,24 +265,34 @@ export default class Workflows extends BaseClass {
250265
}
251266

252267
log.debug(`Workflow schema fix completed`, this.config.auditContext);
253-
await this.writeFixContent(newWorkflowSchema);
268+
await this.writeFixContent(newWorkflowSchema, userConfirm);
254269
}
255270

256-
async writeFixContent(newWorkflowSchema: Record<string, Workflow>) {
271+
async writeFixContent(newWorkflowSchema: Record<string, Workflow>, preConfirmed?: boolean) {
257272
log.debug(`Writing fix content`, this.config.auditContext);
258273
log.debug(`Fix mode: ${this.fix}`, this.config.auditContext);
259274
log.debug(`Copy directory flag: ${this.config.flags['copy-dir']}`, this.config.auditContext);
260275
log.debug(`External config skip confirm: ${this.config.flags['external-config']?.skipConfirm}`, this.config.auditContext);
261276
log.debug(`Yes flag: ${this.config.flags.yes}`, this.config.auditContext);
262277
log.debug(`Workflows to write: ${Object.keys(newWorkflowSchema).length}`, this.config.auditContext);
263278

264-
if (
265-
this.fix &&
266-
(this.config.flags['copy-dir'] ||
267-
this.config.flags['external-config']?.skipConfirm ||
268-
this.config.flags.yes ||
269-
(await cliux.confirm(commonMsg.FIX_CONFIRMATION)))
279+
let shouldWrite: boolean;
280+
if (!this.fix) {
281+
shouldWrite = false;
282+
} else if (preConfirmed !== undefined) {
283+
shouldWrite = preConfirmed;
284+
} else if (
285+
this.config.flags['copy-dir'] ||
286+
this.config.flags['external-config']?.skipConfirm ||
287+
this.config.flags.yes
270288
) {
289+
shouldWrite = true;
290+
} else {
291+
this.completeProgress(true);
292+
shouldWrite = await cliux.confirm(commonMsg.FIX_CONFIRMATION);
293+
}
294+
295+
if (shouldWrite) {
271296
const outputPath = join(this.folderPath, this.config.moduleConfig[this.moduleName].fileName);
272297
log.debug(`Writing fixed workflows to: ${outputPath}`, this.config.auditContext);
273298

‎packages/contentstack-audit/test/unit/base-command.test.ts‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { resolve } from 'path';
33
import { fancy } from 'fancy-test';
44
import { expect } from 'chai';
55
import { FileTransportInstance } from 'winston/lib/winston/transports';
6-
76
import { BaseCommand } from '../../src/base-command';
87
import { mockLogger } from './mock-logger';
98

@@ -168,4 +167,52 @@ describe('BaseCommand class', () => {
168167
}
169168
});
170169
});
170+
171+
describe('init with external-config', () => {
172+
class CMDCheckConfig extends BaseCommand<typeof CMDCheckConfig> {
173+
async run() {
174+
const sc = this.sharedConfig as Record<string, unknown>;
175+
if (sc.testMergeKey !== undefined) this.log(String(sc.testMergeKey));
176+
if (this.flags['external-config']?.noLog) this.log('noLog');
177+
}
178+
}
179+
180+
fancy
181+
.stdout({ print: process.env.PRINT === 'true' || false })
182+
.stub(winston.transports, 'File', () => fsTransport)
183+
.stub(winston, 'createLogger', createMockWinstonLogger)
184+
.stub(BaseCommand.prototype, 'parse', () =>
185+
Promise.resolve({
186+
args: {},
187+
flags: { 'external-config': { config: { testMergeKey: 'merged' } } },
188+
} as any)
189+
)
190+
.do(() => CMDCheckConfig.run([]))
191+
.do((output: { stdout: string }) => expect(output.stdout).to.include('merged'))
192+
.it('merges external-config.config into sharedConfig when present');
193+
194+
fancy
195+
.stdout({ print: process.env.PRINT === 'true' || false })
196+
.stub(winston.transports, 'File', () => fsTransport)
197+
.stub(winston, 'createLogger', createMockWinstonLogger)
198+
.stub(BaseCommand.prototype, 'parse', () =>
199+
Promise.resolve({
200+
args: {},
201+
flags: { 'external-config': { noLog: true } },
202+
} as any)
203+
)
204+
.do(() => CMDCheckConfig.run([]))
205+
.do((output: { stdout: string }) => expect(output.stdout).to.include('noLog'))
206+
.it('hits noLog branch when external-config.noLog is true');
207+
208+
fancy
209+
.stdout({ print: process.env.PRINT === 'true' || false })
210+
.stub(winston.transports, 'File', () => fsTransport)
211+
.stub(winston, 'createLogger', createMockWinstonLogger)
212+
.stub(BaseCommand.prototype, 'parse', () =>
213+
Promise.resolve({ args: {}, flags: { 'external-config': {} } } as any)
214+
)
215+
.do(() => CMDCheckConfig.run([]))
216+
.it('completes when external-config is empty (no merge, no noLog)');
217+
});
171218
});

‎packages/contentstack-audit/test/unit/logger-config.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* Loaded by Mocha via --file before any test. Forces log config to non-debug
3+
* so the real Logger never enables the debug path and unit tests don't throw
4+
* when user has run: csdx config:set:log --level debug
5+
*/
6+
const cliUtils = require('@contentstack/cli-utilities');
7+
const configHandler = cliUtils.configHandler;
8+
const originalGet = configHandler.get.bind(configHandler);
9+
configHandler.get = function (key) {
10+
if (key === 'log') return { level: 'info' };
11+
return originalGet(key);
12+
};

0 commit comments

Comments
 (0)