Staff Management System V1.1

[Kevinking500]

This updates the SMG module according to user feedbacks (latest 24 - 48hrs) and other bug reports

Changes:

• [Bug fix] Fixed default parameters not being updated to the new ones
• [Bug fix] Fixed the activity check message being empty if it was amde with Components V2
• [QoL] Completely rewrote the activity check embed and added new parameters
• [QoL] Minor locales improvements
• [QoL] Made the logic previously yapped by Claude (and during more usage being found annoying) that the duty admin command now refreshes the reply instead of replying again ephemerally
• [QoL] Made the add role on promotion default to false since it's recommended to be off and I forgot to change it before
• [QoL] Updated some config descriptions and moved a config up where it makes more sense
• [QoL] Added a staff permission check to issueInfraction and issueSuspension

No AI usage in this PR

[SCDerox]

Thanks for the PR! One thing on the Components V2 activity-check fix — endActivityCheckProcess in staff-management.js (around the if (msg && msg.embeds.length > 0) guard) still skips the update path for V2 messages.

When the activity-check message was sent via embedTypeV2 with _schema: "v4", the IsComponentsV2 flag means there's no top-level embed in msg.embeds, so the guard is false and the message isn't edited at end-of-check. The default config ships _schema: "v3" so most servers won't notice, but anyone who switched their template to V2 still hits the original empty-message bug this PR is meant to fix.

A couple of fix options: clear the components unconditionally regardless of whether embeds are present, or branch on the V2 flag and rebuild the V2 component tree to mark the check as ended.

[Kevinking500]

Thanks for the reply! This should be the fix.

[Kevinking500]

Thanks for the feedback, just fixed that one specified one. I will also actually apply it to some other places if you don't mind.

[SCDerox]

Thanks for the quick turnaround! One more thing on this path — for V2 templates the new code does run, but the resulting msg.edit({ components: [] }) clears the container components too, so the message ends up visually empty (since V2 messages have no top-level embed to fall back on, all the title/description content lives inside the containers).

Two ways to keep the body visible:

1. Filter the response button by its customId instead of wiping all components — the button is added as a top-level row with a stable id, so something like:

   const kept = msg.components
       .map(row => row.toJSON ? row.toJSON() : row)
       .filter(row => !(row.components || []).some(c => c.custom_id === 'staff-mgmt_ac-respond'));
   await msg.edit({ components: kept });

2. Re-render through embedTypeV2 with the same template (or a "check ended" variant) and pass an empty components: [] as the third argument so no button is appended:

   const ended = await embedTypeV2(endedTemplate, placeholders, { components: [] });
   await msg.edit(ended);

Both keep the original check context visible to users while removing the live button. The first is the smaller change; the second lets you also tweak the styling/title to indicate the check has ended.

[Kevinking500]

Actually just came to this solution, I think it would be great if the user can configure the end message to their liking to like show other text and maybe other custom buttons etc. I will implement that logic which would probably also simplify this a bit

[Kevinking500]

I changed logic for this, though to make this reliable I had to add 2 new columns to a DB model, though I am interested to know if the migration process with the SCNX hosted bots will work to update it properly.
Otherwise, I think this should now work ^^

[SCDerox]

Heads-up while integrating this into our closed-source build for SCNX — small edge case in the new endActivityCheckProcess:

const initiator = activeCheck.isAutomated
    ? localize('staff-management-system', 'label-system')
    : `<@${activeCheck.initiatorId}>`;

For activity checks that were already active at the time a server upgrades to this version, the migration backfills initiatorId: null and isAutomated: false. When one of those checks later expires, %initiator% resolves to the literal string <@null> (Discord won't render that as a mention).

One-line fix: treat a missing initiatorId the same way as automated checks.

const initiator = (activeCheck.isAutomated || !activeCheck.initiatorId)
    ? localize('staff-management-system', 'label-system')
    : `<@${activeCheck.initiatorId}>`;

We're applying this on our side already so SCNX users won't be affected — flagging it here in case you'd like to include it in this PR or address it separately.

[SCDerox]

Side note unrelated to this PR — the broader SMG module has a lot of repeated JSON.parse / JSON.stringify patterns around config templates (counted ~14 instances). Two specifically:

// 1. Parse-if-string defensive guard
if (typeof template === 'string') {
    try { template = JSON.parse(template); } catch (e) {}
}

// 2. Deep-clone via stringify+parse, then patch _schema
template = JSON.parse(JSON.stringify(template));
if (template?.embeds && !template._schema) template._schema = 'v3';

Both end up being no-ops in practice:

• client.configurations[...] returns parsed JSON objects, never strings. The string branch never fires. Every other module in the codebase just passes config.fieldName directly to embedTypeV2.
• The _schema = 'v3' patch is a backwards-compat shim for configs saved before _schema existed in the field default. The current defaults already include "_schema": "v3", so the patch is a no-op for any recently-saved config. The deep-clone (stringify+parse) only exists to avoid the patch mutating the live config object.

The cleaner shape that matches the rest of the codebase is just:

let msgOpts = await embedTypeV2(config.fieldName, placeholders);

For the legacy-config edge case (configs persisted before _schema was a thing), a one-shot startup migration could patch _schema on existing rows once, instead of at every send.

Not a blocker for this PR — just flagging since you're already iterating on the module. Happy to open a separate cleanup PR if useful.

[Kevinking500]

Thanks for the comments - i've addressed the first one locally as well. Since your second comment was a bit vague (are these Claude comments?), should I address the JSON.parse/stringify changes inside this PR or for a different next PR for SMG?

[SCDerox]

Re: cleanup — separate PR makes sense, would muddy this diff. Happy to open one for SMG once this lands.

On the workflow: yeah, fair to ask. I do a manual review locally and test the changes myself, then run Claude alongside it to quickly catch major issues (like the <@null> edge case in endActivityCheckProcess) and filter out the ones that aren't actually issues (of which there are many — false positives are real). Once I've verified the findings, I let it post directly since it explains things way better than I do lol. Lets me get through PRs faster and spend more time on building new stuff — if it feels uncomfortable, lmk and I'll go back to manual writeups.

While we're here, one more thing that came up during the merge that I held off on flagging — and the fix isn't in the current PR head, so it'd need a follow-up commit. startActivityCheck is currently broken.

The previous version had:

let embedTemplate = typeof config.checkMessage === 'string'
    ? JSON.parse(config.checkMessage)
    : config.checkMessage;
let msgOpts = await embedTypeV2(embedTemplate, { ... });

The refactor (around line 1417) removed the let embedTemplate = ... assignment — probably intentional cleanup, since the JSON.parse-if-string branch never actually fires (per the broader observation in my earlier comment). But the embedTypeV2(embedTemplate, ...) call further down still references embedTemplate, so at runtime the function throws ReferenceError: embedTemplate is not defined on the first message build.

The error path matters here: the try/catch block in this function only wraps the targetChannel.send(msgOpts) and ActivityCheck.create(...) calls. The embedTypeV2 call is outside that wrap, so the ReferenceError propagates up to the caller. End result: no DB row is created, no scheduler job is registered, and the user sees no feedback either. Activity checks silently fail to start.

Cleanest fix is to pass config.checkMessage directly — matches how every other module in the codebase reads allowEmbed: true config fields, and aligns with the JSON-parse cleanup from the side note:

let msgOpts = await embedTypeV2(config.checkMessage, {
    '%end-time%': dateToDiscordTimestamp(endTime, 'F'),
    // ...rest of placeholders...
});

We're already running this patch in our build, so SCNX users won't see the regression.

[Kevinking500]

Alright I have pushed the changes according to the feedback ^^

[SCDerox]

Looking great!