Fix: Handle invalid data in build_movies#128
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
trakt/users.py (1)
527-539: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for title-less movie entries.
Please test that invalid entries are skipped while valid entries still become
Movieinstances, including mixed valid/invalid payloads.🤖 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 `@trakt/users.py` around lines 527 - 539, Add regression tests for the movie parsing flow containing title-less entries: verify invalid entries are skipped, valid entries are converted to Movie instances, and a mixed payload returns only the valid movies. Anchor the tests to the parsing method containing the movie_data title check.
🤖 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 `@trakt/users.py`:
- Around line 527-539: Add regression tests for the movie parsing flow
containing title-less entries: verify invalid entries are skipped, valid entries
are converted to Movie instances, and a mixed payload returns only the valid
movies. Anchor the tests to the parsing method containing the movie_data title
check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 14129542-0f65-4853-a278-86d27cd3de3a
📒 Files selected for processing (1)
trakt/users.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
trakt/users.py
📄 CodeRabbit inference engine (AGENTS.md)
When editing
trakt/users.py, watch for nested model conversion and mixin-based attribute delegation.
Files:
trakt/users.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Match the surrounding file; keep local consistency instead of applying broad cleanup.
Import order should be stdlib, third-party, then localtrakt.*imports; letisorthandle ordering.
Preserve existing module docstrings and UTF-8 source headers in legacy files.
Avoid wholesale reflowing; keep line wrapping close to the nearby code.
Use single quotes where the file already does so.
Use type hints sparingly and locally; do not force a repo-wide typing migration.
Naming is conventional Python:snake_casefor functions and variables,PascalCasefor classes,UPPER_SNAKE_CASEfor constants, and leading underscores for private helpers.
Public exports are often tracked with__all__; update it when the import surface changes on purpose.
Keep module and class docstrings short and factual.
The request layer uses generator-based decorators intrakt.decorators; preserve that pattern for new endpoints that fit the existing model.
For import-only changes, runpre-commit run isort --files <touched files>or an equivalent touched-files-onlyisortinvocation.
Files:
trakt/users.py
trakt/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
trakt/**/*.py: Prefer absolute imports within the package, which is the dominant style here.
RaiseValueErrorfor invalid user input and parameter validation.
ReserveRuntimeErrorfor internal impossible states, not ordinary bad input.
Do not swallow exceptions unless the surrounding code already does that for compatibility.
Preserve the currentBadResponseExceptionbehavior when JSON decoding fails.
Follow the existingHttpClientandTokenAuthpatterns instead of adding a new request stack.
Many response objects are thin wrappers over API payloads; preserve fields that are already exposed.
Prefer explicit copying or filtering when normalizing JSON unless the current module intentionally mutates the input.
Keep list and collection classes iterable when the public API exposes them that way.
If you add new model properties, make sure they do not break existing__getattr__delegation in mixins and dataclasses.
Keep public return types stable; tests generally assert classes, counts, and a few important field values.
Files:
trakt/users.py
🔇 Additional comments (1)
trakt/users.py (1)
4-4: LGTM!Also applies to: 20-20
1 participant
/users/me/collection/movies?extended=metadataendpoint seems to be buggy:{'type': 'movie', 'collected_at': '2016-11-21T15:55:31.000Z', 'updated_at': '2019-02-03T16:31:47.000Z', 'available_on': [], 'metadata': None, 'movie': {'ids': {'imdb': None, 'plex': {'guid': None, 'slug': None}, 'slug': None, 'tmdb': None, 'trakt': None}, 'year': None, 'title': None}}x-pagination-page-count: 156, but page 30 is already emptyThis adds a workaround the first problem.
Summary by CodeRabbit