Skip to content

Refactor: Use build_uri for building urls#116

Merged
glensc merged 8 commits into
mainfrom
refactor-build-uri
Jul 7, 2026
Merged

Refactor: Use build_uri for building urls#116
glensc merged 8 commits into
mainfrom
refactor-build-uri

Conversation

@glensc

@glensc glensc commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Pull request overview

Refactors URL construction across the Trakt client to consistently use the shared build_uri(...) helper, centralizing pagination and extended query handling and reducing ad-hoc string concatenation.

Changes:

  • Extend build_uri(...) to support an extended query parameter in addition to pagination.
  • Replace manual URL/query-string building in multiple modules (tvuserssyncpeoplemoviescalendar) with build_uri(...).
  • Update/expand build_uri tests to cover extended query behavior.

Requires:

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3107d264-24dc-42f5-8455-08dcb35cf4af

📥 Commits

Reviewing files that changed from the base of the PR and between f8c30d8 and acb3897.

📒 Files selected for processing (8)
  • tests/test_utils.py
  • trakt/calendar.py
  • trakt/movies.py
  • trakt/people.py
  • trakt/sync.py
  • trakt/tv.py
  • trakt/users.py
  • trakt/utils.py
✅ Files skipped from review due to trivial changes (1)
  • tests/test_utils.py
🚧 Files skipped from review as they are similar to previous changes (7)
  • trakt/calendar.py
  • trakt/movies.py
  • trakt/sync.py
  • trakt/users.py
  • trakt/utils.py
  • trakt/people.py
  • trakt/tv.py

Walkthrough

build_uri now accepts extended query values, and several Trakt endpoint helpers were updated to use it for path and query construction. The utility tests now cover the new parameter ordering and existing-query handling.

Changes

build_uri Refactor

Layer / File(s) Summary
build_uri core extension and tests
trakt/utils.py, tests/test_utils.py
build_uri adds an optional extended parameter and appends it when present; tests verify updated query composition and ordering.
Calendar, media, people, and user URI construction
trakt/calendar.py, trakt/movies.py, trakt/people.py, trakt/users.py
Calendar, Movie, Person, and User URI properties and endpoints now call build_uri instead of assembling paths and query strings manually.
Sync endpoint URI construction
trakt/sync.py
get_watched and get_collection now build optional path segments and query parameters through build_uri instead of manual concatenation.
TV list endpoint URI construction
trakt/tv.py
Show list and recommendation/update endpoints now use build_uri for path templates and pagination/query parameters.
TV entity URI construction
trakt/tv.py
TVShow, TVSeason, and TVEpisode URI properties now use build_uri for templating and extended variants instead of manual formatting and query-string suffixes.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Poem

A rabbit hopped through query streams,
And build_uri stitched the seams.
Paths and params lined up just right,
No stringy tangles left in sight.
Hop hop—URIs now gleam 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: refactoring URL construction to use build_uri.
Description check ✅ Passed The description is directly related and accurately describes the refactor and test updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor-build-uri

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors URL construction across the Trakt client to consistently use the shared build_uri(...) helper, centralizing pagination and extended query handling and reducing ad-hoc string concatenation.

Changes:

  • Extend build_uri(...) to support an extended query parameter in addition to pagination.
  • Replace manual URL/query-string building in multiple modules (tv, users, sync, people, movies, calendar) with build_uri(...).
  • Update/expand build_uri tests to cover extended query behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
trakt/utils.py Adds extended support to build_uri and updates helper documentation.
trakt/users.py Switches collection and liked-lists endpoints to use build_uri.
trakt/tv.py Replaces manual pagination/extended query building with build_uri and removes urlencode usage.
trakt/sync.py Refactors watched/collection endpoint URI building to use build_uri with optional extended.
trakt/people.py Uses build_uri for person endpoints and extended/image variants.
trakt/movies.py Uses build_uri for movie endpoints and extended/image variants.
trakt/calendar.py Uses build_uri for calendar URL construction with optional extended.
tests/test_utils.py Updates tests to assert extended handling through build_uri.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread trakt/utils.py
Comment thread trakt/utils.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
trakt/tv.py (1)

290-304: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

_progress misuses build_uri to work around missing generic query-param support.

The template uri + '?' + '&'.join(f'{key}={{{key}}}' for key in params) reconstructs a query string by hand, then relies solely on build_uri's str.format step to fill it in — none of build_uri's actual query-building (page/limit/extended) is used here. It works, but it's harder to follow than the previous urlencode-based approach and creates a false impression that build_uri is being used for its query-construction purpose.

Consider keeping this on plain urlencode instead of routing it through build_uri:

♻️ Suggested simplification
     def _progress(self, progress_type,
                   specials=False, count_specials=False, hidden=False):
         uri = f'{self.ext}/progress/{progress_type}'
         params = {}
         if specials:
             params['specials'] = 'true'
         if count_specials:
             params['count_specials'] = 'true'
         if hidden:
             params['hidden'] = 'true'

         if params:
-            uri = build_uri(uri + '?' + '&'.join(
-                f'{key}={{{key}}}' for key in params
-            ), **params)
+            uri += '?' + urlencode(params)

         data = yield uri

         yield data
🤖 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/tv.py` around lines 290 - 304, The _progress method is hand-building a
query string and then using build_uri only for placeholder substitution, which
is misleading and harder to follow. Update _progress in trakt/tv.py to build its
specials/count_specials/hidden query parameters directly with urlencode-style
logic instead of routing them through build_uri, and keep the existing URI
construction behavior for progress_type intact.
🤖 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/tv.py`:
- Around line 290-304: The _progress method is hand-building a query string and
then using build_uri only for placeholder substitution, which is misleading and
harder to follow. Update _progress in trakt/tv.py to build its
specials/count_specials/hidden query parameters directly with urlencode-style
logic instead of routing them through build_uri, and keep the existing URI
construction behavior for progress_type intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 75ca0352-c7f3-418c-aba9-e076321803b1

📥 Commits

Reviewing files that changed from the base of the PR and between 7055e76 and a15459b.

📒 Files selected for processing (8)
  • tests/test_utils.py
  • trakt/calendar.py
  • trakt/movies.py
  • trakt/people.py
  • trakt/sync.py
  • trakt/tv.py
  • trakt/users.py
  • trakt/utils.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread trakt/utils.py
Comment thread tests/test_utils.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

glensc and others added 8 commits July 8, 2026 00:21
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
@glensc
glensc force-pushed the refactor-build-uri branch from f8c30d8 to acb3897 Compare July 7, 2026 21:22
@glensc
glensc merged commit fd10b8f into main Jul 7, 2026
8 checks passed
@glensc
glensc deleted the refactor-build-uri branch July 7, 2026 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants