🛠️ DX: Add smart user URL parsing - #220
Conversation
…and full 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. |
📝 WalkthroughWalkthroughThe PR adds ChangesUser identifier normalization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 #220 +/- ##
==========================================
- Coverage 98.46% 98.21% -0.26%
==========================================
Files 39 39
Lines 1045 1065 +20
Branches 244 252 +8
==========================================
+ Hits 1029 1046 +17
- Misses 16 19 +3 ☔ 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
🤖 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 66-70: Update the URL parsing logic around the helper’s trimmed
URL handling to remove the query string and fragment before splitting or
searching for the uzivatel path segment. Ensure identifiers never include query
or fragment data and URLs whose query or fragment merely contains /uzivatel/ are
not accepted; add regression tests covering both cases.
🪄 Autofix
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 Plus
Run ID: 335e519a-1f8f-43fe-b64b-169d65e75ff6
📒 Files selected for processing (4)
src/helpers/global.helper.tssrc/services/user-ratings.service.tssrc/services/user-reviews.service.tstests/helpers.test.ts
| if (trimmed.includes('/') || trimmed.includes('csfd.')) { | ||
| const parts = trimmed.split('/'); | ||
| const uzivatelIndex = parts.indexOf('uzivatel'); | ||
| if (uzivatelIndex !== -1 && parts[uzivatelIndex + 1]) { | ||
| return parts[uzivatelIndex + 1]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Parse only the URL path.
Line 67 splits the complete URL. A query string or fragment can contain /uzivatel/ and be accepted as a user URL. For example, a film URL with ?next=/uzivatel/912-bart returns 912-bart.
A user URL with query parameters also returns an identifier that contains ?, which creates an invalid request URL. Strip the query and fragment before splitting. Add a regression test for both cases.
Proposed fix
- if (trimmed.includes('/') || trimmed.includes('csfd.')) {
- const parts = trimmed.split('/');
+ const path = trimmed.replace(/[?#].*$/, '');
+ if (path.includes('/') || path.includes('csfd.')) {
+ const parts = path.split('/');📝 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.
| if (trimmed.includes('/') || trimmed.includes('csfd.')) { | |
| const parts = trimmed.split('/'); | |
| const uzivatelIndex = parts.indexOf('uzivatel'); | |
| if (uzivatelIndex !== -1 && parts[uzivatelIndex + 1]) { | |
| return parts[uzivatelIndex + 1]; | |
| const path = trimmed.replace(/[?#].*$/, ''); | |
| if (path.includes('/') || path.includes('csfd.')) { | |
| const parts = path.split('/'); | |
| const uzivatelIndex = parts.indexOf('uzivatel'); | |
| if (uzivatelIndex !== -1 && parts[uzivatelIndex + 1]) { | |
| return parts[uzivatelIndex + 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 `@src/helpers/global.helper.ts` around lines 66 - 70, Update the URL parsing
logic around the helper’s trimmed URL handling to remove the query string and
fragment before splitting or searching for the uzivatel path segment. Ensure
identifiers never include query or fragment data and URLs whose query or
fragment merely contains /uzivatel/ are not accepted; add regression tests
covering both cases.
2 participants
💡 What: Introduced a new
extractUserhelper insideglobal.helper.tsto flexibly handle full ČSFD URLs, slugs, or numeric IDs for users. Integrated it seamlessly into theUserRatingsScraperandUserReviewsScraperservices.🎯 Why: Developers often have a full URL (e.g. from a scraper or a frontend input) but previously had to manually extract the exact slug or ID before passing it to the library. Now, the library magically extracts it, dramatically reducing boilerplate and providing a smoother Developer Experience.
🚀 Examples:
PR created automatically by Jules for task 9572647608027139898 started by @bartholomej
Summary by CodeRabbit