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
9 changes: 7 additions & 2 deletions packages/backend/src/routes/management.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,17 @@ export function setupManagementRoutes(app: Express): void {
type as LogType,
limit,
)
res.json({ logs })
const total = await serverLogService.countLogsByType(
guildId,
type as LogType,
)
res.json({ logs, total })
return
}

const logs = await serverLogService.getRecentLogs(guildId, limit)
res.json({ logs })
const total = await serverLogService.countRecentLogs(guildId)
res.json({ logs, total })
}),
)

Expand Down
8 changes: 8 additions & 0 deletions packages/backend/src/routes/twitch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ async function lookupTwitchUser(
}

export function setupTwitchRoutes(app: Express): void {
app.get(
'/api/twitch/status',
asyncHandler(async (_req: Request, res: Response) => {
const configured = !!process.env.TWITCH_CLIENT_ID
res.json({ configured })
Comment on lines +113 to +114

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

/api/twitch/status can report a false positive configured state

Line 113 only checks TWITCH_CLIENT_ID, but lookup logic in this same file also needs either TWITCH_ACCESS_TOKEN or TWITCH_CLIENT_SECRET. This can re-enable the broken UX you’re fixing (button enabled, then lookup fails).

Suggested fix
-            const configured = !!process.env.TWITCH_CLIENT_ID
+            const clientId = process.env.TWITCH_CLIENT_ID
+            const hasAccessToken = !!process.env.TWITCH_ACCESS_TOKEN
+            const hasClientSecret = !!process.env.TWITCH_CLIENT_SECRET
+            const configured = Boolean(clientId && (hasAccessToken || hasClientSecret))
             res.json({ configured })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/routes/twitch.ts` around lines 113 - 114, The
/api/twitch/status endpoint currently sets configured =
!!process.env.TWITCH_CLIENT_ID which can be a false positive; update the check
so configured is true only if TWITCH_CLIENT_ID is present AND at least one of
TWITCH_ACCESS_TOKEN or TWITCH_CLIENT_SECRET is present (i.e. configured =
!!TWITCH_CLIENT_ID && (!!TWITCH_ACCESS_TOKEN || !!TWITCH_CLIENT_SECRET)), then
return that value in res.json({ configured }); locate this in
packages/backend/src/routes/twitch.ts where configured is defined and used.

}),
)

app.get(
'/api/twitch/users',
requireAuth,
Expand Down
12 changes: 9 additions & 3 deletions packages/backend/tests/integration/routes/management.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ jest.mock('@lucky/shared/services', () => ({
searchLogs: jest.fn(),
getUserLogs: jest.fn(),
getStats: jest.fn(),
countRecentLogs: jest.fn(),
countLogsByType: jest.fn(),
logAutoModSettingsChange: jest.fn(),
logCustomCommandChange: jest.fn(),
},
Expand Down Expand Up @@ -337,7 +339,9 @@ describe('Management Routes Integration', () => {
)

const response = await request(app)
.post('/api/guilds/111111111111111111/automod/templates/unknown/apply')
.post(
'/api/guilds/111111111111111111/automod/templates/unknown/apply',
)
.set('Cookie', ['sessionId=valid_session_id'])
.expect(404)

Expand Down Expand Up @@ -718,13 +722,14 @@ describe('Management Routes Integration', () => {
typeof serverLogService
>
mockServerLogService.getRecentLogs.mockResolvedValue(mockLogs)
mockServerLogService.countRecentLogs.mockResolvedValue(10)

const response = await request(app)
.get('/api/guilds/111111111111111111/logs')
.set('Cookie', ['sessionId=valid_session_id'])
.expect(200)

expect(response.body).toEqual({ logs: mockLogs })
expect(response.body).toEqual({ logs: mockLogs, total: 10 })
expect(mockServerLogService.getRecentLogs).toHaveBeenCalledWith(
'111111111111111111',
50,
Expand All @@ -743,13 +748,14 @@ describe('Management Routes Integration', () => {
typeof serverLogService
>
mockServerLogService.getLogsByType.mockResolvedValue(mockLogs)
mockServerLogService.countLogsByType.mockResolvedValue(5)

const response = await request(app)
.get('/api/guilds/111111111111111111/logs?type=error')
.set('Cookie', ['sessionId=valid_session_id'])
.expect(200)

expect(response.body).toEqual({ logs: mockLogs })
expect(response.body).toEqual({ logs: mockLogs, total: 5 })
expect(mockServerLogService.getLogsByType).toHaveBeenCalledWith(
'111111111111111111',
'error',
Expand Down
26 changes: 26 additions & 0 deletions packages/backend/tests/integration/routes/twitch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,32 @@ jest.mock('@lucky/shared/services', () => ({
},
}))

describe('GET /api/twitch/status', () => {
let app: express.Express

beforeEach(() => {
app = express()
app.use(express.json())
setupTwitchRoutes(app)
app.use(errorHandler)
})

test('returns configured=true when TWITCH_CLIENT_ID is set', async () => {
process.env.TWITCH_CLIENT_ID = 'test-client-id'
const res = await request(app).get('/api/twitch/status')
delete process.env.TWITCH_CLIENT_ID
expect(res.status).toBe(200)
expect(res.body.configured).toBe(true)
})

test('returns configured=false when TWITCH_CLIENT_ID is not set', async () => {
delete process.env.TWITCH_CLIENT_ID
const res = await request(app).get('/api/twitch/status')
expect(res.status).toBe(200)
expect(res.body.configured).toBe(false)
})
})

describe('Twitch Routes', () => {
let app: express.Express

Expand Down
7 changes: 5 additions & 2 deletions packages/frontend/src/pages/ServerLogs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,13 @@ export default function ServerLogsPage() {
levelFilter,
pageLimit,
)
: await api.serverLogs.getRecent(selectedGuild.id, pageLimit)
: await api.serverLogs.getRecent(
selectedGuild.id,
pageLimit,
)
const allLogs = res.data.logs
setLogs(allLogs.slice((page - 1) * limit, page * limit))
setTotal(allLogs.length)
setTotal(res.data.total)
} catch {
setLogs([])
setTotal(0)
Expand Down
21 changes: 21 additions & 0 deletions packages/frontend/src/pages/TwitchNotifications.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ describe('TwitchNotificationsPage', () => {
value: vi.fn(),
},
)
vi.mocked(api.twitch.status).mockResolvedValue({
data: { configured: true },
} as any)
vi.mocked(api.guilds.getChannels).mockResolvedValue({
data: {
channels: [{ id: '67890', name: '#general' }],
Expand Down Expand Up @@ -371,4 +374,22 @@ describe('TwitchNotificationsPage', () => {
await screen.findByText('Twitch user not found'),
).toBeInTheDocument()
})

test('shows not-configured banner when twitch api is not set up', async () => {
mockGuildSelection(mockGuild)
vi.mocked(api.twitch.status).mockResolvedValue({
data: { configured: false },
} as any)

renderPage()

await waitFor(() => {
expect(screen.getByText('Not Configured')).toBeInTheDocument()
})

expect(
screen.getByText(/Twitch API is not configured/),
).toBeInTheDocument()
expect(screen.queryByText('Add')).not.toBeInTheDocument()
})
})
38 changes: 37 additions & 1 deletion packages/frontend/src/pages/TwitchNotifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ export default function TwitchNotificationsPage() {
const [showAdd, setShowAdd] = useState(false)
const [newTwitchInput, setNewTwitchInput] = useState('')
const [newChannelId, setNewChannelId] = useState('')
const [twitchConfigured, setTwitchConfigured] = useState<boolean | null>(
null,
)
const notificationsRequestIdRef = useRef(0)
const channelsRequestIdRef = useRef(0)
const selectedGuildIdRef = useRef<string | undefined>(guildId)
Expand All @@ -49,6 +52,18 @@ export default function TwitchNotificationsPage() {
selectedGuildIdRef.current = guildId
}, [guildId])

useEffect(() => {
const checkTwitchStatus = async () => {
try {
const res = await api.twitch.status()
setTwitchConfigured(res.data.configured)
} catch {
setTwitchConfigured(false)
}
Comment on lines +55 to +62

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

Don’t map all status-check failures to “not configured.”

Line 61 currently sets twitchConfigured to false for any exception (network failure, timeout, 5xx), which can show an incorrect “Not Configured” banner and block actions for a temporary outage.

Suggested fix
+    const [twitchStatusError, setTwitchStatusError] = useState<string | null>(null)

     useEffect(() => {
         const checkTwitchStatus = async () => {
             try {
                 const res = await api.twitch.status()
                 setTwitchConfigured(res.data.configured)
+                setTwitchStatusError(null)
             } catch {
-                setTwitchConfigured(false)
+                setTwitchConfigured(null)
+                setTwitchStatusError('Unable to verify Twitch configuration status')
             }
         }
         checkTwitchStatus()
     }, [])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/pages/TwitchNotifications.tsx` around lines 55 - 62,
The current checkTwitchStatus async handler maps any exception from
api.twitch.status() to setTwitchConfigured(false), which treats
network/timeouts/5xx as “not configured”; update checkTwitchStatus to
distinguish a real 404/unsuccessful configuration response from transient errors
by inspecting the error/response: call api.twitch.status() and on success
setTwitchConfigured(res.data.configured); on error, if the error indicates a
definitive “not configured” (e.g., response.status === 404 or error payload
explicitly says not configured) setTwitchConfigured(false), otherwise do not
change the existing twitchConfigured state and optionally set or log a
transientError flag; reference the checkTwitchStatus function, useEffect hook,
setTwitchConfigured, and api.twitch.status to locate where to implement this
logic.

}
checkTwitchStatus()
}, [])

const channelNameById = useMemo(
() => new Map(channels.map((channel) => [channel.id, channel.name])),
[channels],
Expand Down Expand Up @@ -272,6 +287,26 @@ export default function TwitchNotificationsPage() {
)
}

if (twitchConfigured === false) {
return (
<div className='space-y-6 px-1 sm:px-0'>
<header className='flex items-center gap-3'>
<Tv className='h-6 w-6 text-purple-500' />
<h1 className='text-xl font-bold text-white'>
Twitch Notifications
</h1>
</header>
<div className='p-4 rounded-lg bg-lucky-error/10 border border-lucky-error text-lucky-error'>
<p className='font-semibold mb-1'>Not Configured</p>
<p className='text-sm'>
Twitch API is not configured on this server. Contact the
server administrator to set up Twitch credentials.
</p>
</div>
</div>
)
}

return (
<div className='space-y-6 px-1 sm:px-0'>
<header className='flex items-center justify-between'>
Expand All @@ -286,7 +321,8 @@ export default function TwitchNotificationsPage() {
</div>
<button
onClick={() => setShowAdd(!showAdd)}
className='flex cursor-pointer items-center gap-2 rounded-lg bg-purple-600/10 px-3 py-1.5 text-sm text-purple-400 transition-colors hover:bg-purple-600/20'
disabled={!twitchConfigured}
className='flex cursor-pointer items-center gap-2 rounded-lg bg-purple-600/10 px-3 py-1.5 text-sm text-purple-400 transition-colors hover:bg-purple-600/20 disabled:opacity-50 disabled:cursor-not-allowed'
>
<Plus className='w-4 h-4' />
Add
Expand Down
1 change: 1 addition & 0 deletions packages/frontend/src/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ export const api = {
},

twitch: {
status: () => apiClient.get<{ configured: boolean }>('/twitch/status'),
lookupUser: (login: string) =>
apiClient.get<{
id: string
Expand Down
18 changes: 12 additions & 6 deletions packages/frontend/src/services/logsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@ import type { ServerLog } from '@/types'
export function createLogsApi(apiClient: AxiosInstance) {
return {
getRecent: (guildId: string, limit?: number) =>
apiClient.get<{ logs: ServerLog[] }>(`/guilds/${guildId}/logs`, {
params: limit ? { limit } : {},
}),
apiClient.get<{ logs: ServerLog[]; total: number }>(
`/guilds/${guildId}/logs`,
{
params: limit ? { limit } : {},
},
),
getByType: (guildId: string, type: string, limit?: number) =>
apiClient.get<{ logs: ServerLog[] }>(`/guilds/${guildId}/logs`, {
params: { type, ...(limit ? { limit } : {}) },
}),
apiClient.get<{ logs: ServerLog[]; total: number }>(
`/guilds/${guildId}/logs`,
{
params: { type, ...(limit ? { limit } : {}) },
},
),
Comment on lines 6 to +19

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

Use explicit undefined checks for limit query params.

Current truthy checks omit limit when it is 0 (Line 10 and Line 17), which can silently change request behavior.

Suggested fix
         getRecent: (guildId: string, limit?: number) =>
             apiClient.get<{ logs: ServerLog[]; total: number }>(
                 `/guilds/${guildId}/logs`,
                 {
-                    params: limit ? { limit } : {},
+                    params: limit !== undefined ? { limit } : {},
                 },
             ),
         getByType: (guildId: string, type: string, limit?: number) =>
             apiClient.get<{ logs: ServerLog[]; total: number }>(
                 `/guilds/${guildId}/logs`,
                 {
-                    params: { type, ...(limit ? { limit } : {}) },
+                    params: { type, ...(limit !== undefined ? { limit } : {}) },
                 },
             ),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/services/logsApi.ts` around lines 6 - 19, The current
getRecent and getByType calls drop limit when it is 0 because they use truthy
checks; change the param handling to test for undefined explicitly (e.g., limit
!== undefined) so a numeric 0 is passed through; update the params objects in
getRecent and getByType (the apiClient.get calls) to include limit when limit
!== undefined while keeping other params (like type) intact.

search: (
guildId: string,
filters: { type?: string; userId?: string },
Expand Down
10 changes: 10 additions & 0 deletions packages/shared/src/services/ServerLogService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,16 @@ export class ServerLogService {
})
}

async countRecentLogs(guildId: string) {
return await prisma.serverLog.count({ where: { guildId } })
}

async countLogsByType(guildId: string, type: LogType) {
return await prisma.serverLog.count({
where: { guildId, type },
})
}

async searchLogs(
guildId: string,
filters: {
Expand Down