-
Notifications
You must be signed in to change notification settings - Fork 1
feat: outbound webhook notifications for important bot events (#130) #219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2d2418a
f750a4e
1c854c6
1f619ec
5a6860e
dc5f990
44cd6ba
4561f7c
ca04d36
d207c97
f041302
14bcf2c
4051e0b
1fe44d7
51fd8c4
a715c9b
6188984
496e6e7
1e58a02
82f6775
a73c92a
9b69aba
2b3939a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,8 @@ | |
| "!node_modules", | ||
| "!coverage", | ||
| "!logs", | ||
| "!data" | ||
| "!data", | ||
| "!feat-issue-164" | ||
BillChirico marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check if the feat-issue-164 directory exists
if [ -d "feat-issue-164" ]; then
echo "=== Directory exists ==="
echo "Contents:"
find feat-issue-164 -type f | head -20
echo ""
echo "Directory structure:"
ls -la feat-issue-164
else
echo "Directory does not exist"
fi
echo ""
echo "=== Searching for references to feat-issue-164 or issue-164 ==="
rg "feat-issue-164|issue-164|feat.issue.164" --max-count 20Repository: VolvoxLLC/volvox-bot Length of output: 188 🏁 Script executed: #!/bin/bash
# Also check the full biome.json file to understand context
echo "=== Full biome.json content ==="
cat biome.jsonRepository: VolvoxLLC/volvox-bot Length of output: 1233 Remove the stale The 🤖 Prompt for AI Agents |
||
| ] | ||
| }, | ||
| "linter": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* eslint-disable */ | ||
| 'use strict'; | ||
|
|
||
| /** | ||
| * Migration 005: Webhook notifications delivery log | ||
| * | ||
| * Creates webhook_delivery_log table to store delivery attempts | ||
| * per webhook endpoint. Endpoint configs live in the per-guild config JSON. | ||
| */ | ||
|
|
||
BillChirico marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** @param {import('node-pg-migrate').MigrationBuilder} pgm */ | ||
| exports.up = (pgm) => { | ||
| pgm.sql(` | ||
| CREATE TABLE IF NOT EXISTS webhook_delivery_log ( | ||
| id SERIAL PRIMARY KEY, | ||
| guild_id TEXT NOT NULL, | ||
| endpoint_id TEXT NOT NULL, | ||
| event_type TEXT NOT NULL, | ||
| payload JSONB NOT NULL, | ||
| status TEXT NOT NULL CHECK (status IN ('success', 'failed', 'pending')), | ||
BillChirico marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| response_code INTEGER, | ||
| response_body TEXT, | ||
| attempt INTEGER NOT NULL DEFAULT 1, | ||
| delivered_at TIMESTAMPTZ DEFAULT NOW() | ||
| ); | ||
|
|
||
| CREATE INDEX IF NOT EXISTS idx_webhook_delivery_log_guild | ||
| ON webhook_delivery_log (guild_id, delivered_at DESC); | ||
|
|
||
| CREATE INDEX IF NOT EXISTS idx_webhook_delivery_log_endpoint | ||
| ON webhook_delivery_log (endpoint_id, delivered_at DESC); | ||
| `); | ||
| }; | ||
|
|
||
| /** @param {import('node-pg-migrate').MigrationBuilder} pgm */ | ||
| exports.down = (pgm) => { | ||
| pgm.sql(` | ||
| DROP INDEX IF EXISTS idx_webhook_delivery_log_endpoint; | ||
| DROP INDEX IF EXISTS idx_webhook_delivery_log_guild; | ||
| DROP TABLE IF EXISTS webhook_delivery_log; | ||
| `); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,6 +92,11 @@ router.get( | |
| async (req, res, next) => { | ||
| try { | ||
| const guildId = req.params.id; | ||
| const pool = req.app.locals.dbPool; | ||
|
|
||
| if (!pool) { | ||
| return res.status(503).json({ error: 'Database unavailable' }); | ||
| } | ||
|
Comment on lines
+95
to
+99
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use route-standard custom errors (with context logging) instead of inline 503 JSON responses. These early returns bypass the shared API error contract and skip contextual logging for operational diagnosis. Replace direct As per coding guidelines Also applies to: 197-201 🤖 Prompt for AI Agents |
||
|
|
||
| let days = 30; | ||
| if (req.query.days !== undefined) { | ||
|
|
@@ -101,9 +106,10 @@ router.get( | |
| } | ||
| } | ||
|
|
||
| // Let errors bubble up to the outer catch block | ||
| const [stats, trend] = await Promise.all([ | ||
| getFeedbackStats(guildId), | ||
| getFeedbackTrend(guildId, days), | ||
| getFeedbackStats(guildId, pool), | ||
| getFeedbackTrend(guildId, days, pool), | ||
| ]); | ||
|
|
||
| res.json({ | ||
|
|
@@ -188,6 +194,11 @@ router.get( | |
| async (req, res, next) => { | ||
| try { | ||
| const guildId = req.params.id; | ||
| const pool = req.app.locals.dbPool; | ||
|
|
||
| if (!pool) { | ||
| return res.status(503).json({ error: 'Database unavailable' }); | ||
| } | ||
|
|
||
| let limit = 25; | ||
| if (req.query.limit !== undefined) { | ||
|
|
@@ -197,7 +208,19 @@ router.get( | |
| } | ||
| } | ||
|
|
||
| const feedback = await getRecentFeedback(guildId, limit); | ||
| // Let errors bubble up to the outer catch block | ||
| const rawFeedback = await getRecentFeedback(guildId, limit, pool); | ||
|
|
||
| // Normalize DB row keys to camelCase (handles both raw SQL and aliased results) | ||
| const feedback = rawFeedback.map((row) => ({ | ||
| id: row.id, | ||
| messageId: row.messageId ?? row.message_id, | ||
| channelId: row.channelId ?? row.channel_id, | ||
| userId: row.userId ?? row.user_id, | ||
| feedbackType: row.feedbackType ?? row.feedback_type, | ||
| createdAt: row.createdAt ?? row.created_at, | ||
| })); | ||
|
Comment on lines
+214
to
+222
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider removing redundant snake_case fallbacks in response mapping.
🤖 Prompt for AI Agents |
||
|
|
||
| res.json({ feedback }); | ||
| } catch (err) { | ||
| next(err); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
feat-issue-164from ignore list and remove the submodule from the repository (per previous feedback)Prompt To Fix With AI