Conversation
Fixed 3 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Fixed 3 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
fix: CodeRabbit auto-fixes for PR #27
…er nickname is required for the !stats command and improve usage examples
📝 WalkthroughWalkthroughThe pull request updates the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
175-192:⚠️ Potential issue | 🟡 MinorDocumented response text does not match what the server actually sends.
README documents the missing-player body as:
Indica o nickname FACEIT — ex.: !stats s1mpleBut
src/routes/stats.js(line 42) sends:
Indique o nickname FACEIT (ex.: !stats s1mple)Differences: verb form (
IndicavsIndique— imperative mood isIndiquein European/BR Portuguese) and punctuation (em-dash vs parentheses). Please reconcile both occurrences (lines 175 and 192 here) with the code, or vice versa, so the curl example output shown in the README matches reality.📝 Proposed README alignment (if keeping the code text)
-**Missing `player`:** `200` with body `Indica o nickname FACEIT — ex.: !stats s1mple` (plain text for Twitch `urlfetch`). +**Missing `player`:** `200` with body `Indique o nickname FACEIT (ex.: !stats s1mple)` (plain text for Twitch `urlfetch`). ... -# Indica o nickname FACEIT — ex.: !stats s1mple +# Indique o nickname FACEIT (ex.: !stats s1mple)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 175 - 192, The README message for the missing-player response doesn't match the actual text returned by the server; update the README occurrences (around lines 175 and 192) to exactly match the string emitted by src/routes/stats.js (line 42): "Indique o nickname FACEIT (ex.: !stats s1mple)" so the curl example output shown in README reflects reality.
🧹 Nitpick comments (1)
src/routes/elo.js (1)
27-28: ExtractgetStringParamto a shared helper (DRY).The exact same one-liner is now inlined in
src/routes/elo.js,src/routes/streak.js, andsrc/routes/stats.js(with a slightly different formatting instats.js). Any future tweak (e.g. handling empty strings, array of multiple values, non-ASCII whitespace) would need to be applied in three places and risks diverging — and the three copies already vary in code style.Consider extracting to something like
src/utils/queryParams.jsand importing it from each route:♻️ Proposed refactor
// src/utils/queryParams.js export const getStringParam = (p) => { if (typeof p === "string") return p.trim(); if (Array.isArray(p)) return p[0]?.trim() ?? null; return null; };Then in each route:
+import { getStringParam } from "../utils/queryParams.js"; ... - const getStringParam = (p) => typeof p === 'string' ? p.trim() : Array.isArray(p) ? p[0]?.trim() ?? null : null; - const playerQuery = getStringParam(req.query.player) || getStringParam(req.query.nick) || null; + const playerQuery = + getStringParam(req.query.player) || getStringParam(req.query.nick) || null;Also note: the trailing
|| nullinplayerQueryis redundant —getStringParamalready returnsnullfor non-string/non-array inputs, and any falsy string result ("") combined with a missing second param still yieldsundefined→nullvia the final|| null, so it can be kept purely for explicitness. Minor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/elo.js` around lines 27 - 28, Extract the one-liner getStringParam into a shared helper (e.g., create src/utils/queryParams.js exporting getStringParam) and replace the inline definitions in src/routes/elo.js, src/routes/streak.js, and src/routes/stats.js with an import of that helper; update each route to call the shared getStringParam for request query values (e.g., playerQuery) and remove the redundant trailing "|| null" where getStringParam already returns null for non-string/non-array inputs so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/stats.js`:
- Around line 39-43: The response string returned when playerQuery is missing in
the stats route handler ("Indique o nickname FACEIT (ex.: !stats s1mple)")
diverges from README.md ("Indica o nickname FACEIT — ex.: !stats s1mple"); pick
one canonical message and make both the code and README consistent. Update the
string in the stats route's missing-player conditional (the block that checks
playerQuery and calls res.status(200).send(...)) to exactly match the README
text, or change the README entries (lines mentioning the curl example) to match
the handler’s string—ensure the message text, punctuation, and example format
are identical in both places.
---
Outside diff comments:
In `@README.md`:
- Around line 175-192: The README message for the missing-player response
doesn't match the actual text returned by the server; update the README
occurrences (around lines 175 and 192) to exactly match the string emitted by
src/routes/stats.js (line 42): "Indique o nickname FACEIT (ex.: !stats s1mple)"
so the curl example output shown in README reflects reality.
---
Nitpick comments:
In `@src/routes/elo.js`:
- Around line 27-28: Extract the one-liner getStringParam into a shared helper
(e.g., create src/utils/queryParams.js exporting getStringParam) and replace the
inline definitions in src/routes/elo.js, src/routes/streak.js, and
src/routes/stats.js with an import of that helper; update each route to call the
shared getStringParam for request query values (e.g., playerQuery) and remove
the redundant trailing "|| null" where getStringParam already returns null for
non-string/non-array inputs so behavior remains identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc09be9a-01c9-445e-8d15-0730e419f9cb
📒 Files selected for processing (5)
README.mdsrc/index.jssrc/routes/elo.jssrc/routes/stats.jssrc/routes/streak.js
| if (!playerQuery) { | ||
| return res | ||
| .status(200) | ||
| .send("Indique o nickname FACEIT (ex.: !stats s1mple)"); | ||
| } |
There was a problem hiding this comment.
Missing-player response text diverges from README.
This handler returns Indique o nickname FACEIT (ex.: !stats s1mple), but README.md (lines 175 and 192) documents the body as Indica o nickname FACEIT — ex.: !stats s1mple. Differences in verb form and punctuation will make the documented curl example output literally wrong. Pick one canonical string and use it in both places.
📝 Proposed fix (keeping the code's imperative form)
if (!playerQuery) {
return res
.status(200)
.send("Indique o nickname FACEIT (ex.: !stats s1mple)");
}…and update README.md lines 175 and 192 to match, or adjust this string to match the README — either direction is fine as long as they agree.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/stats.js` around lines 39 - 43, The response string returned when
playerQuery is missing in the stats route handler ("Indique o nickname FACEIT
(ex.: !stats s1mple)") diverges from README.md ("Indica o nickname FACEIT — ex.:
!stats s1mple"); pick one canonical message and make both the code and README
consistent. Update the string in the stats route's missing-player conditional
(the block that checks playerQuery and calls res.status(200).send(...)) to
exactly match the README text, or change the README entries (lines mentioning
the curl example) to match the handler’s string—ensure the message text,
punctuation, and example format are identical in both places.
1 participant
This pull request clarifies and enforces the requirement that the
/statsendpoint must be called with a player nickname, both in the backend logic and in the documentation. It also improves how query parameters are parsed for all endpoints, ensuring consistent handling of string and array types. The changes update the user-facing documentation and error messaging to prevent confusion about required parameters.Backend logic and API behavior:
/statsendpoint now requires aplayer(ornick) query parameter; if missing, it returns a 200 response with a plain text prompt asking for the FACEIT nickname, rather than using a default player. [1] [2]playerandnickquery parameters is now consistent across/stats,/elo, and/streakendpoints, correctly handling both string and array values. [1] [2] [3]Documentation updates:
README.mdhas been updated to clearly state that the!statscommand requires a nickname and to show the new required usage and error message when the nickname is missing. [1] [2] [3] [4] [5]/statsendpoint description in the server startup log now explicitly notes that the player parameter is required.