π οΈ DX: Support URL and text slug parsing in User APIs - #211
π οΈ DX: Support URL and text slug parsing in User APIs#211bartholomej wants to merge 1 commit into
Conversation
- Added `extractUser` to `global.helper.ts` to allow passing URLs or full slugs directly to user APIs. - Updated `userRatings` and `userReviews` to utilize `extractUser`. - Added tests to ensure complete backwards compatibility. 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. |
π WalkthroughWalkthroughAdds a new ChangesextractUser Helper and Service Integration
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant UserRatingsScraper as "UserRatingsScraper / UserReviewsScraper"
participant extractUser
Caller->>UserRatingsScraper: userRatings(user, config)
UserRatingsScraper->>extractUser: extractUser(user)
extractUser-->>UserRatingsScraper: extracted value (number/string/null)
alt invalid extracted value
UserRatingsScraper-->>Caller: throw "user must be a valid user ID or slug"
else valid extracted value
UserRatingsScraper->>UserRatingsScraper: build URL with extracted value
UserRatingsScraper-->>Caller: return scraped data
end
Related issues: None linked in the provided context. Suggested labels: enhancement, refactor Suggested reviewers: bartholomej π° A slug or a number, a URL too, π₯ Pre-merge checks | β 4 | β 1β Failed checks (1 warning)
β Passed checks (4 passed)
β¨ 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 #211 +/- ##
==========================================
- Coverage 98.71% 98.37% -0.35%
==========================================
Files 34 34
Lines 781 799 +18
Branches 202 208 +6
==========================================
+ Hits 771 786 +15
- Misses 10 13 +3 β View full report in Codecov by Harness. π New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
π§Ή Nitpick comments (1)
src/services/user-ratings.service.ts (1)
25-31: π Maintainability & Code Quality | π΅ Trivial | β‘ Quick winDuplicate validation logic across services.
This
extractUser+ null/undefined/empty validation + throw block is duplicated verbatim inuser-reviews.service.ts(Lines 27-33). Consider extracting a shared helper, e.g.getValidatedUser(user): string | numberinglobal.helper.ts, that both services call β keeping validation logic (helper concern) out of the orchestration layer.As per coding guidelines, "Services in
src/servicesshould act as orchestrators: call Fetchers, use Helpers to parse data, and return typed DTOs."π€ 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 - 31, Duplicate user validation exists in user-ratings.service.ts and user-reviews.service.ts, and the validation logic should be moved out of the service orchestration layer. Extract the repeated extractUser plus null/undefined/empty check and Error throw into a shared helper such as getValidatedUser(user) in global.helper.ts, then call that helper from both services so user-ratings.service and user-reviews.service only orchestrate fetch and return typed results.Source: Coding guidelines
π€ 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 57-78: Guard invalid numeric input in extractUser so NaN is not
propagated into URLs. Update the extractUser function to validate number inputs
the same way extractId does, returning null for invalid numbers instead of
passing NaN through, and make sure the string parsing path still returns numeric
IDs only when the trimmed value is a valid number while preserving the existing
/uzivatel/ extraction behavior.
---
Nitpick comments:
In `@src/services/user-ratings.service.ts`:
- Around line 25-31: Duplicate user validation exists in user-ratings.service.ts
and user-reviews.service.ts, and the validation logic should be moved out of the
service orchestration layer. Extract the repeated extractUser plus
null/undefined/empty check and Error throw into a shared helper such as
getValidatedUser(user) in global.helper.ts, then call that helper from both
services so user-ratings.service and user-reviews.service only orchestrate fetch
and return typed results.
πͺ 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: 1a3a88b2-2615-41bb-be73-9d746381cf6a
π Files selected for processing (4)
src/helpers/global.helper.tssrc/services/user-ratings.service.tssrc/services/user-reviews.service.tstests/helpers.test.ts
| export const extractUser = (userOrUrl: number | string): string | number => { | ||
| if (typeof userOrUrl === 'number') { | ||
| return userOrUrl; | ||
| } | ||
|
|
||
| if (typeof userOrUrl === 'string') { | ||
| const trimmed = userOrUrl.trim(); | ||
| if (/^\d+$/.test(trimmed)) { | ||
| return Number(trimmed); | ||
| } | ||
|
|
||
| // Check for URL containing /uzivatel/ | ||
| const match = trimmed.match(/\/uzivatel\/([^\/]+)/); | ||
| if (match) { | ||
| return match[1]; | ||
| } | ||
|
|
||
| return trimmed; | ||
| } | ||
|
|
||
| return userOrUrl; | ||
| }; |
There was a problem hiding this comment.
π― Functional Correctness | π‘ Minor | β‘ Quick win
π§© Analysis chain
π Script executed:
#!/bin/bash
# Check strictness settings that determine if TS2367 fires on this comparison
fd tsconfig.json --exec cat {}Repository: bartholomej/node-csfd-api
Length of output: 1072
π Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the helper and the two consuming services.
for f in src/helpers/global.helper.ts src/services/user-ratings.service.ts src/services/user-reviews.service.ts; do
echo "===== $f ====="
wc -l "$f"
cat -n "$f"
doneRepository: bartholomej/node-csfd-api
Length of output: 16414
π Script executed:
#!/bin/bash
set -euo pipefail
rg -n "export const (userRatingsUrl|userReviewsUrl)|function (userRatingsUrl|userReviewsUrl)|const (userRatingsUrl|userReviewsUrl)" srcRepository: bartholomej/node-csfd-api
Length of output: 399
π Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' src/vars.tsRepository: bartholomej/node-csfd-api
Length of output: 2173
π Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/vars.ts | sed -n '1,120p'Repository: bartholomej/node-csfd-api
Length of output: 2544
Guard NaN in extractUser
extractUser(NaN) currently returns NaN, so userRatingsUrl/userReviewsUrl build /uzivatel/NaN/... instead of rejecting the input. Mirror extractId here and return null for invalid numbers.
π€ 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 57 - 78, Guard invalid numeric
input in extractUser so NaN is not propagated into URLs. Update the extractUser
function to validate number inputs the same way extractId does, returning null
for invalid numbers instead of passing NaN through, and make sure the string
parsing path still returns numeric IDs only when the trimmed value is a valid
number while preserving the existing /uzivatel/ extraction behavior.
π‘ What: Added an
extractUserhelper function.π― Why: Developers can now seamlessly pass full URLs or text slugs into the user endpoints (
userRatingsanduserReviews). Previously, this either required exact slug inputs or failed on full profile URLs, causing annoying parsing boilerplate for users. This change brings user endpoints up to parity with themovieandcreatorendpoints which already supportextractId.π Examples:
PR created automatically by Jules for task 3725825361951973973 started by @bartholomej
Summary by CodeRabbit
New Features
Bug Fixes
Tests