refactor: deduplicate AI feedback SQL queries (#188)

[BillChirico]

Summary

Fixes #188.

The AI feedback API route was implemented (in #190) calling module functions, but the route tests were never updated — they still mocked pool.query directly, causing 10 test failures. The route also lost its 503 DB-unavailable guard during the refactor.

Changes

src/api/routes/ai-feedback.js

• Restored 503 guard (req.app.locals.dbPool check) to both /stats and /recent handlers
• No inline SQL — route exclusively delegates to getFeedbackStats, getFeedbackTrend, getRecentFeedback from aiFeedback.js
• Error propagation uses next(err) (Express error middleware)

tests/api/routes/ai-feedback.test.js

• Replaced pool.query mocks with vi.mock('../../../src/modules/aiFeedback.js')
• Route tests now verify correct module function calls + args (e.g. getFeedbackTrend(guildId, 7) for ?days=7)
• Decouples route tests from SQL schema — module tests cover the DB layer independently

Test Results

• ✅ 24/24 ai-feedback tests pass (12 route + 12 module)
• ✅ Pre-existing failures in redis.test.js and triage.coverage.test.js are unrelated to this PR

[coderabbitai]

Warning

Rate limit exceeded

@BillChirico has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 24 minutes and 48 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between bd0658f and 56a26ed.

📒 Files selected for processing (2)

• src/api/routes/ai-feedback.js
• tests/api/routes/ai-feedback.test.js

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/issue-188

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR refactors the AI feedback API route to rely exclusively on the aiFeedback module (removing duplicated SQL concerns from the route layer), restores the DB-unavailable (503) guard, and updates route tests to mock module functions instead of mocking pool.query directly.

Changes:

• Restored 503 Database unavailable guard to both /stats and /recent route handlers.
• Ensured the route delegates to getFeedbackStats, getFeedbackTrend, and getRecentFeedback and uses next(err) for error propagation.
• Updated route tests to mock src/modules/aiFeedback.js and assert correct module call arguments.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/api/routes/ai-feedback.js	Adds back the 503 DB-unavailable guard for /stats and /recent while keeping handlers delegated to module functions.
tests/api/routes/ai-feedback.test.js	Refactors tests to mock aiFeedback module functions and validate handler-module integration (args, defaults, query params).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

Refactored AI feedback routes to delegate all SQL to aiFeedback module functions, decoupling route tests from database schema. Restored 503 guards for database unavailability that were lost during the initial refactor in #190.

Key Changes:

• Both /stats and /recent endpoints now call getFeedbackStats, getFeedbackTrend, and getRecentFeedback from the module layer
• Added database availability checks to prevent errors when pool is null
• Tests now mock module functions instead of pool.query, making them independent of SQL implementation
• Error handling uses next(err) to delegate to Express error middleware

Issues Found:

• Duplicate database pool checks in both route handlers (one check is sufficient)
• Test assertions don't verify that pool parameter is passed to module functions, reducing test coverage of the dependency injection pattern

Confidence Score: 4/5

• Safe to merge after addressing style issues — refactoring is sound with proper separation of concerns
• The refactoring successfully achieves its goal of decoupling route tests from SQL. No logic bugs or security issues introduced. The duplicate pool checks and incomplete test assertions are style/quality issues that don't affect runtime behavior.
• Both files have minor style improvements recommended, but no files require critical attention

Important Files Changed

Filename	Overview
src/api/routes/ai-feedback.js	Restored 503 DB-unavailable guards to both endpoints; duplicate checks should be consolidated
tests/api/routes/ai-feedback.test.js	Refactored to mock module functions instead of pool.query; test assertions should verify pool parameter

_{Last reviewed commit: 56a26ed}