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 2 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,32 @@ 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('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 Down Expand Up @@ -142,14 +171,4 @@ describe(commands.ADMINISTRATIVEUNIT_REMOVE, () => {
await command.action(logger, { options: { 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,15 @@ 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'
});
});
}

#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