Skip to content
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

feat: add support for templates delete command #82

Closed
wants to merge 4 commits into from
Closed
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
225 changes: 225 additions & 0 deletions src/commands/templates/delete.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
/* eslint-disable @typescript-eslint/member-delimiter-style */

Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow the local conventions.

Suggested change
/* eslint-disable @typescript-eslint/member-delimiter-style */

import { confirm, input, select } from "@inquirer/prompts";
import ora from "ora";
import { ServerClient } from "postmark";
import { TemplateTypes } from "postmark/dist/client/models";

import {
fatalError,
log,
logError,
pluralize,
validateToken,
} from "../../utils";

interface TemplateDeleteArguments {
serverToken: string;
requestHost: string;
}

export interface TemplateListOptions {
requestHost: string;
sourceServer: string;
templateType: TemplateTypes | undefined;
}

export async function handler(args: TemplateDeleteArguments): Promise<void> {
const serverToken = await validateToken(args.serverToken);
const requestHost = args.requestHost;
deletePrompts(serverToken, requestHost);
}

/**
* Ask user a series of questions before deleting templates.
*/

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

async function deletePrompts(
serverToken: string,
requestHost: string
): Promise<void> {
const choice = await select({
message: "Choose how you want to delete templates:",
choices: [
{
name: "Delete templates by id or alias",
description:
"NOTE: mixing ids and aliases on the same request is NOT supported",
value: "byIdOrAlias",
},
{
name: "Delete all templates",
value: "all",
},
],
});

/**
* if the user has selected to delete templates by id
*/

Comment on lines +57 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/**
* if the user has selected to delete templates by id
*/

This doesn't add much, the code is self-explanatory.

if (choice === "byIdOrAlias") {
// TODO we probably want an actual validation
// against ids/aliases that exist in the server
Comment on lines +62 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say we don't need to validate that. Ideally running the command with a non-existing ID should display a warning.

const validateUserInput = async (input: string) => {
// do not allow empty strings
if (input) return true;
return false;
};

const userInput = await input({
message:
"Enter template id(s)/alias(es) - separate with commas if multiple:",
validate: validateUserInput,
Copy link
Contributor

@tomek-ac tomek-ac Feb 1, 2024

Choose a reason for hiding this comment

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

Maybe we could inline the validation if it's so simple?

Suggested change
validate: validateUserInput,
validate: (s) => s.trim().length > 0, // don't allow empty input

});

const templateIdsOrAliases = userInput.split(",");

// if user has entered at least one id
if (templateIdsOrAliases.length) {
return deleteTemplates(templateIdsOrAliases, {
requestHost: requestHost,
sourceServer: serverToken,
templateType: undefined,
});
}
}

/**
* if the user has selected to delete all templates
*/

Comment on lines +88 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/**
* if the user has selected to delete all templates
*/

again, the code is self-explanatory. Comments are most valuable when they explain the "why" leaving the "what" to the code.

if (choice === "all") {
let templateType, confirmed, inputIsValid;

templateType = await select({
message: "Which template type do you want to delete?",
choices: [
{
name: "Templates",
value: TemplateTypes.Standard,
},
{
description:
"NOTE: Layouts can only be deleted if not in use by any Templates",
name: "Layouts",
value: TemplateTypes.Layout,
},
],
});

if (templateType) {
confirmed = await confirm({
default: false,
message: `Delete ALL templates? Are you sure?`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
message: `Delete ALL templates? Are you sure?`,
message: 'Delete ALL templates? Are you sure?',

});
}

// user has to type the exact sentence to proceed
const validateInput = async (input: string) =>
input !== "delete all templates" ? false : true;

if (confirmed) {
inputIsValid = await input({
message: 'Enter "delete all templates" to confirm:',
validate: validateInput,
});
}
Comment on lines +118 to +127
Copy link
Contributor

Choose a reason for hiding this comment

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

we could inline the validation here too:

Suggested change
// user has to type the exact sentence to proceed
const validateInput = async (input: string) =>
input !== "delete all templates" ? false : true;
if (confirmed) {
inputIsValid = await input({
message: 'Enter "delete all templates" to confirm:',
validate: validateInput,
});
}
if (confirmed) {
inputIsValid = await input({
message: 'Enter "delete all templates" to confirm:',
validate: (s) => s === 'delete all templates',
});
}


if (inputIsValid) {
return deleteTemplates([], {
templateType: templateType,
sourceServer: serverToken,
requestHost: requestHost,
});
}
}
}

/**
* Delete templates from PM
*/

async function deleteTemplates(
templateIdsOrAliases: string[],
options: TemplateListOptions
) {
const { requestHost, sourceServer, templateType } = options;

// keep track of templates deleted
let totalDeleted = 0;

const spinner = ora("Deleting templates from Postmark...");
spinner.start();

const client = new ServerClient(sourceServer);

if (requestHost !== undefined && requestHost !== "") {
client.setClientOptions({ requestHost });
}

try {
let templates = [];
let totalCount = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we use templates.length instead of adding another variable?


// if user has provided ids or aliases we use them
if (templateIdsOrAliases.length) {
for (const idOrAlias of templateIdsOrAliases) {
const template = await client.getTemplate(idOrAlias);
templates.push(template);
}
totalCount = templates.length;
} else {
// otherwise fetch all templates from server
const response = await client.getTemplates({
count: 300,
templateType,
});
Comment on lines +174 to +177
Copy link
Contributor

Choose a reason for hiding this comment

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

In 99% of the cases we would get all the templates with this call as Postmark allows for 100 templates per Server by default. But to make it a bit more resilient and correct, we should fetch all the templates, not the only first batch. We'd need a loop to fill the templates array.

templates = response.Templates;
totalCount = response.TotalCount;
}

if (!totalCount) {
spinner.stop();
return fatalError("There are no templates on this server.");
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably shouldn't exit with an error if there are no templates on the server. I guess this would only happen if the user requested to delete all the servers. In such case the user's intention is to not have templates so when there are not any, then we should be good.

} else {
spinner.text = `Deleting ${totalCount} templates from Postmark...`;

for (const template of templates) {
spinner.text = `Deleting template: ${template.Alias || template.Name}`;

// TemplateId is always defined so use it instead of Alias
const id = template.TemplateId;

try {
await client.deleteTemplate(id);

spinner.text = `Template: ${
template.Alias || template.Name
} removed.`;

totalDeleted++;
} catch (e) {
spinner.stop();
logError(e);
}
}

// Show feedback when finished deleting templates
if (totalDeleted === totalCount) {
spinner.stop();
log(
`All finished! ${totalDeleted} ${pluralize(
totalDeleted,
"template has",
"templates have"
)} been deleted.`,
{ color: "green" }
);
}
}
} catch (err) {
spinner.stop();
return fatalError(err);
}
}