Conversation
WalkthroughThe Plex watchlist fetch URL in PlexUtils was updated from metadata.provider.plex.tv to discover.provider.plex.tv. Corresponding test expectations were revised to match the new endpoint. No public signatures or control flow changed; parameters and pagination logic remain the same. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/scala/plex/PlexUtils.scala (1)
74-74: Centralize Plex Discover base URI across code and testsThe switch to the Discover endpoint looks correct and no deprecated URLs remain. To prevent future churn, pull the host (
https://discover.provider.plex.tv) into a single constant and use it everywhere—both inPlexUtilsand in your specs.• In src/main/scala/plex/PlexUtils.scala (or its companion object) add:
object PlexUtils { /** Base URI for Plex Discover endpoints */ val DiscoverBase: String = "https://discover.provider.plex.tv" // …existing members… }• In the watchlist callsite, update:
- .unsafeFromString("https://discover.provider.plex.tv/library/sections/watchlist/all") + .unsafeFromString(s"${PlexUtils.DiscoverBase}/library/sections/watchlist/all")• In your specs (PlexUtilsSpec.scala, PlexTokenSyncSpec.scala), replace hard-coded URLs:
- Uri.unsafeFromString("https://discover.provider.plex.tv/library/sections/watchlist/all?X-Plex-Token=test-token&X-Plex-Container-Start=0&X-Plex-Container-Size=300") + Uri.unsafeFromString(s"${PlexUtils.DiscoverBase}/library/sections/watchlist/all?X-Plex-Token=test-token&X-Plex-Container-Start=0&X-Plex-Container-Size=300")This optional refactor keeps the host in one place, so any future endpoint changes only need a single update.
src/test/scala/PlexTokenSyncSpec.scala (1)
73-74: Updated expectation to Discover endpoint — LGTM; build the Uri via query params to reduce brittlenessCurrent expectation uses a fully inlined querystring, which can be brittle if param ordering ever changes. Compose the Uri like production code does.
- "https://discover.provider.plex.tv/library/sections/watchlist/all?X-Plex-Token=plex-token&X-Plex-Container-Start=0&X-Plex-Container-Size=300" + ( + Uri + .unsafeFromString("https://discover.provider.plex.tv/library/sections/watchlist/all") + .withQueryParam("X-Plex-Token", "plex-token") + .withQueryParam("X-Plex-Container-Start", "0") + .withQueryParam("X-Plex-Container-Size", "300") + )src/test/scala/plex/PlexUtilsSpec.scala (1)
82-83: Discover endpoint expectations — LGTM; prefer Uri composition over literal querystringsThese changes are correct. For resilience, compose the Uri via withQueryParam (mirrors production code and avoids strict reliance on param order).
- "https://discover.provider.plex.tv/library/sections/watchlist/all?X-Plex-Token=test-token&X-Plex-Container-Start=0&X-Plex-Container-Size=300" + ( + Uri + .unsafeFromString("https://discover.provider.plex.tv/library/sections/watchlist/all") + .withQueryParam("X-Plex-Token", "test-token") + .withQueryParam("X-Plex-Container-Start", "0") + .withQueryParam("X-Plex-Container-Size", "300") + )- "https://discover.provider.plex.tv/library/sections/watchlist/all?X-Plex-Token=test-token&X-Plex-Container-Start=0&X-Plex-Container-Size=300" + ( + Uri + .unsafeFromString("https://discover.provider.plex.tv/library/sections/watchlist/all") + .withQueryParam("X-Plex-Token", "test-token") + .withQueryParam("X-Plex-Container-Start", "0") + .withQueryParam("X-Plex-Container-Size", "300") + )- "https://discover.provider.plex.tv/library/sections/watchlist/all?X-Plex-Token=test-token&X-Plex-Container-Start=0&X-Plex-Container-Size=300" + ( + Uri + .unsafeFromString("https://discover.provider.plex.tv/library/sections/watchlist/all") + .withQueryParam("X-Plex-Token", "test-token") + .withQueryParam("X-Plex-Container-Start", "0") + .withQueryParam("X-Plex-Container-Size", "300") + )Also applies to: 127-128, 149-150
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/scala/plex/PlexUtils.scala(1 hunks)src/test/scala/PlexTokenSyncSpec.scala(1 hunks)src/test/scala/plex/PlexUtilsSpec.scala(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nylonee
PR: nylonee/watchlistarr#57
File: src/test/scala/configuration/ConfigurationUtilsSpec.scala:121-130
Timestamp: 2024-06-10T19:44:23.144Z
Learning: The expected result for the test case that verifies the splitting of multiple RSS watchlist feeds is based on the mock HTTP client's response, which may contain a single `RSSInfo` object, rather than directly using the `plexWatchlists` configuration provided. This understanding is crucial for accurately assessing the expected outcomes in test cases.
📚 Learning: 2024-06-10T19:44:23.144Z
Learnt from: nylonee
PR: nylonee/watchlistarr#57
File: src/test/scala/configuration/ConfigurationUtilsSpec.scala:121-130
Timestamp: 2024-06-10T19:44:23.144Z
Learning: The expected result for the test case that verifies the splitting of multiple RSS watchlist feeds is based on the mock HTTP client's response, which may contain a single `RSSInfo` object, rather than directly using the `plexWatchlists` configuration provided. This understanding is crucial for accurately assessing the expected outcomes in test cases.
Applied to files:
src/test/scala/plex/PlexUtilsSpec.scala
|
@nylonee if you need any help in getting this through, let me know! I'm seeing the same issue on my end and would love to get this merged in |
|
I've published an image that incorporates the fix in #219: https://hub.docker.com/r/merc248/watchlistarr (replace your image tag with |
i didn't see this post before. Great job and thanks ;) |
|
@nylonee Would be great to get this merged. |
|
Hi all! Sorry for the delay, just saw these messages. Looking into this now, thanks for the PR and the reviews! |
|
@nylonee would you mind creating a new release so the docker image will be built with this fix? |
7 participants
Description
This PR updates the Plex watchlist base URL to the new endpoint, replacing the deprecated one.
Checklist
sbt scalafmtAllRun (and optionallysbt scalafmtSbt)Summary by CodeRabbit
New Features
Bug Fixes
Tests