🛠️ DX: allow userRatings and userReviews to accept URLs#192
Conversation
Developers no longer need to manually parse IDs when using these services. It uses `extractId` like the movie and creator services do. Co-authored-by: bartholomej <5861310+bartholomej@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThis PR extends the URL-based ID parser to recognize pure numeric path segments, then applies this standardized extraction with validation across the user ratings and user reviews services to ensure consistent ID handling in both initial and paginated requests. ChangesURL ID Extraction Standardization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/helpers/global.helper.tsOops! Something went wrong! :( ESLint: 10.4.0 TypeError [ERR_IMPORT_ATTRIBUTE_MISSING]: Module "file:///.eslintrc.json?mtime=1779359995033" needs an import attribute of "type: json" src/services/user-ratings.service.tsOops! Something went wrong! :( ESLint: 10.4.0 TypeError [ERR_IMPORT_ATTRIBUTE_MISSING]: Module "file:///.eslintrc.json?mtime=1779359995033" needs an import attribute of "type: json" src/services/user-reviews.service.tsOops! Something went wrong! :( ESLint: 10.4.0 TypeError [ERR_IMPORT_ATTRIBUTE_MISSING]: Module "file:///.eslintrc.json?mtime=1779359995033" needs an import attribute of "type: json" 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #192 +/- ##
==========================================
- Coverage 98.71% 98.47% -0.25%
==========================================
Files 34 34
Lines 781 789 +8
Branches 202 206 +4
==========================================
+ Hits 771 777 +6
- Misses 10 12 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/helpers/global.helper.ts`:
- Around line 19-22: Normalize the path segment before checking for a pure
numeric ID: in the logic that tests the variable p with /^\d+$/ (the numeric ID
branch), first strip any query or hash suffix by taking the substring up to the
first '?' or '#' (e.g., p.split(/[?#]/)[0]) then run the /^\d+$/ test and return
the numeric value (+parsed || null); update the numeric branch that references p
so it uses the cleaned segment.
In `@src/services/user-ratings.service.ts`:
- Around line 25-28: The current guard using extractId(user) allows 0,
negatives, decimals, and Infinity; update the check after calling extractId to
ensure id is a finite positive integer (id > 0 && Number.isInteger(id) &&
Number.isFinite(id)) and reject anything else by throwing the same
Error('node-csfd-api: user must be a valid number or URL'); modify the
validation logic in the user-ratings.service.ts around the extractId call to
enforce these constraints (reference function/variable: extractId and id).
🪄 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: f00c39db-8651-4ded-879f-8097df83ff01
📒 Files selected for processing (3)
src/helpers/global.helper.tssrc/services/user-ratings.service.tssrc/services/user-reviews.service.ts
| // Also support pure numbers in URL parts (e.g., /uzivatel/1000/) | ||
| if (/^\d+$/.test(p)) { | ||
| return +p || null; | ||
| } |
There was a problem hiding this comment.
Normalize path segments before numeric ID matching.
Line 20 fails for URLs where the numeric segment includes query/hash suffixes (e.g., /uzivatel/1000/?foo=bar). Strip ?/# first so numeric URLs are parsed consistently.
Suggested fix
- const p = parts[i];
+ const p = parts[i]?.split(/[?#]/)[0] ?? '';
if (/^\d+-/.test(p)) {
return +p.split('-')[0] || null;
}
// Also support pure numbers in URL parts (e.g., /uzivatel/1000/)
if (/^\d+$/.test(p)) {
return +p || null;
}🤖 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 `@src/helpers/global.helper.ts` around lines 19 - 22, Normalize the path
segment before checking for a pure numeric ID: in the logic that tests the
variable p with /^\d+$/ (the numeric ID branch), first strip any query or hash
suffix by taking the substring up to the first '?' or '#' (e.g.,
p.split(/[?#]/)[0]) then run the /^\d+$/ test and return the numeric value
(+parsed || null); update the numeric branch that references p so it uses the
cleaned segment.
| const id = extractId(user); | ||
| if (id === null || isNaN(id)) { | ||
| throw new Error('node-csfd-api: user must be a valid number or URL'); | ||
| } |
There was a problem hiding this comment.
Strengthen user ID guard to positive integers only.
Line 26 still allows values like 0, negative, decimal, or Infinity for numeric inputs. Reject those early to keep request URLs valid and behavior consistent.
Suggested fix
const id = extractId(user);
- if (id === null || isNaN(id)) {
+ if (id === null || !Number.isInteger(id) || id <= 0) {
throw new Error('node-csfd-api: user must be a valid number or URL');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const id = extractId(user); | |
| if (id === null || isNaN(id)) { | |
| throw new Error('node-csfd-api: user must be a valid number or URL'); | |
| } | |
| const id = extractId(user); | |
| if (id === null || !Number.isInteger(id) || id <= 0) { | |
| throw new Error('node-csfd-api: user must be a valid number or URL'); | |
| } |
🤖 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 `@src/services/user-ratings.service.ts` around lines 25 - 28, The current guard
using extractId(user) allows 0, negatives, decimals, and Infinity; update the
check after calling extractId to ensure id is a finite positive integer (id > 0
&& Number.isInteger(id) && Number.isFinite(id)) and reject anything else by
throwing the same Error('node-csfd-api: user must be a valid number or URL');
modify the validation logic in the user-ratings.service.ts around the extractId
call to enforce these constraints (reference function/variable: extractId and
id).
💡 What: Updated
userRatingsanduserReviewsservices to useextractId, allowing them to accept raw numeric IDs, string IDs, slugs, and full URLs. Also improvedparseIdFromUrlto support URLs without a text slug (e.g.,/uzivatel/1000/).🎯 Why: To reduce boilerplate for developers using the library. They no longer have to manually parse the numeric ID out of user profile URLs when using
csfd.userRatings()orcsfd.userReviews().🚀 Examples:
Before:
After:
PR created automatically by Jules for task 3544235191838941380 started by @bartholomej
Summary by CodeRabbit
Release Notes
/user/1000/) and formatted URL slugs