Skip to content

[WIP] Migrate 'entra administrativeunit remove' to Zod #6813

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import assert from 'assert';
import sinon from 'sinon';
import { z } from 'zod';
import auth from '../../../../Auth.js';
import commands from '../../commands.js';
import request from '../../../../request.js';
import { telemetry } from '../../../../telemetry.js';
import { cli } from '../../../../cli/cli.js';
import { CommandInfo } from '../../../../cli/CommandInfo.js';
import { Logger } from '../../../../cli/Logger.js';
import { CommandError } from '../../../../Command.js';
import { pid } from '../../../../utils/pid.js';
import { session } from '../../../../utils/session.js';
import { sinonUtil } from '../../../../utils/sinonUtil.js';
import { cli } from '../../../../cli/cli.js';
import { CommandInfo } from '../../../../cli/CommandInfo.js';
import command from './administrativeunit-remove.js';
import { telemetry } from '../../../../telemetry.js';
import request from '../../../../request.js';
import { entraAdministrativeUnit } from '../../../../utils/entraAdministrativeUnit.js';
import commands from '../../commands.js';
import command from './administrativeunit-remove.js';

describe(commands.ADMINISTRATIVEUNIT_REMOVE, () => {
const administrativeUnitId = 'fc33aa61-cf0e-46b6-9506-f633347202ab';
Expand All @@ -21,6 +22,7 @@ describe(commands.ADMINISTRATIVEUNIT_REMOVE, () => {
let log: string[];
let logger: Logger;
let commandInfo: CommandInfo;
let commandOptionsSchema: z.ZodTypeAny;
let promptIssued: boolean;

before(() => {
Expand All @@ -30,6 +32,7 @@ describe(commands.ADMINISTRATIVEUNIT_REMOVE, () => {
sinon.stub(session, 'getId').returns('');
auth.connection.active = true;
commandInfo = cli.getCommandInfo(command);
commandOptionsSchema = commandInfo.command.getSchemaToParse()!;
});

beforeEach(() => {
Expand Down Expand Up @@ -75,6 +78,40 @@ describe(commands.ADMINISTRATIVEUNIT_REMOVE, () => {
assert.notStrictEqual(command.description, null);
});

it('fails validation when neither id nor displayName is specified', () => {
const actual = commandOptionsSchema.safeParse({});
assert.strictEqual(actual.success, false);
});

it('passes validation when id is specified', () => {
const actual = commandOptionsSchema.safeParse({
id: administrativeUnitId
});
assert.strictEqual(actual.success, true);
});

it('passes validation when displayName is specified', () => {
const actual = commandOptionsSchema.safeParse({
displayName: displayName
});
assert.strictEqual(actual.success, true);
});

it('fails validation when id is not a valid UUID', () => {
const actual = commandOptionsSchema.safeParse({
id: 'invalid'
});
assert.strictEqual(actual.success, false);
});

it('fails validation when both id and displayName are specified', () => {
const actual = commandOptionsSchema.safeParse({
id: administrativeUnitId,
displayName: displayName
});
assert.strictEqual(actual.success, false);
});

it('removes the specified administrative unit by id without prompting for confirmation', async () => {
const deleteRequestStub = sinon.stub(request, 'delete').callsFake(async (opts) => {
if (opts.url === `https://graph.microsoft.com/v1.0/directory/administrativeUnits/${administrativeUnitId}`) {
Expand All @@ -84,7 +121,12 @@ describe(commands.ADMINISTRATIVEUNIT_REMOVE, () => {
throw 'Invalid request';
});

await command.action(logger, { options: { id: administrativeUnitId, force: true } });
await command.action(logger, {
options: commandOptionsSchema.parse({
id: administrativeUnitId,
force: true
})
});
assert(deleteRequestStub.called);
});

Expand All @@ -102,7 +144,11 @@ describe(commands.ADMINISTRATIVEUNIT_REMOVE, () => {
sinonUtil.restore(cli.promptForConfirmation);
sinon.stub(cli, 'promptForConfirmation').resolves(true);

await command.action(logger, { options: { displayName: displayName } });
await command.action(logger, {
options: commandOptionsSchema.parse({
displayName: displayName
})
});
assert(deleteRequestStub.called);
});

Expand All @@ -126,30 +172,33 @@ describe(commands.ADMINISTRATIVEUNIT_REMOVE, () => {
throw 'Invalid request';
});

await assert.rejects(command.action(logger, { options: { id: administrativeUnitId, force: true } }),
await assert.rejects(command.action(logger, {
options: commandOptionsSchema.parse({
id: administrativeUnitId,
force: true
})
}),
new CommandError(error.error.message));
});

it('prompts before removing the specified administrative unit when confirm option not passed', async () => {
await command.action(logger, { options: { id: administrativeUnitId } });
await command.action(logger, {
options: commandOptionsSchema.parse({
id: administrativeUnitId
})
});

assert(promptIssued);
});

it('aborts removing administrative unit when prompt not confirmed', async () => {
const deleteSpy = sinon.stub(request, 'delete').resolves();

await command.action(logger, { options: { id: administrativeUnitId } });
await command.action(logger, {
options: commandOptionsSchema.parse({
id: administrativeUnitId
})
});
assert(deleteSpy.notCalled);
});

it('fails validation if id is not a valid GUID', async () => {
const actual = await command.validate({ options: { id: 'invalid' } }, commandInfo);
assert.notStrictEqual(actual, true);
});

it('passes validation when id is a valid GUID', async () => {
const actual = await command.validate({ options: { id: administrativeUnitId } }, commandInfo);
assert.strictEqual(actual, true);
});
});
Original file line number Diff line number Diff line change
@@ -1,22 +1,27 @@
import { z } from 'zod';
import { cli } from '../../../../cli/cli.js';
import { Logger } from '../../../../cli/Logger.js';
import { globalOptionsZod } from '../../../../Command.js';
import request, { CliRequestOptions } from '../../../../request.js';
import { entraAdministrativeUnit } from '../../../../utils/entraAdministrativeUnit.js';
import GlobalOptions from "../../../../GlobalOptions.js";
import { Logger } from "../../../../cli/Logger.js";
import { validation } from "../../../../utils/validation.js";
import request, { CliRequestOptions } from "../../../../request.js";
import GraphCommand from "../../../base/GraphCommand.js";
import commands from "../../commands.js";
import { cli } from "../../../../cli/cli.js";
import { zod } from '../../../../utils/zod.js';
import GraphCommand from '../../../base/GraphCommand.js';
import commands from '../../commands.js';

const options = globalOptionsZod
.extend({
id: zod.alias('i', z.string().uuid().optional()),
displayName: zod.alias('n', z.string().optional()),
force: zod.alias('f', z.boolean().optional())
})
.strict();

declare type Options = z.infer<typeof options>;

interface CommandArgs {
options: Options;
}

interface Options extends GlobalOptions {
id?: string;
displayName?: string;
force?: boolean
}

class EntraAdministrativeUnitRemoveCommand extends GraphCommand {
public get name(): string {
return commands.ADMINISTRATIVEUNIT_REMOVE;
Expand All @@ -25,62 +30,18 @@ class EntraAdministrativeUnitRemoveCommand extends GraphCommand {
return 'Removes an administrative unit';
}

constructor() {
super();

this.#initOptions();
this.#initValidators();
this.#initOptionSets();
this.#initTelemetry();
this.#initTypes();
public get schema(): z.ZodTypeAny | undefined {
return options;
}

#initTelemetry(): void {
this.telemetry.push((args: CommandArgs) => {
Object.assign(this.telemetryProperties, {
id: args.options.id !== 'undefined',
displayName: args.options.displayName !== 'undefined',
force: !!args.options.force
public getRefinedSchema(schema: typeof options): z.ZodEffects<any> | undefined {
return schema
.refine(options => options.id || options.displayName, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should also throw an error if both options are used at the same time.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added mutual exclusion validation using an additional refine constraint that prevents both id and displayName from being specified together. Also added corresponding test case. Changes committed in 72fda67.

message: 'Specify either id or displayName'
})
.refine(options => !(options.id && options.displayName), {
message: 'Specify either id or displayName but not both'
});
});
}

#initOptions(): void {
this.options.unshift(
{
option: '-i, --id [id]'
},
{
option: '-n, --displayName [displayName]'
},
{
option: '-f, --force'
}
);
}

#initOptionSets(): void {
this.optionSets.push(
{
options: ['id', 'displayName']
}
);
}

#initValidators(): void {
this.validators.push(
async (args: CommandArgs) => {
if (args.options.id && !validation.isValidGuid(args.options.id)) {
return `${args.options.id} is not a valid GUID for option id.`;
}

return true;
}
);
}

#initTypes(): void {
this.types.string.push('id', 'displayName');
}

public async commandAction(logger: Logger, args: CommandArgs): Promise<void> {
Expand Down