🛠️ DX: Support URL extraction for user APIs#209
Conversation
Added an `extractUser` helper function in `src/helpers/global.helper.ts` to allow users to pass full URLs, text slugs, numeric slugs, or numbers into `csfd.userRatings()` and `csfd.userReviews()`. Added corresponding unit tests in `tests/helpers.test.ts`. 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. |
📝 WalkthroughWalkthroughAdded ChangesUser parsing normalization
Sequence Diagram(s)sequenceDiagram
participant URat as UserRatingsScraper.userRatings
participant E as extractUser
participant URev as UserReviewsScraper.userReviews
URat->>E: normalize user
E-->>URat: userIdOrSlug
URat->>URat: build initial and paginated ratings URLs
URev->>E: normalize user
E-->>URev: userIdOrSlug
URev->>URev: build initial and paginated review URLs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 ReportCaution This repository is currently using the Sentry GitHub App to receive Codecov PR comments. This integration will be deprecated on July 8, 2026. Please install the Codecov GitHub App to continue receiving coverage reports on your pull requests. Additional details and impacted files@@ Coverage Diff @@
## master #209 +/- ##
==========================================
- Coverage 98.71% 98.26% -0.46%
==========================================
Files 34 34
Lines 781 807 +26
Branches 202 216 +14
==========================================
+ Hits 771 793 +22
- Misses 10 13 +3
- Partials 0 1 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (2)
src/services/user-ratings.service.ts (1)
25-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: hoist the duplicated normalize-and-validate block.
The same
extractUser(user)+ null/NaN guard appears verbatim here and inuser-reviews.service.ts. A small shared helper (e.g.resolveUser(user): number | string) would keep the contract and error message in one place. Thetypeof userIdOrSlug === 'number' && isNaN(...)arm is effectively unreachable today sinceextractUserreturnsnull(not a NaN number) for unparseable input.🤖 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 normalize-and-validate logic for user input is duplicated in UserRatingsService and user-reviews.service, so extract it into a shared helper like resolveUser(user) that returns a validated number or string and centralizes the error message. Update UserRatingsService to call that helper instead of repeating extractUser(user) plus the null/NaN guard, and remove the unreachable NaN branch since extractUser currently returns null for invalid input.tests/helpers.test.ts (1)
70-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd negative-path coverage for
extractUser.The suite only exercises happy paths. Given the fallback behavior flagged in
global.helper.ts, consider adding cases for invalid/empty input and a non-user URL to lock in the intended contract (and prevent regressions of the double-encoding fix).💚 Example additions
test('should return string for full url with text slug', () => { expect(extractUser('https://www.csfd.cz/uzivatel/admin/')).toBe('admin'); }); + test('should reject empty input', () => { + expect(extractUser('')).toBe(null); + }); + test('should reject a non-user url', () => { + expect(extractUser('https://www.csfd.cz/film/228329-avatar/')).toBe(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 `@tests/helpers.test.ts` around lines 70 - 89, The extractUser test suite only covers valid inputs, so add negative-path cases to lock in the fallback behavior in extractUser from global.helper.ts. Extend the existing extractUser describe block with tests for empty/invalid input and a non-user URL, and assert the intended return values so the contract stays stable alongside the double-encoding fix.
🤖 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 68-91: The extractUser logic in global.helper is falling back to
returning the raw input even when it looks like a URL, which later gets
double-encoded by userUrl and can produce broken links. Update the fallback in
extractUser so any userOrUrl value containing “/” returns null unless a valid
“uzivatel” slug was extracted, while keeping the existing numeric-slug and
plain-slug handling intact.
---
Nitpick comments:
In `@src/services/user-ratings.service.ts`:
- Around line 25-28: The normalize-and-validate logic for user input is
duplicated in UserRatingsService and user-reviews.service, so extract it into a
shared helper like resolveUser(user) that returns a validated number or string
and centralizes the error message. Update UserRatingsService to call that helper
instead of repeating extractUser(user) plus the null/NaN guard, and remove the
unreachable NaN branch since extractUser currently returns null for invalid
input.
In `@tests/helpers.test.ts`:
- Around line 70-89: The extractUser test suite only covers valid inputs, so add
negative-path cases to lock in the fallback behavior in extractUser from
global.helper.ts. Extend the existing extractUser describe block with tests for
empty/invalid input and a non-user URL, and assert the intended return values so
the contract stays stable alongside the double-encoding fix.
🪄 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: 1edad948-e744-4178-9205-fb77b44aaa1a
📒 Files selected for processing (4)
src/helpers/global.helper.tssrc/services/user-ratings.service.tssrc/services/user-reviews.service.tstests/helpers.test.ts
| // Check if it's a URL | ||
| if (userOrUrl.includes('/')) { | ||
| const parts = userOrUrl.split('/').filter(Boolean); | ||
| const uzivatelIndex = parts.indexOf('uzivatel'); | ||
| if (uzivatelIndex !== -1 && parts.length > uzivatelIndex + 1) { | ||
| const userSlug = parts[uzivatelIndex + 1]; | ||
| // If slug is numeric, return number, else return slug string | ||
| if (/^\d+-/.test(userSlug)) { | ||
| return +userSlug.split('-')[0] || null; | ||
| } | ||
| if (/^\d+$/.test(userSlug)) { | ||
| return +userSlug || null; | ||
| } | ||
| return userSlug; | ||
| } | ||
| } | ||
|
|
||
| // Direct numeric slug with ID prefix (e.g. "912-hobit") | ||
| if (/^\d+-/.test(userOrUrl)) { | ||
| return +userOrUrl.split('-')[0] || null; | ||
| } | ||
|
|
||
| // Return string slug as fallback (e.g. "admin") | ||
| return userOrUrl; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'export const userUrl' src/vars.tsRepository: bartholomej/node-csfd-api
Length of output: 462
Returning the raw URL in the fallback triggers double-encoding and 404 errors.
The userUrl helper constructs URLs via ${baseUrl}/uzivatel/${encodeURIComponent(user)}. When extractUser receives a URL string missing the uzivatel segment (skipping the extraction logic) and executes return userOrUrl, that full URL is passed to encodeURIComponent. This encodes the internal slashes (e.g., http:// becomes http%3A%2F%2F), creating a malformed URL that resolves to a 404.
Change the fallback to return null for strings containing / to ensure invalid inputs fail fast rather than propagating to the URL builder.
// File: src/helpers/global.helper.ts
// Currently at the end of the function:
// return userOrUrl;
// Proposed:
if (userOrUrl.includes('/')) {
return null;
}
return userOrUrl;🤖 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 68 - 91, The extractUser logic in
global.helper is falling back to returning the raw input even when it looks like
a URL, which later gets double-encoded by userUrl and can produce broken links.
Update the fallback in extractUser so any userOrUrl value containing “/” returns
null unless a valid “uzivatel” slug was extracted, while keeping the existing
numeric-slug and plain-slug handling intact.
2 participants
💡 What:
Introduced an
extractUserhelper touserRatingsanduserReviewsAPIs. Developers can now pass a full URL string (e.g.,'https://www.csfd.cz/uzivatel/admin/') or text slug (e.g.,'admin'), and the API will natively extract the correct user identifier without throwing a 404.🎯 Why:
CSFD user endpoints can take both numeric IDs (
912) and text slugs (admin). Previously, passing a full URL intocsfd.userRatings('https://www.csfd.cz/uzivatel/admin/')caused the URL to be double-encoded insrc/vars.ts, resulting in a404error. This forces the developer to manually parse URLs.With this change, the
userRatingsanduserReviewsfunctions now behave smoothly and flexibly just likecsfd.movie(), vastly improving the developer experience.🚀 Examples:
Before:
After:
PR created automatically by Jules for task 6842156183414163731 started by @bartholomej
Summary by CodeRabbit
New Features
Bug Fixes