feat: enhance booster role functionality with error handling and database migration
This commit is contained in:
@@ -0,0 +1,2 @@
|
||||
ALTER TABLE "booster_roles" ALTER COLUMN "created_at" SET DATA TYPE bigint;--> statement-breakpoint
|
||||
ALTER TABLE "booster_roles" ALTER COLUMN "updated_at" SET DATA TYPE bigint;
|
||||
@@ -0,0 +1,123 @@
|
||||
{
|
||||
"id": "66a1346c-4510-4ec6-a055-a3d0bcf803c3",
|
||||
"prevId": "e654a614-04fe-4432-aee1-659bc4a0475e",
|
||||
"version": "7",
|
||||
"dialect": "postgresql",
|
||||
"tables": {
|
||||
"public.booster_roles": {
|
||||
"name": "booster_roles",
|
||||
"schema": "",
|
||||
"columns": {
|
||||
"guild_id": {
|
||||
"name": "guild_id",
|
||||
"type": "text",
|
||||
"primaryKey": false,
|
||||
"notNull": true
|
||||
},
|
||||
"user_id": {
|
||||
"name": "user_id",
|
||||
"type": "text",
|
||||
"primaryKey": false,
|
||||
"notNull": true
|
||||
},
|
||||
"role_id": {
|
||||
"name": "role_id",
|
||||
"type": "text",
|
||||
"primaryKey": false,
|
||||
"notNull": true
|
||||
},
|
||||
"name": {
|
||||
"name": "name",
|
||||
"type": "text",
|
||||
"primaryKey": false,
|
||||
"notNull": true
|
||||
},
|
||||
"color": {
|
||||
"name": "color",
|
||||
"type": "text",
|
||||
"primaryKey": false,
|
||||
"notNull": false
|
||||
},
|
||||
"icon": {
|
||||
"name": "icon",
|
||||
"type": "text",
|
||||
"primaryKey": false,
|
||||
"notNull": false
|
||||
},
|
||||
"created_at": {
|
||||
"name": "created_at",
|
||||
"type": "bigint",
|
||||
"primaryKey": false,
|
||||
"notNull": true
|
||||
},
|
||||
"updated_at": {
|
||||
"name": "updated_at",
|
||||
"type": "bigint",
|
||||
"primaryKey": false,
|
||||
"notNull": true
|
||||
}
|
||||
},
|
||||
"indexes": {
|
||||
"booster_roles_guild_user_idx": {
|
||||
"name": "booster_roles_guild_user_idx",
|
||||
"columns": [
|
||||
{
|
||||
"expression": "guild_id",
|
||||
"isExpression": false,
|
||||
"asc": true,
|
||||
"nulls": "last"
|
||||
},
|
||||
{
|
||||
"expression": "user_id",
|
||||
"isExpression": false,
|
||||
"asc": true,
|
||||
"nulls": "last"
|
||||
}
|
||||
],
|
||||
"isUnique": true,
|
||||
"concurrently": false,
|
||||
"method": "btree",
|
||||
"with": {}
|
||||
},
|
||||
"booster_roles_guild_role_idx": {
|
||||
"name": "booster_roles_guild_role_idx",
|
||||
"columns": [
|
||||
{
|
||||
"expression": "guild_id",
|
||||
"isExpression": false,
|
||||
"asc": true,
|
||||
"nulls": "last"
|
||||
},
|
||||
{
|
||||
"expression": "role_id",
|
||||
"isExpression": false,
|
||||
"asc": true,
|
||||
"nulls": "last"
|
||||
}
|
||||
],
|
||||
"isUnique": true,
|
||||
"concurrently": false,
|
||||
"method": "btree",
|
||||
"with": {}
|
||||
}
|
||||
},
|
||||
"foreignKeys": {},
|
||||
"compositePrimaryKeys": {},
|
||||
"uniqueConstraints": {},
|
||||
"policies": {},
|
||||
"checkConstraints": {},
|
||||
"isRLSEnabled": false
|
||||
}
|
||||
},
|
||||
"enums": {},
|
||||
"schemas": {},
|
||||
"sequences": {},
|
||||
"roles": {},
|
||||
"policies": {},
|
||||
"views": {},
|
||||
"_meta": {
|
||||
"columns": {},
|
||||
"schemas": {},
|
||||
"tables": {}
|
||||
}
|
||||
}
|
||||
@@ -8,6 +8,13 @@
|
||||
"when": 1779350490110,
|
||||
"tag": "0000_simple_titania",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 1,
|
||||
"version": "7",
|
||||
"when": 1779352840357,
|
||||
"tag": "0001_misty_wraith",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
+3
-3
@@ -1,4 +1,4 @@
|
||||
import { integer, pgTable, text, uniqueIndex } from "drizzle-orm/pg-core";
|
||||
import { bigint, pgTable, text, uniqueIndex } from "drizzle-orm/pg-core";
|
||||
|
||||
export const boosterRoles = pgTable(
|
||||
"booster_roles",
|
||||
@@ -9,8 +9,8 @@ export const boosterRoles = pgTable(
|
||||
name: text("name").notNull(),
|
||||
color: text("color"),
|
||||
icon: text("icon"),
|
||||
createdAt: integer("created_at").notNull(),
|
||||
updatedAt: integer("updated_at").notNull()
|
||||
createdAt: bigint("created_at", { mode: "number" }).notNull(),
|
||||
updatedAt: bigint("updated_at", { mode: "number" }).notNull()
|
||||
},
|
||||
(table) => ({
|
||||
userIdx: uniqueIndex("booster_roles_guild_user_idx").on(table.guildId, table.userId),
|
||||
|
||||
@@ -120,4 +120,16 @@ describe("handleInteraction", () => {
|
||||
|
||||
expect(interaction.replies[0]).toEqual({ content: "Role name is already used", flags: MessageFlags.Ephemeral });
|
||||
});
|
||||
|
||||
test("hides failed query details from user replies", async () => {
|
||||
const interaction = new FakeInteraction("claim", { name: "VIP" });
|
||||
const service = new FakeService();
|
||||
service.claimRole = async () => {
|
||||
throw new Error("Failed query: insert into booster_roles params: secret");
|
||||
};
|
||||
|
||||
await handleInteraction(interaction, service, { isBoosting: async () => true });
|
||||
|
||||
expect(interaction.replies[0]).toEqual({ content: "Failed to save booster role. Any created role was cleaned up. Try again.", flags: MessageFlags.Ephemeral });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -95,10 +95,24 @@ export async function handleInteraction(
|
||||
throw new Error("Unknown booster-role subcommand");
|
||||
} catch (error) {
|
||||
logger.warn("Booster-role command failed", { error });
|
||||
await interaction.reply({ content: error instanceof Error ? error.message : "Command failed", flags: MessageFlags.Ephemeral });
|
||||
await interaction.reply({ content: toUserErrorMessage(error), flags: MessageFlags.Ephemeral });
|
||||
}
|
||||
}
|
||||
|
||||
function toUserErrorMessage(error: unknown): string {
|
||||
if (!(error instanceof Error)) return "Command failed";
|
||||
|
||||
if (error.message.includes("Failed query")) {
|
||||
return "Failed to save booster role. Any created role was cleaned up. Try again.";
|
||||
}
|
||||
|
||||
if (error.message.includes("Missing Permissions")) {
|
||||
return "Bot is missing permissions or role position to manage this role.";
|
||||
}
|
||||
|
||||
return error.message;
|
||||
}
|
||||
|
||||
function requireGuildId(guildId: string | null): string {
|
||||
if (!guildId) throw new Error("This command can only be used in a server");
|
||||
return guildId;
|
||||
|
||||
@@ -17,10 +17,17 @@ class MemoryRoleStore {
|
||||
}
|
||||
}
|
||||
|
||||
class FailingCreateRoleStore extends MemoryRoleStore {
|
||||
async create(): Promise<void> {
|
||||
throw new Error("Database insert failed");
|
||||
}
|
||||
}
|
||||
|
||||
class FakeRoleRepository implements RoleRepository {
|
||||
roles = new Map<string, { id: string; name: string; permissions: string[]; position: number; color: string | null; icon?: string | null }>();
|
||||
deletedRoleIds: string[] = [];
|
||||
assignedRoles: Array<{ userId: string; roleId: string }> = [];
|
||||
removedRoles: Array<{ userId: string; roleId: string }> = [];
|
||||
|
||||
constructor(initialRoles = [{ id: "existing-vip", name: "VIP", permissions: [], position: 1, color: null }]) {
|
||||
for (const role of initialRoles) {
|
||||
@@ -48,6 +55,10 @@ class FakeRoleRepository implements RoleRepository {
|
||||
this.assignedRoles.push({ userId, roleId });
|
||||
}
|
||||
|
||||
async removeRole(userId: string, roleId: string) {
|
||||
this.removedRoles.push({ userId, roleId });
|
||||
}
|
||||
|
||||
async deleteRole(roleId: string) {
|
||||
this.deletedRoleIds.push(roleId);
|
||||
this.roles.delete(roleId);
|
||||
@@ -130,4 +141,16 @@ describe("BoosterRoleService", () => {
|
||||
expect(roles.deletedRoleIds).toEqual([claimed.roleId]);
|
||||
expect(await store.findByUser("guild", "user")).toBeNull();
|
||||
});
|
||||
|
||||
test("rolls back created and assigned role when storing claim fails", async () => {
|
||||
const roles = new FakeRoleRepository([]);
|
||||
const service = new BoosterRoleService(new FailingCreateRoleStore(), roles, { anchorPosition: 10 });
|
||||
|
||||
await expect(service.claimRole({ guildId: "guild", userId: "user", name: "First Role", color: null, isBoosting: true })).rejects.toThrow("Database insert failed");
|
||||
|
||||
expect(roles.assignedRoles).toEqual([{ userId: "user", roleId: "created-1" }]);
|
||||
expect(roles.removedRoles).toEqual([{ userId: "user", roleId: "created-1" }]);
|
||||
expect(roles.deletedRoleIds).toEqual(["created-1"]);
|
||||
expect(roles.roles.has("created-1")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -29,6 +29,7 @@ export type RoleRepository = {
|
||||
createRole(input: { name: string; color: string | null; permissions: string[]; position: number }): Promise<{ id: string }>;
|
||||
updateRole(roleId: string, input: { name?: string; color?: string | null; icon?: string | null }): Promise<void>;
|
||||
assignRole(userId: string, roleId: string): Promise<void>;
|
||||
removeRole(userId: string, roleId: string): Promise<void>;
|
||||
deleteRole(roleId: string): Promise<void>;
|
||||
};
|
||||
|
||||
@@ -79,27 +80,35 @@ export class BoosterRoleService {
|
||||
assertRolePositionIsSafe(position, this.options.anchorPosition);
|
||||
|
||||
const role = await this.roles.createRole({ name, color, permissions: [], position });
|
||||
if (input.icon) {
|
||||
this.validateRoleIcon(input.icon);
|
||||
await this.roles.updateRole(role.id, { icon: input.icon.dataUri });
|
||||
let assigned = false;
|
||||
|
||||
try {
|
||||
if (input.icon) {
|
||||
this.validateRoleIcon(input.icon);
|
||||
await this.roles.updateRole(role.id, { icon: input.icon.dataUri });
|
||||
}
|
||||
|
||||
await this.roles.assignRole(userId, role.id);
|
||||
assigned = true;
|
||||
|
||||
const timestamp = this.now();
|
||||
const record = {
|
||||
guildId,
|
||||
userId,
|
||||
roleId: role.id,
|
||||
name,
|
||||
color,
|
||||
icon: input.icon?.dataUri ?? null,
|
||||
createdAt: timestamp,
|
||||
updatedAt: timestamp
|
||||
};
|
||||
|
||||
await this.store.create(record);
|
||||
return record;
|
||||
} catch (error) {
|
||||
await this.rollbackClaim({ guildId, userId, roleId: role.id, assigned });
|
||||
throw error;
|
||||
}
|
||||
|
||||
await this.roles.assignRole(userId, role.id);
|
||||
|
||||
const timestamp = this.now();
|
||||
const record = {
|
||||
guildId,
|
||||
userId,
|
||||
roleId: role.id,
|
||||
name,
|
||||
color,
|
||||
icon: input.icon?.dataUri ?? null,
|
||||
createdAt: timestamp,
|
||||
updatedAt: timestamp
|
||||
};
|
||||
|
||||
await this.store.create(record);
|
||||
return record;
|
||||
}
|
||||
|
||||
async renameRole(input: { guildId: string; userId: string; name: string }): Promise<void> {
|
||||
@@ -139,6 +148,15 @@ export class BoosterRoleService {
|
||||
await this.store.delete(guildId, userId);
|
||||
}
|
||||
|
||||
private async rollbackClaim(input: { guildId: string; userId: string; roleId: string; assigned: boolean }): Promise<void> {
|
||||
if (input.assigned) {
|
||||
await ignoreRollbackError(() => this.roles.removeRole(input.userId, input.roleId));
|
||||
}
|
||||
|
||||
await ignoreRollbackError(() => this.roles.deleteRole(input.roleId));
|
||||
await ignoreRollbackError(() => this.store.delete(input.guildId, input.userId));
|
||||
}
|
||||
|
||||
private validateRoleIcon(icon: RoleIcon): void {
|
||||
if (!icon.contentType.startsWith("image/")) {
|
||||
throw new Error("Role icon must be an image");
|
||||
@@ -157,3 +175,10 @@ export class BoosterRoleService {
|
||||
return record;
|
||||
}
|
||||
}
|
||||
|
||||
async function ignoreRollbackError(action: () => Promise<void>): Promise<void> {
|
||||
try {
|
||||
await action();
|
||||
} catch {
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,9 @@ import type { ColorResolvable, Guild, Role } from "discord.js";
|
||||
import type { RoleRepository } from "./boosterRoleService";
|
||||
|
||||
export class DiscordRoleRepository implements RoleRepository {
|
||||
constructor(private readonly guild: Guild, private readonly anchorRoleId: string | null) {}
|
||||
constructor(private readonly guild: Guild, anchorRoleId: string | null) {
|
||||
void anchorRoleId;
|
||||
}
|
||||
|
||||
async listRoles() {
|
||||
await this.guild.roles.fetch();
|
||||
@@ -34,6 +36,11 @@ export class DiscordRoleRepository implements RoleRepository {
|
||||
await member.roles.add(roleId);
|
||||
}
|
||||
|
||||
async removeRole(userId: string, roleId: string): Promise<void> {
|
||||
const member = await this.guild.members.fetch(userId);
|
||||
await member.roles.remove(roleId);
|
||||
}
|
||||
|
||||
async deleteRole(roleId: string): Promise<void> {
|
||||
const role = await this.fetchRole(roleId);
|
||||
await role.delete();
|
||||
|
||||
Reference in New Issue
Block a user