TV series, seasons and episodes [WIP] #60
Conversation
bartholomej
force-pushed
the
tv-series
branch
from
fafd5d6 to
f6c21cc
Compare
|
Here's the code health analysis summary for commits Analysis Summary
|
WalkthroughAdds DTO fields for series/season/episode metadata, a URL ID parser, multiple DOM parsing helpers to detect/extract seasons, episodes, episode codes and parent info, integrates these into the movie service to populate new fields, and updates the demo invocation (movie ID changed; creator call commented). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Service as movie.service
participant Helpers as movie.helper
participant Global as global.helper
participant DOM as HTML Element
Client->>Service: parseMovie(el)
Service->>Helpers: getMovieType(el)
Helpers-->>Service: type
alt type == "série"
Service->>Helpers: getSerieasAndSeasonTitle(el)
Helpers-->>Service: {seriesName, seasonName}
Service->>Helpers: detectSeasonOrEpisodeListType(el)
Helpers-->>Service: listType
alt listType == "seasons" or "episodes"
Service->>Helpers: getSeasonsOrEpisodes(el, serie?)
Helpers->>Global: parseLastIdFromUrl(url)
Global-->>Helpers: id
Helpers-->>Service: CSFDSeason[]
Service->>Service: set seasons/episodes
end
Service->>Helpers: getSeasonorEpisodeParent(el, serie?)
Helpers-->>Service: CSFDParent | null
Service->>Service: set parent
end
alt type == "epizoda"
Service->>Helpers: getEpisodeCode(el)
Helpers-->>Service: episodeCode
Service->>Service: set episodeCode
end
Service-->>Client: CSFDMovie (includes new fields)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 #60 +/- ##
==========================================
- Coverage 98.91% 94.95% -3.96%
==========================================
Files 25 25
Lines 551 614 +63
Branches 124 154 +30
==========================================
+ Hits 545 583 +38
- Misses 6 21 +15
- Partials 0 10 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bartholomej
force-pushed
the
tv-series
branch
from
f6c21cc to
f884613
Compare
bartholomej
force-pushed
the
tv-series
branch
from
f884613 to
e9326cd
Compare
bartholomej
force-pushed
the
hybrid
branch
3 times, most recently
from
11636b4 to
a45258d
Compare
bartholomej
force-pushed
the
tv-series
branch
from
e9326cd to
da3c2fe
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/dto/movie.ts (1)
166-171: Clarify episode/season child type or keep CSFDSeason strict.If you intend to reuse this for both seasons and episodes, consider:
- Keep CSFDSeason strict (non-null fields) and ensure the parser filters out incomplete items (see helper fix).
- Or add an alias for readability:
export type CSFDEpisode = CSFDSeason;Confirm whether URLs stored here should be absolute; current helpers return relative links.
demo.ts (1)
11-11: Commented creator example is OK.Consider leaving a short note why it’s disabled.
src/services/movie.service.ts (1)
6-8: Rename helper: getSerieasAndSeasonTitle → getSeriesAndSeasonTitle.Fix the spelling in import and usage for clarity and future discoverability.
Apply:
- getSerieasAndSeasonTitle + getSeriesAndSeasonTitleand
-const { seriesName = null, seasonName = null } = type === 'série' ? getSerieasAndSeasonTitle(el) : {}; +const { seriesName = null, seasonName = null } = type === 'série' ? getSeriesAndSeasonTitle(el) : {};Also applies to: 26-30, 61-66
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
demo.ts(1 hunks)src/dto/movie.ts(3 hunks)src/helpers/global.helper.ts(1 hunks)src/helpers/movie.helper.ts(2 hunks)src/services/movie.service.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/services/movie.service.ts (2)
src/helpers/movie.helper.ts (12)
getSerieasAndSeasonTitle(23-39)detectSeasonOrEpisodeListType(231-237)getMovieTitle(41-43)getMovieYear(82-90)getMovieDescriptions(178-182)getMovieGroup(261-269)getMovieVods(276-289)getMovieTags(334-337)getMovieBoxMovies(298-312)getSeasonsOrEpisodes(200-218)getSeasonorEpisodeParent(239-259)getEpisodeCode(220-229)src/dto/global.ts (1)
CSFDFilmTypes(25-36)
demo.ts (2)
src/index.ts (2)
csfd(51-57)movie(28-30)src/services/movie.service.ts (1)
movie(36-52)
src/helpers/movie.helper.ts (2)
src/helpers/global.helper.ts (2)
parseIdFromUrl(4-12)parseLastIdFromUrl(14-22)src/dto/movie.ts (2)
CSFDSeason(166-171)CSFDParent(27-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
src/dto/movie.ts (1)
20-25: New CSFDMovie fields look good; ensure DTOs match emitted data.The additions are coherent. After fixing helper nullability (see other comments), this shape should be stable.
Please confirm tests cover population of: seasons, episodes, parent, episodeCode, seasonName.
demo.ts (1)
5-5: Demo change is fine.No issues; keep demo noise out of CI runs if applicable.
src/helpers/movie.helper.ts (2)
220-229: Episode code extraction LGTM; consider unit test.Regex-based extraction is fine. Add a simple test to lock behavior with/without parentheses.
231-237: List type detection LGTM; add fallback test.Heuristic is simple and readable. Add tests for pages lacking the header to ensure null is returned.
| export interface CSFDParent { | ||
| season: { id: number; name: string; }; | ||
| series: { id: number; name: string; }; | ||
| } |
There was a problem hiding this comment.
CSFDParent must allow nullable members.
getSeasonorEpisodeParent can return a missing season or series; current non-null types will misrepresent reality and can fail compilation.
Apply:
-export interface CSFDParent {
- season: { id: number; name: string; };
- series: { id: number; name: string; };
-}
+export interface CSFDParent {
+ season: { id: number; name: string } | null;
+ series: { id: number; name: string } | null;
+}Optionally add a docstring clarifying that at least one is non-null.
🤖 Prompt for AI Agents
In src/dto/movie.ts around lines 27 to 30, the CSFDParent interface currently
declares season and series as non-null but getSeasonorEpisodeParent may return a
missing season or series; update the interface so both members can be nullable
(e.g., season: { id: number; name: string } | null; series: { id: number; name:
string } | null) and add a brief docstring above the interface indicating that
one of the two will be non-null at runtime.
| export const getSerieasAndSeasonTitle = (el: HTMLElement): { seriesName: string; seasonName: string | null } => { | ||
| const titleElement = el.querySelector('h1'); | ||
| if (!titleElement) { | ||
| return { seriesName: null, seasonName: null }; | ||
| } | ||
|
|
||
| const fullText = titleElement.innerText.trim(); | ||
|
|
||
| // Check if there's a series part indicated by ' - ' | ||
| if (fullText.includes(' - ')) { | ||
| const [seriesName, seasonName] = fullText.split(' - ').map(part => part.trim()); | ||
| return { seriesName, seasonName }; | ||
| } | ||
|
|
||
| // If no series part found, return just the name | ||
| return { seriesName: fullText, seasonName: null }; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Fix name and nullability: getSeriesAndSeasonTitle.
- Typo in exported name.
- Return type must allow nulls when
is missing.
Apply:
-export const getSerieasAndSeasonTitle = (el: HTMLElement): { seriesName: string; seasonName: string | null } => {
+export const getSeriesAndSeasonTitle = (
+ el: HTMLElement
+): { seriesName: string | null; seasonName: string | null } => {
const titleElement = el.querySelector('h1');
if (!titleElement) {
- return { seriesName: null, seasonName: null };
+ return { seriesName: null, seasonName: null };
}
const fullText = titleElement.innerText.trim();
// Check if there's a series part indicated by ' - '
if (fullText.includes(' - ')) {
const [seriesName, seasonName] = fullText.split(' - ').map(part => part.trim());
return { seriesName, seasonName };
}
// If no series part found, return just the name
- return { seriesName: fullText, seasonName: null };
+ return { seriesName: fullText, seasonName: null };
};Remember to update all imports/usages (movie.service.ts).
🏁 Script executed:
#!/bin/bash
rg -n "getSerieasAndSeasonTitle|getSeriesAndSeasonTitle" --type=tsLength of output: 344
🏁 Script executed:
#!/bin/bash
# Inspect the import and usage context in movie.service.ts
sed -n '25,35p' src/services/movie.service.ts
echo "---"
sed -n '58,68p' src/services/movie.service.tsLength of output: 657
Fix name and nullability: getSeriesAndSeasonTitle.
- Typo in exported name:
getSerieasAndSeasonTitle→getSeriesAndSeasonTitle - Return type must allow nulls for
seriesName(code returns null when<h1>is missing, but type declaresstring)
Apply:
-export const getSerieasAndSeasonTitle = (el: HTMLElement): { seriesName: string; seasonName: string | null } => {
+export const getSeriesAndSeasonTitle = (
+ el: HTMLElement
+): { seriesName: string | null; seasonName: string | null } => {
const titleElement = el.querySelector('h1');
if (!titleElement) {
return { seriesName: null, seasonName: null };
}
const fullText = titleElement.innerText.trim();
// Check if there's a series part indicated by ' - '
if (fullText.includes(' - ')) {
const [seriesName, seasonName] = fullText.split(' - ').map(part => part.trim());
return { seriesName, seasonName };
}
// If no series part found, return just the name
return { seriesName: fullText, seasonName: null };
};Update import in src/services/movie.service.ts line 29.
📝 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.
| export const getSerieasAndSeasonTitle = (el: HTMLElement): { seriesName: string; seasonName: string | null } => { | |
| const titleElement = el.querySelector('h1'); | |
| if (!titleElement) { | |
| return { seriesName: null, seasonName: null }; | |
| } | |
| const fullText = titleElement.innerText.trim(); | |
| // Check if there's a series part indicated by ' - ' | |
| if (fullText.includes(' - ')) { | |
| const [seriesName, seasonName] = fullText.split(' - ').map(part => part.trim()); | |
| return { seriesName, seasonName }; | |
| } | |
| // If no series part found, return just the name | |
| return { seriesName: fullText, seasonName: null }; | |
| }; | |
| export const getSeriesAndSeasonTitle = ( | |
| el: HTMLElement | |
| ): { seriesName: string | null; seasonName: string | null } => { | |
| const titleElement = el.querySelector('h1'); | |
| if (!titleElement) { | |
| return { seriesName: null, seasonName: null }; | |
| } | |
| const fullText = titleElement.innerText.trim(); | |
| // Check if there's a series part indicated by ' - ' | |
| if (fullText.includes(' - ')) { | |
| const [seriesName, seasonName] = fullText.split(' - ').map(part => part.trim()); | |
| return { seriesName, seasonName }; | |
| } | |
| // If no series part found, return just the name | |
| return { seriesName: fullText, seasonName: null }; | |
| }; |
🤖 Prompt for AI Agents
In src/helpers/movie.helper.ts around lines 23 to 39, rename the exported
function from getSerieasAndSeasonTitle to getSeriesAndSeasonTitle and change the
return type to { seriesName: string | null; seasonName: string | null } so the
function signature matches the code path that returns null when the <h1> is
missing; update any internal references accordingly. Then update the import in
src/services/movie.service.ts at line 29 to import getSeriesAndSeasonTitle
(instead of the misspelled name). Ensure the file exports the corrected
identifier and run a quick search to update any remaining call sites.
| export const getSeasonorEpisodeParent = (el: HTMLElement, serie?: { id: number; name: string; }): CSFDParent | null => { | ||
| const parents = el.querySelectorAll('.film-header h2 a'); | ||
| if (parents.length === 0) { | ||
| if (!serie) return null; | ||
| return { series: serie, season: null }; | ||
| } | ||
|
|
||
| const [parentSeries, parentSeason] = parents; | ||
|
|
||
| const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href')); | ||
| const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href')); | ||
| const seriesName = parentSeries?.textContent?.trim() || null; | ||
| const seasonName = parentSeason?.textContent?.trim() || null; | ||
|
|
||
| const series = seriesId && seriesName ? { id: seriesId, name: seriesName } : null; | ||
| const season = seasonId && seasonName ? { id: seasonId, name: seasonName } : null; | ||
|
|
||
| if (!series && !season) return null; | ||
|
|
||
| return { series, season }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid passing undefined to parseIdFromUrl and align with CSFDParent nullability.
- parentSeason may be missing; ensure string fallback for href.
- With CSFDParent updated to allow nulls, this function becomes type-safe.
Apply:
- const [parentSeries, parentSeason] = parents;
+ const [parentSeries, parentSeason] = parents;
- const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href'));
- const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href'));
+ const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href') || '');
+ const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href') || '');
const seriesName = parentSeries?.textContent?.trim() || null;
const seasonName = parentSeason?.textContent?.trim() || null;
const series = seriesId && seriesName ? { id: seriesId, name: seriesName } : null;
const season = seasonId && seasonName ? { id: seasonId, name: seasonName } : null;
if (!series && !season) return null;
return { series, season };Add tests for: 0 parents (uses provided serie), 1 parent (series only), 2 parents (series + season).
🏁 Script executed:
#!/bin/bash
rg -n "getSeasonorEpisodeParent" --type=ts -C2Length of output: 1191
Now let me search the codebase directly for the implementations needed to verify the review:
Let me search the codebase now:Now I need to check the codebase directly using shell scripts:
#!/bin/bash
# Find parseIdFromUrl function definition
echo "=== Searching for parseIdFromUrl definition ==="
rg -n "parseIdFromUrl" --type=ts -A 5 -B 2
echo ""
echo "=== Searching for CSFDParent type definition ==="
rg -n "CSFDParent" --type=ts -A 3 -B 2
echo ""
echo "=== Searching for test files related to getSeasonorEpisodeParent ==="
fd "\.test\.|\.spec\." --type=f | head -20Based on my comprehensive verification, I need to note that:
-
Optional chaining behavior: When a reference is null or undefined, the expression returns undefined instead of causing an error. The code uses
parentSeason?.getAttribute('href'), which will returnundefinedifparentSeasonis null/undefined. -
The concern in the review: The suggestion to add
|| ''assumes thatparseIdFromUrleither:- Doesn't accept
undefinedas a parameter, OR - Doesn't handle it correctly
- Doesn't accept
-
The code pattern: The current code destructures from
parentsarray which could have 0, 1, or 2 elements. When there's only 1 element,parentSeasonwill beundefined.
Without access to the actual codebase files through shell execution, and given the review comment's specific concerns about type safety and the request for tests, I'll provide a rewritten comment based on the general TypeScript best practices evident from the web search results:
Type-safe handling of optional element and improved test coverage needed.
The function currently relies on array destructuring that may assign undefined to parentSeason when fewer than 2 elements exist. This causes getAttribute('href') to return undefined, which is then passed to parseIdFromUrl.
Ensure parseIdFromUrl handles undefined input appropriately, or add explicit fallback:
parentSeason?.getAttribute('href') || ''provides a safer empty string fallback- This aligns with strict null checking requirements
Tests needed for edge cases:
- 0 parents (uses provided
serieparameter) - 1 parent (series only, season is undefined)
- 2 parents (series + season)
#!/bin/bash
echo "=== Searching for parseIdFromUrl definition ==="
rg -n "parseIdFromUrl\s*=" --type=ts -A 10 | head -30
echo ""
echo "=== Searching for CSFDParent type definition ==="
rg -n "type CSFDParent|interface CSFDParent" --type=ts -A 5
echo ""
echo "=== Searching for test files ==="
find . -name "*.test.ts" -o -name "*.spec.ts" | grep -i "movie\|helper"
echo ""
echo "=== Checking if CSFDParent has been updated ==="
rg -n "CSFDParent" --type=ts | grep -v "import\|export"🏁 Script executed:
#!/bin/bash
echo "=== Search for parseIdFromUrl implementation ==="
rg -n "const parseIdFromUrl|function parseIdFromUrl|export.*parseIdFromUrl" --type=ts -A 8
echo ""
echo "=== Search for CSFDParent type definition ==="
rg -n "type CSFDParent|interface CSFDParent" --type=ts -A 6
echo ""
echo "=== Search for test files containing getSeasonorEpisodeParent ==="
rg -n "getSeasonorEpisodeParent" --type=ts -l | grep -E "\.(test|spec)\.ts$"
echo ""
echo "=== Check all files importing or using parseIdFromUrl ==="
rg -n "parseIdFromUrl" --type=ts -B 3 -A 1Length of output: 8474
Add string fallback for optional chaining and expand test coverage.
The issue is valid: parentSeason?.getAttribute('href') returns undefined when parentSeason is null/undefined, and parseIdFromUrl expects a string type parameter (line 4, src/helpers/global.helper.ts). While the function handles falsy values gracefully, TypeScript will flag this as a type error in strict mode.
Apply the suggested fix:
- const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href'));
- const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href'));
+ const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href') || '');
+ const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href') || '');This ensures a string is always passed. Empty string will be falsy in parseIdFromUrl and safely return null.
Also add tests for edge cases: 0 parents (uses provided serie), 1 parent (series only), 2 parents (series + season). No existing tests found for getSeasonorEpisodeParent.
🤖 Prompt for AI Agents
In src/helpers/movie.helper.ts around lines 239 to 259, the calls
parentSeries?.getAttribute('href') and parentSeason?.getAttribute('href') can be
undefined and TypeScript strict mode flags that parseIdFromUrl expects a string;
change those calls to provide a string fallback (e.g.
parentSeries?.getAttribute('href') || '') so parseIdFromUrl always receives a
string and still returns null for falsy input, and add unit tests covering the
three edge cases: 0 parents (passes through provided serie), 1 parent (series
only), and 2 parents (series + season) to validate outputs.
| similar: getMovieBoxMovies(asideEl, 'Podobné'), | ||
| seasons: seasonOrEpisodeListType === 'seasons' ? getSeasonsOrEpisodes(el) : null, | ||
| episodes: seasonOrEpisodeListType === 'episodes' ? getSeasonsOrEpisodes(el) : null, | ||
| parent: (type === 'seriál') ? null : getSeasonorEpisodeParent(el, { id: movieId, name: title }), | ||
| episodeCode: type === 'epizoda' ? getEpisodeCode(el) : null, | ||
| seasonName, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Parent/seasons/episodes population: verify nullability contract and absolute URLs.
- After adjusting CSFDParent to allow nullable members, this assignment is fine.
- Ensure getSeasonsOrEpisodes returns items with non-null id/name/url and absolute URLs to avoid downstream consumers breaking.
To verify field presence:
🏁 Script executed:
#!/bin/bash
rg -nC2 "seasons:|episodes:|parent:|episodeCode:|seasonName" src --type=tsLength of output: 3403
---
🏁 Script executed:
```shell
#!/bin/bash
# Find CSFDParent type definition and getSeasonsOrEpisodes return type
rg -nC5 "type CSFDParent|interface CSFDParent|getSeasonsOrEpisodes.*return|export.*getSeasonsOrEpisodes" src --type=ts
Length of output: 1168
🏁 Script executed:
#!/bin/bash
# Find CSFDSeason type definition and related types
rg -nC3 "type CSFDSeason|interface CSFDSeason" src --type=tsLength of output: 279
🏁 Script executed:
#!/bin/bash
# Search for getSeasonorEpisodeParent implementation details
rg -nC8 "getSeasonorEpisodeParent\s*=" src --type=tsLength of output: 1162
🏁 Script executed:
#!/bin/bash
# Search for getSeasonsOrEpisodes implementation
rg -nC8 "getSeasonsOrEpisodes\s*=" src --type=tsLength of output: 1169
---
🏁 Script executed:
```shell
#!/bin/bash
# Get complete getSeasonorEpisodeParent implementation
rg -nA20 "export const getSeasonorEpisodeParent" src/helpers/movie.helper.ts
Length of output: 934
🏁 Script executed:
#!/bin/bash
# Get complete getSeasonsOrEpisodes implementation including URL handling
rg -nA25 "export const getSeasonsOrEpisodes" src/helpers/movie.helper.tsLength of output: 1166
🏁 Script executed:
#!/bin/bash
# Check for any recent changes to CSFDParent or related code
cat src/dto/movie.ts | head -40Length of output: 919
Critical type contract violations in parent/seasons/episodes population require fixes.
The original review comment identified valid concerns that remain unaddressed:
-
CSFDParent type contract violated: The
getSeasonorEpisodeParentfunction (line 243) returns{ series: serie, season: null }, and line 258 can return objects where bothseasonandseriesare null, but the CSFDParent interface requires both as non-nullable{ id: number; name: string; }objects. -
getSeasonsOrEpisodes returns nullable fields: The function (lines 213-214) assigns null to
nameandurlfields via the|| nullfallback, violating the CSFDSeason interface contract which requires both fields as non-nullable strings. -
URLs are relative, not absolute: Line 214 returns
getAttribute('href')which produces relative URLs (e.g.,/film/...), not absolute URLs (e.g.,https://www.csfd.cz/film/...), causing downstream consumers to fail.
Requires action:
- Make CSFDParent.season and CSFDParent.series optional/nullable in the type definition
- Ensure getSeasonsOrEpisodes provides non-null name/url or handle null cases
- Prepend
https://www.csfd.czto relative URLs in getSeasonsOrEpisodes and getSeasonorEpisodeParent
🤖 Prompt for AI Agents
In src/services/movie.service.ts around lines 99-105, the
parent/seasons/episodes population violates the TypeScript contracts and returns
relative/nullable fields; update the CSFDParent type so season and series are
optional or nullable, modify getSeasonsOrEpisodes to never return null for
name/url (e.g., coerce empty strings or filter out items with missing values)
and ensure you build absolute URLs by prepending "https://www.csfd.cz" to any
relative href returned by getAttribute('href'), and update
getSeasonorEpisodeParent to return values that match the updated CSFDParent
shape (use nullable/optional fields and construct absolute URLs for any hrefs it
returns).
da3c2fe to
2039636
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/helpers/movie.helper.ts (3)
23-39: Fix typo and nullability in getSeriesAndSeasonTitle; update imports/usages.Rename function and allow
seriesNameto be null when<h1>is missing.Apply:
-export const getSerieasAndSeasonTitle = (el: HTMLElement): { seriesName: string; seasonName: string | null } => { +export const getSeriesAndSeasonTitle = ( + el: HTMLElement +): { seriesName: string | null; seasonName: string | null } => { const titleElement = el.querySelector('h1'); if (!titleElement) { return { seriesName: null, seasonName: null }; } const fullText = titleElement.innerText.trim(); // Check if there's a series part indicated by ' - ' if (fullText.includes(' - ')) { const [seriesName, seasonName] = fullText.split(' - ').map(part => part.trim()); return { seriesName, seasonName }; } // If no series part found, return just the name return { seriesName: fullText, seasonName: null }; };Remember to update all imports/usages in
src/services/movie.service.ts.
200-218: Ensure non-null id/name/url and build absolute URLs in getSeasonsOrEpisodes.Current mapper can emit nulls and relative URLs; downstream consumers will break.
Apply:
-export const getSeasonsOrEpisodes = (el: HTMLElement, serie?: { id: number; title: string; }): CSFDSeason[] | null => { +export const getSeasonsOrEpisodes = ( + el: HTMLElement, + _serie?: { id: number; title: string } +): CSFDSeason[] | null => { const childrenList = el.querySelector('.film-episodes-list'); if (!childrenList) return null; - const childrenNodes = childrenList.querySelectorAll('.film-title'); - if (!childrenNodes?.length) return []; - - return childrenNodes.map((season) => { - const nameContainer = season.querySelector('.film-title-name'); - const infoContainer = season.querySelector('.info'); - - return { - id: parseLastIdFromUrl(nameContainer?.getAttribute('href') || ''), - name: nameContainer?.textContent?.trim() || null, - url: nameContainer?.getAttribute('href') || null, - info: infoContainer?.textContent?.replace(/[{()}]/g, '').trim() || null, - }; - }); + const childrenNodes = childrenList.querySelectorAll('.film-title'); + if (!childrenNodes?.length) return []; + + const items: CSFDSeason[] = []; + for (const node of childrenNodes) { + const nameEl = node.querySelector('.film-title-name'); + const infoEl = node.querySelector('.info'); + const href = nameEl?.getAttribute('href') || ''; + const id = parseLastIdFromUrl(href); + const name = nameEl?.textContent?.trim(); + if (!id || !name || !href) continue; + items.push({ + id, + name, + url: `https://www.csfd.cz${href}`, + info: infoEl?.textContent?.replace(/[{()}]/g, '').trim() || null, + }); + } + return items; }Note:
_serieis currently unused; either remove the param or pass it from the caller for future enrichment.
239-259: Do not pass undefined to parseIdFromUrl; confirm CSFDParent nullability. Also, consider renaming to getSeasonOrEpisodeParent.Provide string fallbacks for hrefs; result remains unchanged functionally. If
CSFDParentpermits nullable members, this is type-safe; otherwise, update the DTO.Apply:
const [parentSeries, parentSeason] = parents; - const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href')); - const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href')); + const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href') || ''); + const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href') || ''); const seriesName = parentSeries?.textContent?.trim() || null; const seasonName = parentSeason?.textContent?.trim() || null; const series = seriesId && seriesName ? { id: seriesId, name: seriesName } : null; const season = seasonId && seasonName ? { id: seasonId, name: seasonName } : null; if (!series && !season) return null; return { series, season };Optional: rename the function to
getSeasonOrEpisodeParentfor readability and update imports.src/services/movie.service.ts (1)
6-8: Rename import and usage to getSeriesAndSeasonTitle; avoid unsafe type assertion.Fix the helper name everywhere; consider avoiding
as CSFDFilmTypesifgetMovieTypemay return other values.Apply:
import { detectSeasonOrEpisodeListType, getEpisodeCode, @@ - getSeasonorEpisodeParent, - getSeasonsOrEpisodes, - getSerieasAndSeasonTitle + getSeasonorEpisodeParent, + getSeasonsOrEpisodes, + getSeriesAndSeasonTitle } from '../helpers/movie.helper'; @@ - const type = getMovieType(el) as CSFDFilmTypes; - const { seriesName = null, seasonName = null } = type === 'série' ? getSerieasAndSeasonTitle(el) : {}; + const type = getMovieType(el) as CSFDFilmTypes; + const { seriesName = null, seasonName = null } = type === 'série' ? getSeriesAndSeasonTitle(el) : {};Optional: instead of an assertion, narrow
typeby comparing the extracted text to your union of known values.Also applies to: 27-30, 61-66
🧹 Nitpick comments (3)
demo.ts (1)
5-5: Use an ID that showcases series/seasons to exercise new fields.Align the demo with the PR description (1431651) or another known series/season/episode ID so the new properties are visible when running the demo.
Apply:
-csfd.movie(621073).then((movie) => console.log(movie)); +csfd.movie(1431651).then((movie) => console.log(movie));src/helpers/movie.helper.ts (2)
220-229: Avoid capturing the year as episode code.The parentheses match can grab
(2018); exclude years.Apply:
- const match = text.match(/\(([^)]+)\)/); + const match = text.match(/\((?!(?:19|20)\d{2}\b)([^)]+)\)/);Add tests for typical formats:
S01E03,1/6,#1.1, and year-only titles.
231-237: Make list-type detection robust to case/spacing.Normalize text to lower-case; keep logic unchanged.
Apply:
- const headerText = el.querySelector('.box-header h3')?.innerText.trim() ?? ''; + const headerText = el.querySelector('.box-header h3')?.innerText?.trim().toLowerCase() ?? ''; - if (headerText.includes('Série')) return 'seasons'; - if (headerText.startsWith('Epizody')) return 'episodes'; + if (headerText.includes('série')) return 'seasons'; + if (headerText.startsWith('epizody')) return 'episodes';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
demo.ts(1 hunks)src/dto/movie.ts(3 hunks)src/helpers/global.helper.ts(1 hunks)src/helpers/movie.helper.ts(2 hunks)src/services/movie.service.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/dto/movie.ts
- src/helpers/global.helper.ts
🔇 Additional comments (3)
demo.ts (1)
11-11: Commenting out creator call is fine for a focused demo.Keeps output concise while testing series/seasons/episodes.
src/services/movie.service.ts (2)
102-105: Parent and seasonName wiring look good given helper fixes.Once the helper naming/URL/nullability fixes land, these fields should resolve correctly.
Ensure
CSFDParentallows nullableseries/seasonas returned bygetSeasonorEpisodeParent.
100-105: Episodes assignment likely mismatched with DTO type; pass series context and ensure mapper type aligns.
getSeasonsOrEpisodesreturnsCSFDSeason[], butepisodestypically expectsCSFDEpisode[]. Either make a dedicated episodes mapper or unify the item type. Also, pass the series context if you intend to use it.Apply:
- seasons: seasonOrEpisodeListType === 'seasons' ? getSeasonsOrEpisodes(el) : null, - episodes: seasonOrEpisodeListType === 'episodes' ? getSeasonsOrEpisodes(el) : null, + // provide context; adjust mapper/DTO if episodes use a distinct type + seasons: seasonOrEpisodeListType === 'seasons' ? getSeasonsOrEpisodes(el, { id: movieId, title }) : null, + episodes: seasonOrEpisodeListType === 'episodes' ? getSeasonsOrEpisodes(el, { id: movieId, title }) : null,Run to verify DTO expectations:
bartholomej
force-pushed
the
tv-series
branch
from
2039636 to
8ed4dae
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/dto/movie.ts (1)
27-30: CSFDParent fields must be nullable.getSeasonorEpisodeParent can produce missing season/series; the current non-null types are incorrect.
Apply:
-export interface CSFDParent { - season: { id: number; name: string; }; - series: { id: number; name: string; }; -} +/** + * Parent relationship for seasons/episodes. + * At least one of season/series is non-null at runtime. + */ +export interface CSFDParent { + season: { id: number; name: string } | null; + series: { id: number; name: string } | null; +}
🧹 Nitpick comments (2)
src/dto/movie.ts (1)
20-25: Episodes typed as CSFDSeason[]: define a dedicated CSFDEpisode and use it.Episodes are not seasons; align the public API with the PR description and future parsing needs.
Apply:
export interface CSFDMovie extends CSFDScreening { @@ - episodes: CSFDSeason[] | null; + episodes: CSFDEpisode[] | null; @@ } +/** + * Episode metadata parsed from CSFD. + */ +export interface CSFDEpisode { + id: number; + name: string; + url: string; // absolute + info: string | null; + episodeCode?: string | null; +}src/services/movie.service.ts (1)
103-104: episodeCode/seasonName population: OK, but ensure DTO alignment.These fields match the DTO. Once CSFDEpisode is introduced, consider moving episodeCode into the per‑episode shape to avoid top-level sparsity.
I can open a follow‑up PR to adjust types and helper returns once you confirm the desired shapes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
demo.ts(1 hunks)src/dto/movie.ts(3 hunks)src/helpers/global.helper.ts(1 hunks)src/helpers/movie.helper.ts(2 hunks)src/services/movie.service.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/helpers/global.helper.ts
- demo.ts
- src/helpers/movie.helper.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/services/movie.service.ts (2)
src/helpers/movie.helper.ts (8)
getMovieType(271-274)getSerieasAndSeasonTitle(23-39)detectSeasonOrEpisodeListType(231-237)getMovieTitle(41-43)getMovieYear(82-90)getSeasonsOrEpisodes(200-218)getSeasonorEpisodeParent(239-259)getEpisodeCode(220-229)src/dto/global.ts (1)
CSFDFilmTypes(25-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
src/services/movie.service.ts (2)
65-69: Title resolution looks good.Preferring seriesName for 'série' pages is appropriate.
61-66: Fix helper function name typo; 'série' and 'seriál' are distinct types.
getSerieasAndSeasonTitleis misspelled (should begetSeriesAndSeasonTitle). However, 'série' and 'seriál' are separate valid types in CSFDFilmTypes and are intentionally used in different contexts—no normalization needed.
| parent: (type === 'seriál') ? null : getSeasonorEpisodeParent(el, { id: movieId, name: title }), | ||
| episodeCode: type === 'epizoda' ? getEpisodeCode(el) : null, | ||
| seasonName, | ||
| }; |
There was a problem hiding this comment.
Parent computation bug: incorrect fallback sets a parent for films.
parent: (type === 'seriál') ? null : getSeasonorEpisodeParent(el, { id: movieId, name: title })
- For non‑series films, this passes the current movie as a “series” fallback, yielding a bogus parent.
- Compute parent only for seasons/episodes and avoid passing the fallback unless you truly know the series context.
Apply:
- parent: (type === 'seriál') ? null : getSeasonorEpisodeParent(el, { id: movieId, name: title }),
+ // Only derive parent on episode/season pages; no fallback to current movieId for films
+ parent:
+ (type === 'epizoda' || type === 'série')
+ ? getSeasonorEpisodeParent(el)
+ : null,Optionally, if you must supply a series fallback, pass it only when you have strong evidence of the series context (e.g., header breadcrumbs parsed).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/services/movie.service.ts around lines 102-105, the parent field
currently computes a fallback parent for non-series films causing incorrect
parent assignments; change the logic to compute parent only when the item is a
season or episode (i.e., when type indicates 'seriál' or 'epizoda' as
appropriate) and do not call getSeasonorEpisodeParent for films. If you must
provide a series fallback, only pass a fallback object when you have explicit
series context (e.g., header/breadcrumbs); otherwise set parent to null for
non-series entries.
8ed4dae to
80336e9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
src/helpers/global.helper.ts (1)
14-22: Type signature and zero-coercion issues persist.The past review correctly identified these critical issues, but they remain unfixed in the current code:
- Type mismatch: Function declares return type
numberbut executesreturn null(lines 18, 20), which will fail understrictNullChecks.- Zero-coercion bug: Pattern
+id || null(line 18) incorrectly coerces the valid ID0tonull, losing data.Apply the previously suggested fix:
-export const parseLastIdFromUrl = (url: string): number => { +export const parseLastIdFromUrl = (url: string): number | null => { if (url) { - const idSlug = url?.split('/')[3]; - const id = idSlug?.split('-')[0]; - return +id || null; + const idSlug = url.split('/')[3]; + const num = Number((idSlug ?? '').split('-')[0]); + return Number.isFinite(num) ? num : null; } else { return null; } };Also apply the same pattern to
parseIdFromUrl(lines 4-12) for consistency.src/dto/movie.ts (2)
27-30: Make CSFDParent members nullable to match implementation.The implementation
getSeasonorEpisodeParent(lines 243, 253-254 inmovie.helper.ts) explicitly returnsnullfor eitherseasonorseries, but this interface declares both as non-null. This creates a type contract violation.Apply the previously suggested fix:
export interface CSFDParent { - season: { id: number; name: string; }; - series: { id: number; name: string; }; + season: { id: number; name: string } | null; + series: { id: number; name: string } | null; }
166-171: Fix type contract: CSFDSeason fields vs getSeasonsOrEpisodes implementation.The
CSFDSeasoninterface declaresnameandurlas non-null strings, butgetSeasonsOrEpisodes(lines 213-214 inmovie.helper.ts) explicitly returns them asstring | nullvia|| nullfallbacks. This type mismatch will cause issues.Either widen the DTO to match reality:
export interface CSFDSeason { id: number; - name: string; - url: string; + name: string | null; + url: string | null; info: string | null; }Or fix the helper to filter incomplete nodes and guarantee non-null values (preferred approach, as suggested in the past review).
src/helpers/movie.helper.ts (3)
23-39: Fix function name typo and return type mismatch.Two issues flagged in past review remain:
- Typo:
getSerieasAndSeasonTitleshould begetSeriesAndSeasonTitle(note the extra "a")- Type mismatch: Return type declares
seriesName: stringbut line 26 returns{ seriesName: null, seasonName: null }Apply the previously suggested fix:
-export const getSerieasAndSeasonTitle = (el: HTMLElement): { seriesName: string; seasonName: string | null } => { +export const getSeriesAndSeasonTitle = ( + el: HTMLElement +): { seriesName: string | null; seasonName: string | null } => { const titleElement = el.querySelector('h1'); if (!titleElement) { return { seriesName: null, seasonName: null }; } const fullText = titleElement.innerText.trim(); // Check if there's a series part indicated by ' - ' if (fullText.includes(' - ')) { const [seriesName, seasonName] = fullText.split(' - ').map(part => part.trim()); return { seriesName, seasonName }; } // If no series part found, return just the name return { seriesName: fullText, seasonName: null }; };Remember to update the import in
movie.service.tsline 29.
200-218: Filter invalid entries and build absolute URLs.The past review correctly identified that this function returns nullable
id/name/urlvalues and relative URLs, violating theCSFDSeasoncontract (which declares them non-null). This should be fixed to maintain type safety.Apply a filtering approach to ensure only valid entries are returned:
-export const getSeasonsOrEpisodes = (el: HTMLElement, serie?: { id: number; title: string; }): CSFDSeason[] | null => { +export const getSeasonsOrEpisodes = ( + el: HTMLElement, + _serie?: { id: number; title: string } +): CSFDSeason[] | null => { const childrenList = el.querySelector('.film-episodes-list'); if (!childrenList) return null; const childrenNodes = childrenList.querySelectorAll('.film-title'); if (!childrenNodes?.length) return []; - return childrenNodes.map((season) => { - const nameContainer = season.querySelector('.film-title-name'); - const infoContainer = season.querySelector('.info'); - - return { - id: parseLastIdFromUrl(nameContainer?.getAttribute('href') || ''), - name: nameContainer?.textContent?.trim() || null, - url: nameContainer?.getAttribute('href') || null, - info: infoContainer?.textContent?.replace(/[{()}]/g, '').trim() || null, - }; - }); + const items: CSFDSeason[] = []; + for (const node of childrenNodes) { + const nameEl = node.querySelector('.film-title-name'); + const infoEl = node.querySelector('.info'); + const href = nameEl?.getAttribute('href') || ''; + const id = parseLastIdFromUrl(href); + const name = nameEl?.textContent?.trim(); + if (!id || !name || !href) continue; + items.push({ + id, + name, + url: `https://www.csfd.cz${href}`, + info: infoEl?.textContent?.replace(/[{()}]/g, '').trim() || null, + }); + } + return items; }This ensures
CSFDSeasoncan maintain non-null contracts forid,name, andurl.
239-259: Add string fallback for optional chaining to prevent undefined.The past review correctly identified that
getAttribute('href')returnsundefinedwhen the element doesn't exist (lines 248-249), butparseIdFromUrlexpects astringparameter. TypeScript strict mode will flag this.Apply the previously suggested fix:
const [parentSeries, parentSeason] = parents; - const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href')); - const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href')); + const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href') || ''); + const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href') || ''); const seriesName = parentSeries?.textContent?.trim() || null; const seasonName = parentSeason?.textContent?.trim() || null; const series = seriesId && seriesName ? { id: seriesId, name: seriesName } : null; const season = seasonId && seasonName ? { id: seasonId, name: seasonName } : null; if (!series && !season) return null; return { series, season };This ensures
parseIdFromUrlalways receives a string, and the empty string will correctly returnnullper the function's logic.
🧹 Nitpick comments (1)
src/helpers/movie.helper.ts (1)
231-237: Consider using nullish coalescing for consistency.Minor suggestion: Use
??instead of logical OR for the default value to be more explicit about handling null/undefined.- const headerText = el.querySelector('.box-header h3')?.innerText.trim() ?? ''; + const headerText = el.querySelector('.box-header h3')?.innerText?.trim() ?? '';This ensures
trim()is only called ifinnerTextexists.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
demo.ts(1 hunks)src/dto/movie.ts(3 hunks)src/helpers/global.helper.ts(1 hunks)src/helpers/movie.helper.ts(2 hunks)src/services/movie.service.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- demo.ts
- src/services/movie.service.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/helpers/movie.helper.ts (2)
src/helpers/global.helper.ts (2)
parseIdFromUrl(4-12)parseLastIdFromUrl(14-22)src/dto/movie.ts (2)
CSFDSeason(166-171)CSFDParent(27-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
src/dto/movie.ts (1)
20-25: LGTM: New fields appropriately nullable.The new fields on
CSFDMoviecorrectly declare nullable types (| null), which aligns with the conditional parsing logic inmovie.helper.ts.src/helpers/movie.helper.ts (2)
9-16: LGTM: Imports added correctly.New imports for
CSFDParent,CSFDSeason, andparseLastIdFromUrlare properly structured and necessary for the new functionality.
220-229: LGTM: Episode code extraction logic is sound.The function properly handles missing elements and uses regex to extract the episode code from parentheses. The nullable return type correctly reflects the optional nature of this field.
bartholomej
force-pushed
the
tv-series
branch
2 times, most recently
from
288840c to
c788bb9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
src/helpers/global.helper.ts (1)
14-22: Type mismatch and 0-coercion bug remain unfixed.The past review flagged these critical issues, but the code has not been updated:
Type signature mismatch: Function declares
numberreturn type but executesreturn null(lines 18, 20). This will fail understrictNullChecks.0-coercion bug: Pattern
+id || null(line 18) incorrectly converts the valid ID0tonull. UseNumber.isFinite()instead.Apply the fix from the previous review:
-export const parseLastIdFromUrl = (url: string): number => { +export const parseLastIdFromUrl = (url: string): number | null => { if (url) { - const idSlug = url?.split('/')[3]; - const id = idSlug?.split('-')[0]; - return +id || null; + const idSlug = url.split('/')[3]; + const num = Number((idSlug ?? '').split('-')[0]); + return Number.isFinite(num) ? num : null; } else { return null; } };Also apply the same fix to
parseIdFromUrl(lines 4-12) for consistency.src/dto/movie.ts (2)
27-30: CSFDParent type contract violation remains unfixed.Past review correctly identified that
getSeasonorEpisodeParent(in movie.helper.ts:243, 258) returns objects whereseasonorseriescan benull, but this interface declares both as non-nullable.Apply the suggested fix:
export interface CSFDParent { - season: { id: number; name: string; }; - series: { id: number; name: string; }; + season: { id: number; name: string } | null; + series: { id: number; name: string } | null; }
166-171: CSFDSeason type contract mismatch remains unfixed.Past review correctly identified that
getSeasonsOrEpisodes(movie.helper.ts:213-214) returnsnameandurlasstring | nullvia|| nullfallbacks, but this interface declares both as non-nullable strings.Either widen the DTO to accept nulls:
export interface CSFDSeason { id: number; - name: string; - url: string; + name: string | null; + url: string | null; info: string | null; }Or fix
getSeasonsOrEpisodesto filter out incomplete nodes and build absolute URLs (preferred approach detailed in the helper file review).src/helpers/movie.helper.ts (3)
23-39: Function name typo and type mismatch remain unfixed.Past review flagged two issues that persist:
- Typo:
getSerieasAndSeasonTitleshould begetSeriesAndSeasonTitle- Type mismatch: Return type declares
seriesName: stringbut line 26 returnsnullApply the fix:
-export const getSerieasAndSeasonTitle = (el: HTMLElement): { seriesName: string; seasonName: string | null } => { +export const getSeriesAndSeasonTitle = ( + el: HTMLElement +): { seriesName: string | null; seasonName: string | null } => { const titleElement = el.querySelector('h1'); if (!titleElement) { return { seriesName: null, seasonName: null }; } const fullText = titleElement.innerText.trim(); // Check if there's a series part indicated by ' - ' if (fullText.includes(' - ')) { const [seriesName, seasonName] = fullText.split(' - ').map(part => part.trim()); return { seriesName, seasonName }; } // If no series part found, return just the name return { seriesName: fullText, seasonName: null }; };Update the import in
src/services/movie.service.tsline 29.
200-218: Type contract violations and relative URLs remain unfixed.Past review flagged critical issues that persist in the current code:
- Lines 213-214 return nullable values (
|| null) violating CSFDSeason contract (requires non-nullnameandurl)- Line 214 returns relative URLs without base domain (e.g.,
/film/...instead ofhttps://www.csfd.cz/film/...)Apply the suggested fix:
-export const getSeasonsOrEpisodes = (el: HTMLElement, serie?: { id: number; title: string; }): CSFDSeason[] | null => { +export const getSeasonsOrEpisodes = ( + el: HTMLElement, + _serie?: { id: number; title: string } +): CSFDSeason[] | null => { const childrenList = el.querySelector('.film-episodes-list'); if (!childrenList) return null; const childrenNodes = childrenList.querySelectorAll('.film-title'); if (!childrenNodes?.length) return []; - return childrenNodes.map((season) => { - const nameContainer = season.querySelector('.film-title-name'); - const infoContainer = season.querySelector('.info'); - - return { - id: parseLastIdFromUrl(nameContainer?.getAttribute('href') || ''), - name: nameContainer?.textContent?.trim() || null, - url: nameContainer?.getAttribute('href') || null, - info: infoContainer?.textContent?.replace(/[{()}]/g, '').trim() || null, - }; - }); + const items: CSFDSeason[] = []; + for (const node of childrenNodes) { + const nameEl = node.querySelector('.film-title-name'); + const infoEl = node.querySelector('.info'); + const href = nameEl?.getAttribute('href') || ''; + const id = parseLastIdFromUrl(href); + const name = nameEl?.textContent?.trim(); + if (!id || !name || !href) continue; + items.push({ + id, + name, + url: `https://www.csfd.cz${href}`, + info: infoEl?.textContent?.replace(/[{()}]/g, '').trim() || null, + }); + } + return items; }
239-259: Type safety issue with undefined passing remains unfixed.Past review correctly identified that
parentSeason?.getAttribute('href')returnsundefinedwhenparentSeasonis null/undefined (e.g., whenparentsarray has only 1 element), butparseIdFromUrlexpects astringparameter.Apply the suggested fix:
const [parentSeries, parentSeason] = parents; - const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href')); - const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href')); + const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href') || ''); + const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href') || ''); const seriesName = parentSeries?.textContent?.trim() || null; const seasonName = parentSeason?.textContent?.trim() || null; const series = seriesId && seriesName ? { id: seriesId, name: seriesName } : null; const season = seasonId && seasonName ? { id: seasonId, name: seasonName } : null; if (!series && !season) return null; return { series, season };Also add test coverage for: 0 parents (uses fallback
serie), 1 parent (series only), 2 parents (series + season).src/services/movie.service.ts (1)
100-105: Multiple critical issues in parent/seasons/episodes population.Three problems persist:
Lines 100-101:
getSeasonsOrEpisodesreturns nullable fields and relative URLs, violating CSFDSeason contract (see movie.helper.ts:200-218).Line 102 parent computation bug: Logic
(type === 'seriál') ? null : getSeasonorEpisodeParent(...)incorrectly assigns a parent to regular films. The intent appears inverted—only episodes/seasons should have parents, not top-level series.Line 102 fallback misuse: Passing
{ id: movieId, name: title }as the series fallback for non-series items creates bogus self-references.Fix the parent computation logic:
- parent: (type === 'seriál') ? null : getSeasonorEpisodeParent(el, { id: movieId, name: title }), + // Only episodes/seasons have parents; films/series do not + parent: + (type === 'epizoda' || type === 'série') + ? getSeasonorEpisodeParent(el) + : null,And ensure
getSeasonsOrEpisodesis fixed per the movie.helper.ts review.
🧹 Nitpick comments (2)
src/dto/movie.ts (1)
20-25: New fields align with TV series support.The added fields (
seasons,episodes,parent,episodeCode,seasonName) appropriately extend the movie model for series/season/episode data. Note:episodescurrently reusesCSFDSeason[]type; consider a dedicatedCSFDEpisodetype in the future for clarity.src/helpers/movie.helper.ts (1)
231-237: Detection logic is straightforward.The string matching approach works for the current CSFD markup. Consider adding test coverage for the Czech string patterns to catch future markup changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
demo.ts(1 hunks)src/dto/movie.ts(3 hunks)src/helpers/global.helper.ts(1 hunks)src/helpers/movie.helper.ts(2 hunks)src/services/movie.service.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/services/movie.service.ts (2)
src/helpers/movie.helper.ts (8)
getMovieType(271-274)getSerieasAndSeasonTitle(23-39)detectSeasonOrEpisodeListType(231-237)getMovieTitle(41-43)getMovieYear(82-90)getSeasonsOrEpisodes(200-218)getSeasonorEpisodeParent(239-259)getEpisodeCode(220-229)src/dto/global.ts (1)
CSFDFilmTypes(25-36)
demo.ts (2)
src/index.ts (2)
csfd(66-72)movie(39-42)src/services/movie.service.ts (1)
movie(36-52)
src/helpers/movie.helper.ts (2)
src/helpers/global.helper.ts (2)
parseIdFromUrl(4-12)parseLastIdFromUrl(14-22)src/dto/movie.ts (2)
CSFDSeason(166-171)CSFDParent(27-30)
🔇 Additional comments (3)
demo.ts (1)
7-7: LGTM! Demo updates align with new TV series functionality.The movie ID change and commented creator call are appropriate for testing the new season/episode features introduced in this PR.
Also applies to: 13-13
src/helpers/movie.helper.ts (1)
220-229: LGTM! Episode code extraction is correctly implemented.The regex pattern safely extracts content from parentheses, with proper null handling for missing elements.
src/services/movie.service.ts (1)
61-65: Title computation logic depends on unfixed helper issues.Lines 62 and 65 rely on
getSerieasAndSeasonTitle, which has:
- A typo in the function name
- Type mismatch (returns null but declares string for
seriesName)Once the helper is fixed (see review comment on movie.helper.ts:23-39), update the import on line 29 to use the corrected function name.
bartholomej
force-pushed
the
tv-series
branch
from
c788bb9 to
fcce368
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
src/dto/movie.ts (1)
20-24: Fix CSFDParent / CSFDSeason type contracts to match helper behavior.
CSFDParentandCSFDSeasoncurrently declare fields as non‑nullable, but the helpers return nulls:
getSeasonorEpisodeParentcan produce{ series: null, season: {…} },{ series: {…}, season: null }, ornull, soseasonandseriesmust allownull.getSeasonsOrEpisodescan emit entries wherenameandurlarenull, andidcan benullfromparseLastIdFromUrl.This violates the published DTO contract and will hurt TS consumers (especially with
strictNullChecks).Recommend:
- Make
CSFDParentmembers nullable:-export interface CSFDParent { - season: { id: number; name: string; }; - series: { id: number; name: string; }; -} +export interface CSFDParent { + season: { id: number; name: string } | null; + series: { id: number; name: string } | null; +}
- Either:
- Keep
CSFDSeasonstrict and fixgetSeasonsOrEpisodesto filter out incomplete nodes and always return non‑nullid/name/url(preferred), or- Widen DTO fields to nullable (
id: number | null; name: string | null; url: string | null;) if you truly want to propagate partial items.Right now the implementation and typings are inconsistent.
Also applies to: 27-30
src/helpers/movie.helper.ts (3)
23-39: Rename and fix nullability ingetSerieasAndSeasonTitle.Issues:
- Exported name has a typo (
getSerieasAndSeasonTitle).- Return type says
seriesName: string, but you returnseriesName: nullwhen<h1>is missing.Recommend:
-export const getSerieasAndSeasonTitle = (el: HTMLElement): { seriesName: string; seasonName: string | null } => { +export const getSeriesAndSeasonTitle = ( + el: HTMLElement +): { seriesName: string | null; seasonName: string | null } => { const titleElement = el.querySelector('h1'); if (!titleElement) { return { seriesName: null, seasonName: null }; } const fullText = titleElement.innerText.trim(); if (fullText.includes(' - ')) { const [seriesName, seasonName] = fullText.split(' - ').map((part) => part.trim()); return { seriesName, seasonName }; } return { seriesName: fullText, seasonName: null }; };Then update the import and call site in
src/services/movie.service.tsto usegetSeriesAndSeasonTitle.
200-218: HardengetSeasonsOrEpisodes: avoid nulls, fix.trim()chain, and normalize URLs.Current implementation has several problems:
name,url, andidcan benull, butCSFDSeasondeclares them as non‑null.infoContainer?.textContent?.replace(/…/g, '').trim()will throw ifinfoContaineris missing, because.trim()is not behind optional chaining.urlis returned as a relative path; elsewhere you expose absolute URLs.Suggested rewrite:
-export const getSeasonsOrEpisodes = (el: HTMLElement, serie?: { id: number; title: string; }): CSFDSeason[] | null => { +export const getSeasonsOrEpisodes = (el: HTMLElement): CSFDSeason[] | null => { const childrenList = el.querySelector('.film-episodes-list'); if (!childrenList) return null; const childrenNodes = childrenList.querySelectorAll('.film-title'); if (!childrenNodes?.length) return []; - return childrenNodes.map((season) => { - const nameContainer = season.querySelector('.film-title-name'); - const infoContainer = season.querySelector('.info'); - - return { - id: parseLastIdFromUrl(nameContainer?.getAttribute('href') || ''), - name: nameContainer?.textContent?.trim() || null, - url: nameContainer?.getAttribute('href') || null, - info: infoContainer?.textContent?.replace(/[{()}]/g, '').trim() || null, - }; - }); + const items: CSFDSeason[] = []; + for (const node of childrenNodes) { + const nameEl = node.querySelector('.film-title-name'); + const infoEl = node.querySelector('.info'); + const href = nameEl?.getAttribute('href') || ''; + const id = parseLastIdFromUrl(href); + const name = nameEl?.textContent?.trim(); + + if (!id || !name || !href) continue; + + const rawInfo = infoEl?.textContent + ?.replace(/[{()}]/g, '') + .trim(); + + items.push({ + id, + name, + url: href.startsWith('http') ? href : `https://www.csfd.cz${href}`, + info: rawInfo || null, + }); + } + return items; }This keeps
CSFDSeasonnon‑nullable, avoids a potential runtime error on.trim(), and makes URLs consistent with the rest of the helpers.
239-258: AligngetSeasonorEpisodeParentwith types and avoid passing undefined toparseIdFromUrl.Issues:
parseIdFromUrl(parentSeries?.getAttribute('href'))can receiveundefined; the helper expects a string.- Function can return
{ series: null, season: {...} }or{ series: {...}, season: null }, butCSFDParentcurrently requires both fields non‑null.- With
parents.length === 0, you fabricate a parent{ series: serie, season: null }which is then used even for plain films via the service fallback, yielding a bogus “self‑parent”.At minimum:
- const [parentSeries, parentSeason] = parents; - - const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href')); - const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href')); + const [parentSeries, parentSeason] = parents; + + const seriesId = parseIdFromUrl(parentSeries?.getAttribute('href') || ''); + const seasonId = parseIdFromUrl(parentSeason?.getAttribute('href') || ''); const seriesName = parentSeries?.textContent?.trim() || null; const seasonName = parentSeason?.textContent?.trim() || null; const series = seriesId && seriesName ? { id: seriesId, name: seriesName } : null; const season = seasonId && seasonName ? { id: seasonId, name: seasonName } : null; if (!series && !season) return null; return { series, season };Combined with updating
CSFDParentto allow nullable members, this becomes type‑safe and matches real DOM possibilities. Also consider dropping theseriefallback entirely and just returningnullwhenparents.length === 0; the movie service can then decide when to compute a parent.src/services/movie.service.ts (2)
61-66: Fix helper name usage and clarify title/seasonName derivation.Once
getSerieasAndSeasonTitleis renamed and its types fixed, update this call accordingly:- const type = getMovieType(el) as CSFDFilmTypes; - const { seriesName = null, seasonName = null } = type === 'série' ? getSerieasAndSeasonTitle(el) : {}; + const type = getMovieType(el) as CSFDFilmTypes; + const { seriesName = null, seasonName = null } = + type === 'série' ? getSeriesAndSeasonTitle(el) : {};The
titleassignment logic (type === 'série' && seriesName ? seriesName : getMovieTitle(el)) looks reasonable and keeps the DTO’s title focused on the series name for season pages.Also applies to: 99-105
100-104: Correctparentcomputation: avoid bogus self‑parent for films and restrict to seasons/episodes.Current logic:
parent: (type === 'seriál') ? null : getSeasonorEpisodeParent(el, { id: movieId, name: title }),For plain films (non‑series), this calls
getSeasonorEpisodeParentwith a fallback{ id: movieId, name: title }, and the helper returns{ series: serie, season: null }when no parents are found. That incorrectly assigns every film a “parent” equal to itself.Recommend:
- parent: (type === 'seriál') ? null : getSeasonorEpisodeParent(el, { id: movieId, name: title }), + // Only derive parent for season/episode detail pages; films and root series have no parent. + parent: + type === 'epizoda' || type === 'série' + ? getSeasonorEpisodeParent(el) + : null,This aligns
parentwith the real hierarchy (episodes/seasons have parents; films and main series entries do not).
🧹 Nitpick comments (3)
src/dto/movie.ts (1)
20-22: Consider a distinct episode DTO instead of reusing CSFDSeason.
episodes: CSFDSeason[] | nullreuses the season type for episodes. If episode items differ even slightly (e.g., include an episode code, air date, or per‑episode info vs per‑season info), a dedicatedCSFDEpisodeinterface will keep the model clearer and safer to evolve.If the structures are truly identical, adding a short docstring clarifying that
CSFDSeasonis also used for episode list entries would help future readers.Also applies to: 166-171
src/helpers/movie.helper.ts (1)
231-237: MakedetectSeasonOrEpisodeListTypemore robust.Right now it inspects only the first
.box-header h3. If the seasons/episodes section isn’t the first box, you’ll silently returnnull.Consider scanning all headers instead:
-export const detectSeasonOrEpisodeListType = (el: HTMLElement) => { - const headerText = el.querySelector('.box-header h3')?.innerText.trim() ?? ''; - - if (headerText.includes('Série')) return 'seasons'; - if (headerText.startsWith('Epizody')) return 'episodes'; - return null; -} +export const detectSeasonOrEpisodeListType = (el: HTMLElement): 'seasons' | 'episodes' | null => { + const headers = el.querySelectorAll('.box-header h3'); + for (const header of headers) { + const text = header.innerText.trim(); + if (text.includes('Série')) return 'seasons'; + if (text.startsWith('Epizody')) return 'episodes'; + } + return null; +}Not critical, but makes the detection less brittle if CSFD changes box order.
src/services/movie.service.ts (1)
100-103: Ensure seasons/episodes arrays and DTOs stay consistent with helper behavior.The wiring:
seasons: seasonOrEpisodeListType === 'seasons' ? getSeasonsOrEpisodes(el) : null, episodes: seasonOrEpisodeListType === 'episodes' ? getSeasonsOrEpisodes(el) : null, episodeCode: type === 'epizoda' ? getEpisodeCode(el) : null,is fine conceptually, but depends on
getSeasonsOrEpisodesreturning items that matchCSFDSeason(non‑null id/name/url, absolute URLs). Once you adjust the helper as suggested, this assignment will be consistent and safe for consumers. Until then, the DTO over‑promises non‑nullable fields.After updating the helper, consider adding a couple of integration tests hitting a known series, season, and episode page to lock in expected shapes of
seasons,episodes,parent, andepisodeCode.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
demo.ts(1 hunks)src/dto/movie.ts(3 hunks)src/helpers/global.helper.ts(1 hunks)src/helpers/movie.helper.ts(2 hunks)src/services/movie.service.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/helpers/global.helper.ts
🧰 Additional context used
🧬 Code graph analysis (3)
demo.ts (2)
src/index.ts (2)
csfd(70-76)movie(39-42)src/services/movie.service.ts (1)
movie(36-52)
src/helpers/movie.helper.ts (2)
src/helpers/global.helper.ts (2)
parseIdFromUrl(4-12)parseLastIdFromUrl(14-22)src/dto/movie.ts (2)
CSFDSeason(166-171)CSFDParent(27-30)
src/services/movie.service.ts (1)
src/helpers/movie.helper.ts (6)
getSerieasAndSeasonTitle(23-39)detectSeasonOrEpisodeListType(231-237)getMovieTitle(41-43)getSeasonsOrEpisodes(200-218)getSeasonorEpisodeParent(239-259)getEpisodeCode(220-229)
🔇 Additional comments (1)
demo.ts (1)
7-7: Demo change looks fine.Switching the demo movie ID and commenting out the creator call is harmless and keeps the example simple.
What's new and why I made this pull request?
I've prepared the first version of the parsing for series, seasons, and episodes. Ready for testing – feel free to join in!
Just use
await csfd.movie(1431651)method and there are some brand new properties:Pull request type
Summary by CodeRabbit
New Features
Updates
Chore
✏️ Tip: You can customize this high-level summary in your review settings.