Ticket system upgrade

[Kingbaby102155]

Upgraded ticket system

[CLAassistant]

All committers have signed the CLA.

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	discord-html-transcripts@​3.2.0

View full report

[SCDerox]

I'm closing this PR. It can't be merged and it can't be salvaged, and I've written up why in full, because I don't want you leaving with the impression this came down to house style or a picky reviewer.

This code has never been run. The tickets module doesn't load. Nothing in it registers, and the bot exits before it reaches ready. You'd have seen that the first time you started it.

Before the technical part, disclosure. Mine first: I used an AI assistant on this, both to check the loader contracts against your diff and to draft this reply. I'd normally write a review like this myself. After the day this PR cost me I wasn't willing to spend an evening writing it up as well. Every claim below I've verified myself and I stand behind all of it, but it's only fair to say so given what I'm about to ask you.

Which is: please state in the PR description whether this was written with an AI assistant, and which one. We don't ban that. We expect it declared, up front, without being asked. The evidence here is hard to miss. There are 22 commits and the arc goes from "make the tests pass" to "delete the tests", with messages like "instantiate dynamic array structure fallbacks for automated testing engines." And there are comments in shipped module code that talk to the test runner:

// Fetch the mock configuration block or fall back to an empty template structure
// This prevents undefined reference errors when interacting with original test objects
const moduleConfig = client.configurations?.tickets?.config?.[0] || { categories: [] };

// This stops the module from interfering with global test scripts (like 'Error: kaboom')
if (!isMenu && !isButton && !isModal) return;

Module code should never know that tests exist. Every ?. and fallback in this diff that's only there to survive a mock is a spot where the code got fitted to test output instead of to the framework, and at every one of those spots the real contract turned out to be something else.

Use whatever tools you want. You still own every line you submit. That means you understand it and you've run it. Editing the tests until they stop complaining is neither.

The bar

Run the bot with your change. Start it, load the module, click the buttons in a real server. If you touched a code path, exercise it. A PR that has never been run isn't ready to open. Working out what an unrun change does takes us longer than writing it took you, and that time comes out of maintenance the rest of the userbase is waiting on.

npm test and npm run test:unit pass, unmodified. Several of our tests encode bugs we've already shipped and fixed and don't want back. If one fails, the test is doing its job. The change moves.

No deleting, stubbing or skipping tests to get green. If you're convinced a test is wrong, leave it failing and say so, and we'll look. Quietly neutralising one gets a PR closed, and that's most of why this one is closed.

Read the contracts before writing against them. They're in developer-docs/. Nearly every problem below maps to one document.

Scope stays tight. Build scripts, dependency pins, modules you aren't working on: out of the diff.

---

The technical part

The module doesn't load

module.json was rewritten and everything the loader actually reads got dropped.

main.js:1069-1071 reads models-dir, commands-dir and events-dir. All three gone, so nothing in commands/, events/ or models/ ever gets required. src/functions/configuration.js:370-372 reads config-example-files, also gone, so client.configurations['tickets'] stays empty and no config file is generated or validated. intents: ["GuildMessages"] gone too, which is why CI is red on all five Node versions (tests/intents/moduleDeclarations.test.js and eventIntentCrossCheck.test.js). And humanReadableName, description, fa-icon, author.scnxOrgID, openSourceURL and tags are what the dashboard builds the module card from.

The keys that replaced them (id, version, dependencies) aren't read by anything in this repo. There's no per-module dependency installer, so dependencies does nothing at all.

Put the original module.json back and add to it. developer-docs/writing-a-module.md.

Commands, events and models all use the wrong shape

Commands: loadCommandsInDir (main.js:1017-1044) reads props.config.name, .type, .description, .options, .defaultMemberPermissions. All six new and rewritten command files export a flat {name, description, run} with no config object, so the first one loaded throws Cannot read properties of undefined (reading 'name'). That's the boot crash.

ticketsetup.js is worse. It exports execute instead of run, and it's written as a message command (message.member, message.reply, message.delete, args). We don't have message commands. That file can never run under any circumstances.

Nothing declares an options array either, so interaction.options.getUser('user') in ticketadd.js can never return anything, because the option was never registered with Discord. developer-docs/commands.md.

Events: main.js:958 calls handlers as eventFunction.run(client, ...cArgs). The new interactionCreate.js is async run(interaction), so that first parameter is the client. Not the interaction. interaction.client is undefined, interaction.isButton isn't a function, and every guard in the handler is inspecting the wrong object. developer-docs/events.md.

Models: loadModelsInDir (main.js:886-921) calls model.init(db) and reads model.config.name. Ticket.js is now a (sequelize, DataTypes) => sequelize.define(...) factory, so model.init is not a function and the process exits 78. developer-docs/database-models.md.

Also, module models are namespaced. client.models['tickets']['Ticket'] (main.js:913), not client.models.Ticket.

config.json is a schema, not a settings file

This causes most of the rest. Not obvious if you haven't read the docs, but it is documented.

modules/tickets/config.json describes fields. The content array is a list of setting definitions (name, humanName, description, type, default, dependsOn, params), and src/functions/configuration.js uses it to generate and type-check the actual config under client.configDir. The SCNX dashboard reads the same file to render the settings UI. configElements: true is the flag that gives the module multiple ticket categories in the first place.

The PR swapped that for a values file with placeholder snowflakes baked in:

"ticket_category_id": "123456789012345678",
"staff_role_id": "112233445566778899",

So the dashboard has nothing to render and nobody can configure the module through the UI at all. The multi-category model is gone. And every install ships pointed at channels and roles that don't exist.

Downstream of that: ticketpanel.js and InactivityChecker.js call fs.writeFileSync on modules/tickets/config.json at runtime. Module directories aren't writable on hosted installs and get replaced on update, so those writes either fail or vanish. A slash command has no business writing config, that belongs to the config layer and the dashboard. Separately, TicketManager.js, claim.js, ticketadd.js, ticketremove.js, unclaim.js and ticketsetup.js all require('../config.json') at load time, so the cached object and the file on disk drift apart the moment ticketpanel writes, and every reader serves stale values until restart.

All of it should be reading client.configurations['tickets']['config']. developer-docs/configuration.md.

The schema change eats existing data

open to status, userID to userId, channelID to channelId. msgLogURL, msgCount, addedUsers and type dropped entirely. And tableName: 'ticket_Ticketv2' removed, so Sequelize points at a different table name.

Every existing install loses its whole ticket history, quietly, on first boot after update. If you need to change the schema, write a migration. modules/tickets/migrations/tickets_Ticket__V1.js is right there as the pattern, and developer-docs/migration.md explains it.

Things outside the diff that this breaks

None of these show up in the files-changed view. commands/create-ticket-about-message.js imports {createTicket} from events/interactionCreate.js, and the PR deletes that export. events/messageCreate.js queries channelID and open and increments msgCount, all three of which no longer exist. And src/core/analytics/moduleQueries.js:718-728 reads models.tickets.Ticket for ticket-volume analytics and depends on open, so that breaks silently with no error anywhere.

The tests you deleted were doing real work

tests/tickets/interactionCreate.test.js existed because of a bug we actually shipped once. Ticket creation used to do channel-create, message-send and pin before acknowledging the interaction, which blew past Discord's 3 second window and threw Unknown interaction (10062). The test asserted deferReply() comes before any slow Discord call.

You replaced it with expect(true).toBe(true). And this PR brings the bug back. The no-questions branch in interactionCreate.js:

if (!categoryData.questions || categoryData.questions.length === 0) {
    return await openTicketChannel(interaction, categoryData, activeStaffRole, TicketModel, config, [], client);
}

Straight to openTicketChannel with no deferReply, and openTicketChannel finishes with interaction.editReply(...), which throws "Interaction has not been replied or deferred." Any category configured without a form is broken.

That test was telling you your new code had a bug, and it got removed instead of read. The other two files covered the context-menu commands this PR breaks anyway.

package.json

None of this has anything to do with tickets:

• "start": "node main.js" deleted, so the documented way to run the bot is gone
• "test": "npx eslint ./" deleted, which is the command CONTRIBUTING.md tells contributors to run
• generate-config and generate-template deleted
• verify-configs repointed at src/functions/verifyModuleConfigs.js, which doesn't exist (it's scripts/verify-config-defaults.js)
• discord.js unpinned from 14.26.4 to ^14.26.4, and that pin is there on purpose
• an allowScripts block added, which is a @lavamoat/allow-scripts thing we don't use, listing unrs-resolver@1.1.2.2, which isn't valid semver
• indentation broken on both "scripts" and "allowScripts"

Revert the file and reapply only what the feature actually needs.

On the new dependency: discord-html-transcripts drags in @derockdev/discord-components-react, @stencil/core, highlight.js and react/react-dom peers. That's a React rendering stack and 261 lockfile lines going into a Discord bot. We already have messageLogToStringToPaste() in src/functions/helpers.js, which produces the transcript links with 1 year retention that the dashboard links to. If you want HTML transcripts, open an issue for it separately instead of smuggling it into a module rewrite.

Also there are three .DS_Store files committed. Add that to your global gitignore.

Localisation

Every localize() call in the module is gone, replaced with about 40 hardcoded English strings. We ship 27 locales and localisation is required for anything a user sees. developer-docs/localization.md. Config field labels and descriptions also need entries under config-localizations/, see config-localization.md.

Bugs in the new code itself

If the module did load, this is what you'd hit. Most of it within minutes of using it.

claim.js and unclaim.js use permissionOverwrites.set(), which replaces the entire overwrite list rather than adding to it. So claiming a ticket silently kicks out every other staff member plus anyone added with /ticketadd, and unclaiming kicks them out for good. You want .edit().

ticketremove.js reads interaction.mentions.users.first(). Interactions don't have .mentions, that's on Message. Guaranteed TypeError. The fallback path reads args, which never gets passed to a command's run.

The modal builder does questions.slice(0, 5) for Discord's five-row cap, but the submit handler loops over all of categoryData.questions calling getTextInputValue(q.id). Configure six questions and every submission throws on the sixth.

ticketsetup.js puts one button per category into a single ActionRow. Discord allows five components per row, so six categories is an API error.

InactivityChecker.start() is never called from anywhere. The entire file is dead code. If it were wired up: it hardcodes a 5 minute interval while ignoring the check_interval_minutes it just read from config, does a synchronous readFileSync and JSON.parse inside that interval, and fetches messages once per open ticket per tick, which will rate-limit you on any busy server. The anti-spam guard checks lastMessage.content.includes('⚠️') against a warn message the admin can edit, so anyone who removes that emoji gets warned every 5 minutes forever.

TicketManager.closeTicket() calls channel.delete() immediately. No grace period, no confirmation. The current flow locks the channel, posts a notice and deletes after 20 seconds so people can read what happened. Worse, it does client.channels.fetch(config.log_channel_id) first, on the placeholder ID, which rejects and aborts the function before dbInstance.update({status: 'CLOSED'}) runs. The ticket ends up permanently OPEN in the database attached to a channel that can't be closed.

close-ticket.js has an unreachable error path: if (interaction.replied === false) in the catch, a few lines after interaction.reply(...) set it to true. Users never hear about failures. It also replies before doing the work instead of deferring.

The guild and readiness guards are gone. The old handler started with if (!client.botReadyAt) return; and if (interaction.guild.id !== client.config.guildID) return;.

Permissions got less capable. ticketRoles was an array so a server could give several teams access to one ticket type. Now it's a single global staff_role_id plus one optional custom_staff_role per category.

console.error everywhere instead of client.logger.error, which loses Sentry context and the path sanitisation in main.js:963.

Smaller stuff: ephemeral: true is deprecated in discord.js 14.26 (InteractionResponses.js:98), use flags: MessageFlags.Ephemeral. type: 0 should be ChannelType.GuildText. interaction.options._hoistedOptions is private API. {[PermissionFlagsBits.ViewChannel]: true} in ticketadd.js evaluates to {"1024": true} and only works by accident, because BitField.resolve happens to have a numeric-string fallback. Use {ViewChannel: true}.

---

One more thing you should know

All of this already exists, built and supported, in the SCNX Support Bot, included on the Professional and Unlimited plans. Its ticket system covers every feature in this PR and then some:

• Ticket claiming, with a claim button, optional lock-before-claim, hidden unclaimed tickets and automatic channel rename on claim (docs)
• Ticket topics, which is the multi-category model you were rebuilding, each with its own category, team roles, log channel and messages (docs)
• Forms, so per-topic modal questions (docs)
• Auto-close on inactivity with configurable reminders (docs)
• Adding and removing members from a ticket, transcripts, AI ticket summaries, feedback ratings, opening hours, escalation and analytics

Plus Modmail, Forum Support and Voice Support alongside it, all sharing one blocklist and one analytics view.

I mention it because if what you actually wanted was a better ticket system for your own server, you've spent a lot of effort rebuilding, less completely, something you may already have access to. The Custom-Bot ticket module is kept simple on purpose. We don't develop the advanced support features there, and a PR trying to turn it into the Support Bot isn't a direction we'd take even if it were implemented correctly.

---

Closing this.

Why write this much on a closed PR? Because none of it needed a reviewer to find. Starting the bot once catches the boot crash. Opening one ticket catches the interaction timeout, and reading the failing test catches the bug it was pointing at. Instead all of that landed on us, and working through 1729 changed lines to establish that the module never loads cost me most of a day I'd rather have spent elsewhere.

So: don't open another PR in this shape. Generated code that hasn't been run, with the failing tests removed to get a green check, is not something we can take, and I won't review it at this length a second time. If a future PR shows the same pattern I'll close it with a link back here.

If you come back with something you've built and run yourself, scoped small enough to review properly, I'll give it a fair look.