Fix 'Create new playlist' button in 'currently playing' menu not adding the song into the created playlist#88
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces direct playlist creation in the now-playing flow with Changes
Sequence Diagram(s)sequenceDiagram
participant UI as NowPlayingScreen
participant Provider as LibraryProvider
participant Service as SubsonicService
participant Server as Subsonic Server
UI->>Provider: createPlaylist(name, [songId])
Provider->>Service: createPlaylist(name, [songId])
Service->>Server: GET /createPlaylist?songId=...&name=...
Server-->>Service: HTTP 200 with subsonic-response
Service-->>Provider: parsed result / success or error
Provider-->>UI: success -> refresh playlists / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/services/subsonic_service.dart`:
- Around line 468-470: The code sets subsonicResponse differently for String vs
Map data but when response.data is a JSON string json.decode(data) returns the
whole payload instead of the nested 'subsonic-response' object; change the
String branch to json.decode(data)['subsonic-response'] (or decode to a Map and
then read the 'subsonic-response' key) so the variable subsonicResponse always
holds the nested object used later when checking status; update the logic around
response.data / json.decode accordingly to avoid reading status from the wrong
level.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 971c78b1-9e47-4689-bde4-285088e691ed
📒 Files selected for processing (2)
lib/screens/now_playing_screen.dartlib/services/subsonic_service.dart
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/subsonic_service.dart (1)
494-525:⚠️ Potential issue | 🟠 MajorAvoid logging the raw
updatePlaylistURL.Line 508 prints the fully assembled URL, which includes auth query params (
uand eitherport/s). That bypasses_sanitizeUrl()and can leak credentials into device logs or crash reports.🔒 Proposed fix
- debugPrint('updatePlaylist URL: $url'); + debugPrint('updatePlaylist URL: ${_sanitizeUrl(url)}');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/subsonic_service.dart` around lines 494 - 525, The updatePlaylist method currently logs the full URL (variable url) which may contain auth credentials; replace that raw logging by passing the URL through the existing _sanitizeUrl helper before printing (i.e., use _sanitizeUrl(url) in the debugPrint call) so credentials are redacted; update the debugPrint in updatePlaylist accordingly and ensure _sanitizeUrl is used wherever raw URLs are logged elsewhere in this class (e.g., any other debugPrints that reference _buildUrl output).
🧹 Nitpick comments (1)
lib/services/subsonic_service.dart (1)
464-480: Extract the Subsonic unwrap/status check into one helper.
createPlaylist,updatePlaylist,jukeboxControl, and_requestare now all decoding and validating the same payload shape a little differently. Pulling that into one helper will keep playlist creation/update behavior aligned and prevent the next response-parsing fix from landing in only one path.♻️ Sketch of the refactor
- final response = await _dio.get(url); - final data = response.data; - - final decoded = data is String ? json.decode(data) : data; - final subsonicResponse = decoded['subsonic-response']; - if (subsonicResponse == null) { - throw Exception('Invalid response format'); - } - - if (subsonicResponse['status'] != 'ok') { - final error = subsonicResponse['error']; - throw Exception(error?['message'] ?? 'Unknown error'); - } + _parseSubsonicResponse((await _dio.get(url)).data);Map<String, dynamic> _parseSubsonicResponse(dynamic data) { final decoded = data is String ? json.decode(data) : data; final subsonicResponse = decoded['subsonic-response']; if (subsonicResponse == null) { throw Exception('Invalid response format'); } if (subsonicResponse['status'] != 'ok') { final error = subsonicResponse['error']; throw Exception(error?['message'] ?? 'Unknown error'); } return Map<String, dynamic>.from(subsonicResponse as Map); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/subsonic_service.dart` around lines 464 - 480, Multiple methods (createPlaylist, updatePlaylist, jukeboxControl, and _request) duplicate decoding/validation of the Subsonic payload; extract that logic into a single helper (e.g., _parseSubsonicResponse) that accepts the raw response data, decodes JSON when necessary, validates presence of 'subsonic-response', checks status == 'ok', throws the same Exceptions on error, and returns the validated subsonic-response as a Map<String, dynamic>; then replace the inline decoding/validation in createPlaylist, updatePlaylist, jukeboxControl, and _request to call _parseSubsonicResponse and use its returned map.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/services/subsonic_service.dart`:
- Around line 494-525: The updatePlaylist method currently logs the full URL
(variable url) which may contain auth credentials; replace that raw logging by
passing the URL through the existing _sanitizeUrl helper before printing (i.e.,
use _sanitizeUrl(url) in the debugPrint call) so credentials are redacted;
update the debugPrint in updatePlaylist accordingly and ensure _sanitizeUrl is
used wherever raw URLs are logged elsewhere in this class (e.g., any other
debugPrints that reference _buildUrl output).
---
Nitpick comments:
In `@lib/services/subsonic_service.dart`:
- Around line 464-480: Multiple methods (createPlaylist, updatePlaylist,
jukeboxControl, and _request) duplicate decoding/validation of the Subsonic
payload; extract that logic into a single helper (e.g., _parseSubsonicResponse)
that accepts the raw response data, decodes JSON when necessary, validates
presence of 'subsonic-response', checks status == 'ok', throws the same
Exceptions on error, and returns the validated subsonic-response as a
Map<String, dynamic>; then replace the inline decoding/validation in
createPlaylist, updatePlaylist, jukeboxControl, and _request to call
_parseSubsonicResponse and use its returned map.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66ccc39b-9179-4b34-9d39-c92c3e8cf32b
📒 Files selected for processing (1)
lib/services/subsonic_service.dart
2 participants
Fixes #87
Summary by CodeRabbit
New Features
Bug Fixes