Skip to content

🛠️ DX: Support URL extraction for user APIs #209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bartholomej wants to merge 1 commit into
base: master
Choose a base branch
from
Open

🛠️ DX: Support URL extraction for user APIs #209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions src/helpers/global.helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,50 @@ export const extractId = (idOrUrl: number | string): number | null => {
return null;
};

/**
* Extracts a user ID or text slug from a number, string, slug, or full URL.
* Designed for Developer Experience (DX) to allow flexible inputs for user endpoints.
*/
export const extractUser = (userOrUrl: number | string): number | string | null => {
if (typeof userOrUrl === 'number') {
return isNaN(userOrUrl) ? null : userOrUrl;
}

if (typeof userOrUrl === 'string') {
// Pure number string
if (/^\d+$/.test(userOrUrl)) {
return Number(userOrUrl);
}

// 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;
Comment on lines +68 to +91

@coderabbitai coderabbitai Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP -C3 'export const userUrl' src/vars.ts

Repository: 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.

}

return null;
};

export const parseLastIdFromUrl = (url: string): number => {
if (url) {
const idSlug = url?.split('/')[3];
Expand Down
11 changes: 8 additions & 3 deletions src/services/user-ratings.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { HTMLElement, parse } from 'node-html-parser';
import { CSFDColorRating, CSFDFilmTypes, CSFDStars } from '../dto/global';
import { CSFDUserRatingConfig, CSFDUserRatings } from '../dto/user-ratings';
import { fetchPage } from '../fetchers';
import { sleep } from '../helpers/global.helper';
import { extractUser, sleep } from '../helpers/global.helper';
import {
getUserRating,
getUserRatingColorRating,
Expand All @@ -22,9 +22,14 @@ export class UserRatingsScraper {
config?: CSFDUserRatingConfig,
options?: CSFDOptions
): Promise<CSFDUserRatings[]> {
const userIdOrSlug = extractUser(user);
if (userIdOrSlug === null || (typeof userIdOrSlug === 'number' && isNaN(userIdOrSlug))) {
throw new Error('node-csfd-api: user must be a valid number, slug, or url');
}

let allMovies: CSFDUserRatings[] = [];
const pageToFetch = config?.page || 1;
const url = userRatingsUrl(user, pageToFetch > 1 ? pageToFetch : undefined, {
const url = userRatingsUrl(userIdOrSlug, pageToFetch > 1 ? pageToFetch : undefined, {
language: options?.language
});
const response = await fetchPage(url, { ...options?.request });
Expand All @@ -40,7 +45,7 @@ export class UserRatingsScraper {
if (config?.allPages) {
for (let i = 2; i <= pages; i++) {
config.onProgress?.(i, pages);
const url = userRatingsUrl(user, i, { language: options?.language });
const url = userRatingsUrl(userIdOrSlug, i, { language: options?.language });
const response = await fetchPage(url, { ...options?.request });

const items = parse(response);
Expand Down
11 changes: 8 additions & 3 deletions src/services/user-reviews.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { HTMLElement, parse } from 'node-html-parser';
import { CSFDColorRating, CSFDFilmTypes, CSFDStars } from '../dto/global';
import { CSFDUserReviews, CSFDUserReviewsConfig } from '../dto/user-reviews';
import { fetchPage } from '../fetchers';
import { sleep } from '../helpers/global.helper';
import { extractUser, sleep } from '../helpers/global.helper';
import {
getUserReviewColorRating,
getUserReviewDate,
Expand All @@ -24,9 +24,14 @@ export class UserReviewsScraper {
config?: CSFDUserReviewsConfig,
options?: CSFDOptions
): Promise<CSFDUserReviews[]> {
const userIdOrSlug = extractUser(user);
if (userIdOrSlug === null || (typeof userIdOrSlug === 'number' && isNaN(userIdOrSlug))) {
throw new Error('node-csfd-api: user must be a valid number, slug, or url');
}

let allReviews: CSFDUserReviews[] = [];
const pageToFetch = config?.page || 1;
const url = userReviewsUrl(user, pageToFetch > 1 ? pageToFetch : undefined, {
const url = userReviewsUrl(userIdOrSlug, pageToFetch > 1 ? pageToFetch : undefined, {
language: options?.language
});
const response = await fetchPage(url, { ...options?.request });
Expand All @@ -42,7 +47,7 @@ export class UserReviewsScraper {
if (config?.allPages) {
for (let i = 2; i <= pages; i++) {
config.onProgress?.(i, pages);
const url = userReviewsUrl(user, i, { language: options?.language });
const url = userReviewsUrl(userIdOrSlug, i, { language: options?.language });
const response = await fetchPage(url, { ...options?.request });

const items = parse(response);
Expand Down
23 changes: 22 additions & 1 deletion tests/helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, test } from 'vitest';
import { addProtocol, extractId, parseColor, parseIdFromUrl } from '../src/helpers/global.helper';
import { addProtocol, extractId, extractUser, parseColor, parseIdFromUrl } from '../src/helpers/global.helper';

describe('Add protocol', () => {
test('Handle without protocol', () => {
Expand Down Expand Up @@ -67,6 +67,27 @@ describe('extractId', () => {
});
});

describe('extractUser', () => {
test('should return number for numeric input', () => {
expect(extractUser(912)).toBe(912);
});
test('should return number for numeric string input', () => {
expect(extractUser('912')).toBe(912);
});
test('should return number for slug input', () => {
expect(extractUser('912-hobit')).toBe(912);
});
test('should return number for full url with numeric slug', () => {
expect(extractUser('https://www.csfd.cz/uzivatel/912-hobit/')).toBe(912);
});
test('should return string for text slug', () => {
expect(extractUser('admin')).toBe('admin');
});
test('should return string for full url with text slug', () => {
expect(extractUser('https://www.csfd.cz/uzivatel/admin/')).toBe('admin');
});
});

describe('Parse color', () => {
test('Red', () => {
const url = parseColor('red');
Expand Down