🛠️ DX: Support full URLs and raw IDs in parsing and API methods#195
🛠️ DX: Support full URLs and raw IDs in parsing and API methods#195bartholomej wants to merge 1 commit into
Conversation
💡 What
- Enhanced `parseIdFromUrl` to correctly extract IDs from both traditional slugs (`/uzivatel/912-bart/`) and raw IDs (`/uzivatel/912/`).
- Updated `userUrl` to handle raw URL strings, preventing double encoding when a full profile URL is passed.
- Refactored `userRatings` and `userReviews` methods to gracefully handle both raw IDs and full URLs by natively leveraging `extractId`.
🎯 Why
- Greatly improves Developer Experience (DX) by making API methods robust against various inputs (raw numbers, URL slugs, or fully qualified URLs).
- Prevents brittle failures when users pass a link copied directly from a browser.
🚀 Examples
```typescript
// Before
csfd.userRatings(912) // ✅ ok
csfd.userRatings('https://www.csfd.cz/uzivatel/912-bart/') // ❌ double encoded as /uzivatel/http...
// After
csfd.userRatings(912) // ✅ ok
csfd.userRatings('https://www.csfd.cz/uzivatel/912-bart/') // ✅ cleanly extracts 912
csfd.userRatings('https://www.csfd.cz/uzivatel/912/') // ✅ cleanly extracts 912
```
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 URL ID parsing to handle purely numeric path segments, refactors user-ratings and user-reviews services to extract and validate numeric user IDs with error handling, enhances the userUrl helper to pass through full HTTP URLs directly, re-exports DTO types from the main package index, and reformats CLI output and various code helpers for improved readability without functional changes. ChangesCode refactoring and API improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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/bin/export-reviews.tsOops! Something went wrong! :( ESLint: 10.4.0 TypeError [ERR_IMPORT_ATTRIBUTE_MISSING]: Module "file:///.eslintrc.json?mtime=1779963837916" needs an import attribute of "type: json" src/bin/lookup-movie.tsOops! Something went wrong! :( ESLint: 10.4.0 TypeError [ERR_IMPORT_ATTRIBUTE_MISSING]: Module "file:///.eslintrc.json?mtime=1779963837916" needs an import attribute of "type: json" src/bin/search.tsOops! Something went wrong! :( ESLint: 10.4.0 TypeError [ERR_IMPORT_ATTRIBUTE_MISSING]: Module "file:///.eslintrc.json?mtime=1779963837916" 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 #195 +/- ##
==========================================
- Coverage 98.71% 98.48% -0.24%
==========================================
Files 34 34
Lines 781 790 +9
Branches 202 206 +4
==========================================
+ Hits 771 778 +7
- 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: 0
🧹 Nitpick comments (1)
src/services/user-ratings.service.ts (1)
26-30: ⚡ Quick winExtract shared user-ID validation to a single helper.
The same normalization/throw logic is now duplicated here and in
src/services/user-reviews.service.ts. Centralizing it avoids drift in validation/error behavior.♻️ Proposed refactor
- 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 = resolveUserIdOrThrow(user);// src/helpers/global.helper.ts export const resolveUserIdOrThrow = (user: string | number): number => { const id = extractId(user); if (id === null || isNaN(id)) { throw new Error('node-csfd-api: user must be a valid number or url'); } return id; };🤖 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 26 - 30, Create a new helper function resolveUserIdOrThrow that wraps the existing extractId call and throws the same Error when id is null or NaN, returning the numeric id; then replace the duplicated validation block in both user-ratings.service (the code using extractId currently throwing 'node-csfd-api: user must be a valid number or url') and user-reviews.service to call resolveUserIdOrThrow instead; ensure the helper is exported and imported where needed and retains the exact error message and number return type so behavior stays identical to extractId + manual check.
🤖 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.
Nitpick comments:
In `@src/services/user-ratings.service.ts`:
- Around line 26-30: Create a new helper function resolveUserIdOrThrow that
wraps the existing extractId call and throws the same Error when id is null or
NaN, returning the numeric id; then replace the duplicated validation block in
both user-ratings.service (the code using extractId currently throwing
'node-csfd-api: user must be a valid number or url') and user-reviews.service to
call resolveUserIdOrThrow instead; ensure the helper is exported and imported
where needed and retains the exact error message and number return type so
behavior stays identical to extractId + manual check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16c346f6-4c61-4f2d-b78a-1f1d43589a85
📒 Files selected for processing (13)
src/bin/export-reviews.tssrc/bin/lookup-movie.tssrc/bin/search.tssrc/bin/utils.tssrc/dto/options.tssrc/helpers/global.helper.tssrc/helpers/search-user.helper.tssrc/index.tssrc/services/movie.service.tssrc/services/user-ratings.service.tssrc/services/user-reviews.service.tssrc/types.tssrc/vars.ts
💤 Files with no reviewable changes (1)
- src/index.ts
🛠️ DX: Support full URLs and raw IDs in parsing and API methods
💡 What
parseIdFromUrlto correctly extract IDs from both traditional slugs (/uzivatel/912-bart/) and raw IDs (/uzivatel/912/).userUrlto handle raw URL strings, preventing double encoding when a full profile URL is passed.userRatingsanduserReviewsmethods to gracefully handle both raw IDs and full URLs by natively leveragingextractId.🎯 Why
🚀 Examples
PR created automatically by Jules for task 11425765255038408731 started by @bartholomej
Summary by CodeRabbit
New Features
Bug Fixes
Refactor