Skip to content
Merged
Show file tree
Hide file tree
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
11 changes: 11 additions & 0 deletions packages/bot/src/handlers/eventHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ const namedSessionListMock = jest.fn()
const cleanupGuildStateMock = jest.fn()
const aiDevToolkitStartMock = jest.fn()
const handleReactionRolesMock = jest.fn()
const recordGuildJoinMock = jest.fn(async () => undefined)
const recordGuildLeaveMock = jest.fn(async () => undefined)
const syncGuildsOnReadyMock = jest.fn(async () => undefined)

jest.mock('../utils/general/interactionReply', () => ({
interactionReply: (...args: unknown[]) => interactionReplyMock(...args),
Expand Down Expand Up @@ -91,6 +94,14 @@ jest.mock('../services/AiDevToolkitService', () => ({
},
}))

jest.mock('../services/guildMembershipService', () => ({
recordGuildJoin: async (...args: unknown[]) => recordGuildJoinMock(...args),
recordGuildLeave: async (...args: unknown[]) =>
recordGuildLeaveMock(...args),
syncGuildsOnReady: async (...args: unknown[]) =>
syncGuildsOnReadyMock(...args),
}))
Comment on lines +97 to +103

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add direct assertions for guild membership service integrations.

The new mocks are wired, but there’s no explicit assertion that recordGuildJoin, recordGuildLeave, and syncGuildsOnReady are called on GuildCreate/GuildDelete/clientReady. Please add those checks to lock in the new behavior.

Also applies to: 385-437, 474-503

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/bot/src/handlers/eventHandler.spec.ts` around lines 97 - 103, Add
explicit assertions in the eventHandler.spec tests to verify the mocked guild
membership service functions are invoked when the corresponding events fire:
assert recordGuildJoinMock is called on the GuildCreate test,
recordGuildLeaveMock is called on the GuildDelete test, and
syncGuildsOnReadyMock is called on the clientReady test (also add similar
assertions in the other test blocks around lines 385-437 and 474-503). Locate
the existing jest.mock wiring that defines
recordGuildJoin/recordGuildLeave/syncGuildsOnReady and then add expect(...)
assertions referencing recordGuildJoinMock, recordGuildLeaveMock, and
syncGuildsOnReadyMock to confirm they were called with the expected args after
each simulated event.


function createMockClient() {
const onMock = jest.fn()
const onceMock = jest.fn()
Expand Down
37 changes: 37 additions & 0 deletions packages/bot/src/handlers/eventHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,24 @@ import { reactionRolesService } from '@lucky/shared/services'
import { aiDevToolkitService } from '../services/AiDevToolkitService'
import { namedSessionService } from '../utils/music/namedSessions'
import { cleanupGuildState } from './player/trackNowPlaying'
import {
recordGuildJoin,
recordGuildLeave,
syncGuildsOnReady,
} from '../services/guildMembershipService'

function handleClientReady(client: Client): void {
client.once('clientReady', () => {
infoLog({ message: `Logged in as ${client.user?.tag}!` })
debugLog({
message: `Bot is ready with ${(client as CustomClient).commands.size} commands loaded`,
})
syncGuildsOnReady(client).catch((error) => {
errorLog({
message: 'guildMembershipService: on-ready sync failed',
error,
})
})
Comment on lines +38 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and read the relevant sections
wc -l packages/bot/src/handlers/eventHandler.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 112


🏁 Script executed:

# Read the file with line numbers to see the context around the mentioned lines
head -80 packages/bot/src/handlers/eventHandler.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 3359


🏁 Script executed:

# Also check the specific line ranges mentioned (61-66, 229-234)
sed -n '55,75p' packages/bot/src/handlers/eventHandler.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 812


🏁 Script executed:

# Search for the logAndSwallow and logAndRethrow utilities
fd -t f '*.ts' -x grep -l 'logAndSwallow\|logAndRethrow' {} \;

Repository: LucasSantana-Dev/Lucky

Length of output: 350


🏁 Script executed:

# Look for the errorLog utility import/definition
rg 'errorLog|logAndSwallow|logAndRethrow' packages/bot/src/handlers/eventHandler.ts -B 2 -A 2

Repository: LucasSantana-Dev/Lucky

Length of output: 2224


🏁 Script executed:

# Search for logAndSwallow and logAndRethrow definitions
rg 'export.*logAndSwallow|export.*logAndRethrow|function logAndSwallow|function logAndRethrow' packages/

Repository: LucasSantana-Dev/Lucky

Length of output: 379


🏁 Script executed:

# Check line 229-234
sed -n '225,240p' packages/bot/src/handlers/eventHandler.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 781


🏁 Script executed:

# Search if these utilities are used anywhere in the codebase
rg 'logAndSwallow|logAndRethrow' --type ts packages/

Repository: LucasSantana-Dev/Lucky

Length of output: 4721


🏁 Script executed:

# Read the logAndRethrow and logAndSwallow implementation
cat -n packages/shared/src/utils/error/logAndRethrow.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 1720


Use logAndSwallow() utility in the three error handler catch blocks instead of errorLog() directly.

Lines 38-43, 61-66, and 229-234 use direct errorLog() calls in .catch() blocks. Per coding guidelines, use the logAndSwallow() utility for operations where failure is acceptable and errors are swallowed. These are fire-and-forget operations, so logAndSwallow() is the appropriate choice.

Import from @lucky/shared/utils/error and wrap each catch block accordingly:

.catch((error) => {
    logAndSwallow(error, 'guildMembershipService: on-ready sync failed')
})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/bot/src/handlers/eventHandler.ts` around lines 38 - 43, Replace the
direct errorLog(...) calls in the three fire-and-forget .catch(...) blocks (the
one attached to syncGuildsOnReady(...) and the two other similar catch blocks)
with the logAndSwallow utility: import { logAndSwallow } from
'`@lucky/shared/utils/error`' and in each .catch pass the caught error and a
descriptive string (e.g., 'guildMembershipService: on-ready sync failed') to
logAndSwallow(error, '...') so failures are logged and swallowed per guideline;
update the three catch blocks accordingly and remove the old errorLog usage
there.

if (process.env.AI_DEV_TOOLKIT_BOARD_ENABLED === 'true') {
aiDevToolkitService.start(client).catch((error) => {
errorLog({
Expand All @@ -41,6 +52,21 @@ function handleClientReady(client: Client): void {
})
}

function handleGuildCreate(client: Client): void {
client.on(Events.GuildCreate, (guild) => {
infoLog({
message: 'Joined guild',
data: { guildId: guild.id, name: guild.name },
})
recordGuildJoin(guild).catch((error) => {
errorLog({
message: 'Error recording guild join',
error,
})
})
})
}

async function handleCommandNotFound(
interaction: ChatInputCommandInteraction,
): Promise<void> {
Expand Down Expand Up @@ -196,6 +222,16 @@ function handleDebug(client: Client): void {

function handleGuildDelete(client: Client): void {
client.on(Events.GuildDelete, async (guild) => {
infoLog({
message: 'Left guild',
data: { guildId: guild.id, name: guild.name },
})
await recordGuildLeave(guild.id, guild.name).catch((error) => {
errorLog({
message: 'Error recording guild leave',
error,
})
})
try {
const duplicateDetection =
(await import('../utils/music/duplicateDetection/index.js')) as {
Expand Down Expand Up @@ -243,6 +279,7 @@ export default function handleEvents(client: Client) {
handleError(client)
handleWarn(client)
handleDebug(client)
handleGuildCreate(client)
handleGuildDelete(client)
handleChannelDelete(client)
}
169 changes: 169 additions & 0 deletions packages/bot/src/services/guildMembershipService.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import { beforeEach, describe, expect, it, jest } from '@jest/globals'

const upsertMock = jest.fn(async () => ({}))
const updateManyMock = jest.fn(async () => ({ count: 1 }))
const createMock = jest.fn(async () => ({}))
const findUniqueMock = jest.fn<
() => Promise<{ joinedAt: Date | null } | null>
>(async () => null)
const transactionMock = jest.fn(async (ops: unknown) => {
// Prisma's $transaction([...]) accepts an array of pending queries;
// resolve them so call assertions on the inner mocks run.
if (Array.isArray(ops)) {
return Promise.all(ops)
}
return ops
})
const errorLogMock = jest.fn()
const infoLogMock = jest.fn()

jest.mock('@lucky/shared/utils', () => ({
getPrismaClient: () => ({
guild: {
upsert: (...args: unknown[]) => upsertMock(...args),
updateMany: (...args: unknown[]) => updateManyMock(...args),
findUnique: (...args: unknown[]) => findUniqueMock(...args),
},
guildMembershipEvent: {
create: (...args: unknown[]) => createMock(...args),
},
$transaction: (...args: unknown[]) => transactionMock(...args),
}),
errorLog: (...args: unknown[]) => errorLogMock(...args),
infoLog: (...args: unknown[]) => infoLogMock(...args),
}))

import {
recordGuildJoin,
recordGuildLeave,
syncGuildsOnReady,
} from './guildMembershipService'

type FakeGuild = {
id: string
name: string
icon: string | null
ownerId: string
joinedTimestamp: number | null
}

function fakeGuild(overrides: Partial<FakeGuild> = {}): FakeGuild {
return {
id: '111',
name: 'Test Guild',
icon: null,
ownerId: 'owner-1',
joinedTimestamp: 1747200000000,
...overrides,
}
}

describe('guildMembershipService', () => {
beforeEach(() => {
jest.clearAllMocks()
})

describe('recordGuildJoin', () => {
it('upserts guild and writes JOIN event in a transaction', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await recordGuildJoin(fakeGuild() as any)

expect(transactionMock).toHaveBeenCalledTimes(1)
expect(upsertMock).toHaveBeenCalledTimes(1)
const call = upsertMock.mock.calls[0]?.[0] as Record<
string,
unknown
>
expect(call.where).toEqual({ discordId: '111' })
const create = call.create as Record<string, unknown>
expect(create.discordId).toBe('111')
expect(create.joinedAt).toBeInstanceOf(Date)
expect(create.leftAt).toBeNull()

expect(createMock).toHaveBeenCalledTimes(1)
const eventArgs = createMock.mock.calls[0]?.[0] as {
data: Record<string, unknown>
}
expect(eventArgs.data.kind).toBe('JOIN')
expect(eventArgs.data.guildDiscordId).toBe('111')
expect(eventArgs.data.guildName).toBe('Test Guild')
})

it('falls back to now() when guild.joinedTimestamp is null', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await recordGuildJoin(fakeGuild({ joinedTimestamp: null }) as any)

const call = upsertMock.mock.calls[0]?.[0] as Record<
string,
unknown
>
const update = call.update as Record<string, unknown>
expect(update.joinedAt).toBeInstanceOf(Date)
})

it('logs an error and does not throw when the transaction fails', async () => {
transactionMock.mockRejectedValueOnce(new Error('db down'))
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await expect(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
recordGuildJoin(fakeGuild() as any),
).resolves.toBeUndefined()
expect(errorLogMock).toHaveBeenCalledTimes(1)
})
})

describe('recordGuildLeave', () => {
it('stamps leftAt on Guild and writes LEAVE event', async () => {
await recordGuildLeave('222', 'Departing Guild')

expect(transactionMock).toHaveBeenCalledTimes(1)
expect(updateManyMock).toHaveBeenCalledTimes(1)
const updateCall = updateManyMock.mock.calls[0]?.[0] as Record<
string,
unknown
>
expect(updateCall.where).toEqual({ discordId: '222' })
const data = updateCall.data as Record<string, unknown>
expect(data.leftAt).toBeInstanceOf(Date)

const eventArgs = createMock.mock.calls[0]?.[0] as {
data: Record<string, unknown>
}
expect(eventArgs.data.kind).toBe('LEAVE')
expect(eventArgs.data.guildDiscordId).toBe('222')
})
})

describe('syncGuildsOnReady', () => {
it('skips guilds that already have joinedAt and upserts the rest', async () => {
findUniqueMock.mockResolvedValueOnce({
joinedAt: new Date('2026-01-01'),
})
findUniqueMock.mockResolvedValueOnce(null)

const guildA = fakeGuild({ id: 'a' })
const guildB = fakeGuild({ id: 'b', joinedTimestamp: null })
const client = {
guilds: {
cache: new Map([
['a', guildA],
['b', guildB],
]),
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any

await syncGuildsOnReady(client)

expect(findUniqueMock).toHaveBeenCalledTimes(2)
// Only guildB should be upserted (guildA already has joinedAt).
expect(upsertMock).toHaveBeenCalledTimes(1)
const call = upsertMock.mock.calls[0]?.[0] as Record<
string,
unknown
>
expect(call.where).toEqual({ discordId: 'b' })
expect(infoLogMock).toHaveBeenCalledTimes(1)
})
})
})
Loading
Loading