π οΈ DX: Support full URLs for users and prevent double-encoding#200
π οΈ DX: Support full URLs for users and prevent double-encoding#200bartholomej wants to merge 1 commit into
Conversation
β¦ble-encoding - Enhance UserRatingsScraper and UserReviewsScraper to correctly extract numeric IDs from full CSFD URLs instead of breaking on double-encoding. - Update global `userUrl`, `movieUrl`, and `creatorUrl` builders in `vars.ts` to smartly ignore trailing paths and automatically extract the ID using `extractId`. - Adds tests in `tests/vars.test.ts` to assert that full URLs correctly return canonical endpoint URLs. 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 enhances ID handling across URL construction by extracting numeric IDs from various URL formats. The parser recognizes numeric-only path segments, core URL builders normalize user/movie/creator inputs via extractId before encoding, and user-ratings and user-reviews services leverage this normalization for consistent pagination URL construction. Tests validate the normalization behavior. ChangesURL ID Normalization Enhancement
Estimated code review effortπ― 2 (Simple) | β±οΈ ~10 minutes Possibly related PRs
Suggested labels
Poem
π₯ Pre-merge checks | β 4 | β 1β Failed checks (1 inconclusive)
β 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 Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #200 +/- ##
=======================================
Coverage 98.71% 98.72%
=======================================
Files 34 34
Lines 781 785 +4
Branches 202 206 +4
=======================================
+ Hits 771 775 +4
Misses 10 10 β 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.
π§Ή Nitpick comments (1)
tests/vars.test.ts (1)
13-16: β‘ Quick winAdd explicit path-only normalization test cases.
Coverage is strong for full URLs, but adding
/uzivatel/...,/film/..., and/tvurce/...path-only inputs here would better lock the normalization contract and prevent regressions.Suggested test additions
describe('Vars User Ratings', () => { + test('Assemble User rating with full path', () => { + const url = userRatingsUrl('/uzivatel/912-bart/hodnoceni/'); + expect(url).toBe('https://www.csfd.cz/uzivatel/912/hodnoceni/'); + }); }); describe('Vars Movies', () => { + test('Assemble movieUrl with full path', () => { + const url = movieUrl('/film/535121-zlatokopky/', {}); + expect(url).toBe('https://www.csfd.cz/film/535121/prehled/'); + }); }); describe('Vars Creator', () => { + test('Assemble creatorUrl with full path', () => { + const url = creatorUrl('/tvurce/1-mel-gibson/', {}); + expect(url).toBe('https://www.csfd.cz/tvurce/1'); + }); });π€ 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/vars.test.ts` around lines 13 - 16, Add explicit unit tests that pass path-only strings (e.g. '/uzivatel/912-bart', '/film/123-nazev', '/tvurce/45-jmeno') to the normalization functions to ensure they produce full normalized URLs; specifically add cases alongside the existing userRatingsUrl test that call userRatingsUrl with '/uzivatel/912-bart' and assert 'https://www.csfd.cz/uzivatel/912/hodnoceni/', and add equivalent path-only tests for the corresponding film and creator normalization functions (e.g. filmRatingsUrl and tvurce/creatorRatingsUrl) to assert their expected normalized outputs.
π€ 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 `@tests/vars.test.ts`:
- Around line 13-16: Add explicit unit tests that pass path-only strings (e.g.
'/uzivatel/912-bart', '/film/123-nazev', '/tvurce/45-jmeno') to the
normalization functions to ensure they produce full normalized URLs;
specifically add cases alongside the existing userRatingsUrl test that call
userRatingsUrl with '/uzivatel/912-bart' and assert
'https://www.csfd.cz/uzivatel/912/hodnoceni/', and add equivalent path-only
tests for the corresponding film and creator normalization functions (e.g.
filmRatingsUrl and tvurce/creatorRatingsUrl) to assert their expected normalized
outputs.
βΉοΈ Review info
βοΈ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 594b1c11-dd20-4f64-bc45-05598804638e
π Files selected for processing (6)
demo.tssrc/helpers/global.helper.tssrc/services/user-ratings.service.tssrc/services/user-reviews.service.tssrc/vars.tstests/vars.test.ts
π€ Files with no reviewable changes (1)
- demo.ts
2 participants
π‘ What: Added full URL parsing support across all primary CSFD scrapers and URL builders (
userRatings,userReviews,vars.ts).π― Why: Developers often copy-paste full HTTP URLs from their browsers directly into the API (e.g.
csfd.userRatings('https://www.csfd.cz/uzivatel/912-bart/hodnoceni/')). Previously, passing a full URL to User APIs triggered a 404 error because the URL builders blindly encoded the entire string. By systematically passing inputs through theextractIdhelper, the library now safely ignores trailing paths and returns canonical endpoints.π Examples:
PR created automatically by Jules for task 722844711197869297 started by @bartholomej
Summary by CodeRabbit
New Features
Tests