Skip to content

🛠️ DX: allow userRatings and userReviews to accept URLs#192

Open
bartholomej wants to merge 1 commit into
masterfrom
jules-3544235191838941380-24365e40
Open

🛠️ DX: allow userRatings and userReviews to accept URLs#192
bartholomej wants to merge 1 commit into
masterfrom
jules-3544235191838941380-24365e40

Conversation

@bartholomej
Copy link
Copy Markdown
Owner

@bartholomej bartholomej commented May 21, 2026

💡 What: Updated userRatings and userReviews services to use extractId, allowing them to accept raw numeric IDs, string IDs, slugs, and full URLs. Also improved parseIdFromUrl to 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() or csfd.userReviews().

🚀 Examples:
Before:

const url = 'https://www.csfd.cz/uzivatel/1000-pomoc/';
// Needed to parse the ID manually
const result = await csfd.userRatings(1000); 

After:

const url = 'https://www.csfd.cz/uzivatel/1000-pomoc/';
// Works out of the box!
const result = await csfd.userRatings(url); 

PR created automatically by Jules for task 3544235191838941380 started by @bartholomej

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • URL parsing now properly recognizes numeric user identifiers in both direct path segments (e.g., /user/1000/) and formatted URL slugs
    • User ratings and reviews services now extract and validate user identifiers before making requests, preventing potential errors from invalid inputs
    • Pagination requests for user ratings and reviews now consistently use validated identifiers

Review Change Stack

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>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

📝 Walkthrough

Walkthrough

This 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.

Changes

URL ID Extraction Standardization

Layer / File(s) Summary
Extended URL ID parsing
src/helpers/global.helper.ts
parseIdFromUrl now detects and returns pure numeric URL segments (e.g., /uzivatel/1000/) in addition to existing id-slug format extraction.
User ratings ID extraction and validation
src/services/user-ratings.service.ts
userRatings imports extractId, derives and validates a numeric ID from user input, and uses the extracted ID for both initial and paginated rating requests.
User reviews ID extraction and validation
src/services/user-reviews.service.ts
userReviews imports extractId, derives and validates a numeric ID from user input, and uses the extracted ID for both initial and paginated review requests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

A rabbit hops through URLs bright,
Extracting IDs left and right,
Numeric paths now clearly seen,
From ratings, reviews, and in-between! 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is comprehensive with clear motivation, examples, and technical context. However, it does not follow the required template structure with Type of change, Related Issues, and Checklist sections. Complete the required template sections: select Type of change (appears to be New feature or Refactoring), include Related Issues if applicable, and confirm checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: enabling userRatings and userReviews services to accept URLs as input.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jules-3544235191838941380-24365e40

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/helpers/global.helper.ts

Oops! 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"
at validateAttributes (node:internal/modules/esm/assert:88:15)
at defaultLoadSync (node:internal/modules/esm/load:164:3)
at #loadAndMaybeBlockOnLoaderThread (node:internal/modules/esm/loader:776:12)
at #loadSync (node:internal/modules/esm/loader:796:49)
at ModuleLoader.load (node:internal/modules/esm/loader:762:26)
at ModuleLoader.loadAndTranslate (node:internal/modules/esm/loader:504:31)
at #getOrCreateModuleJobAfterResolve (node:internal/modules/esm/loader:555:36)
at afterResolve (node:internal/modules/esm/loader:603:52)
at ModuleLoader.getOrCreateModuleJob (node:internal/modules/esm/loader:609:12)
at node:internal/modules/esm/loader:628:32

src/services/user-ratings.service.ts

Oops! 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"
at validateAttributes (node:internal/modules/esm/assert:88:15)
at defaultLoadSync (node:internal/modules/esm/load:164:3)
at #loadAndMaybeBlockOnLoaderThread (node:internal/modules/esm/loader:776:12)
at #loadSync (node:internal/modules/esm/loader:796:49)
at ModuleLoader.load (node:internal/modules/esm/loader:762:26)
at ModuleLoader.loadAndTranslate (node:internal/modules/esm/loader:504:31)
at #getOrCreateModuleJobAfterResolve (node:internal/modules/esm/loader:555:36)
at afterResolve (node:internal/modules/esm/loader:603:52)
at ModuleLoader.getOrCreateModuleJob (node:internal/modules/esm/loader:609:12)
at node:internal/modules/esm/loader:628:32

src/services/user-reviews.service.ts

Oops! 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"
at validateAttributes (node:internal/modules/esm/assert:88:15)
at defaultLoadSync (node:internal/modules/esm/load:164:3)
at #loadAndMaybeBlockOnLoaderThread (node:internal/modules/esm/loader:776:12)
at #loadSync (node:internal/modules/esm/loader:796:49)
at ModuleLoader.load (node:internal/modules/esm/loader:762:26)
at ModuleLoader.loadAndTranslate (node:internal/modules/esm/loader:504:31)
at #getOrCreateModuleJobAfterResolve (node:internal/modules/esm/loader:555:36)
at afterResolve (node:internal/modules/esm/loader:603:52)
at ModuleLoader.getOrCreateModuleJob (node:internal/modules/esm/loader:609:12)
at node:internal/modules/esm/loader:628:32


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

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

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.47%. Comparing base (51c7528) to head (8a3ebad).

Files with missing lines Patch % Lines
src/services/user-ratings.service.ts 80.00% 1 Missing ⚠️
src/services/user-reviews.service.ts 80.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 51c7528 and 8a3ebad.

📒 Files selected for processing (3)
  • src/helpers/global.helper.ts
  • src/services/user-ratings.service.ts
  • src/services/user-reviews.service.ts

Comment on lines +19 to +22
// Also support pure numbers in URL parts (e.g., /uzivatel/1000/)
if (/^\d+$/.test(p)) {
return +p || null;
}
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 | ⚡ Quick win

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.

Comment on lines +25 to +28
const id = extractId(user);
if (id === null || isNaN(id)) {
throw new Error('node-csfd-api: user must be a valid number or URL');
}
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 | ⚡ Quick win

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.

Suggested change
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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants