[Playback Errors] Display special text for episode access issues#5195
[Playback Errors] Display special text for episode access issues#5195
Conversation
sztomek
added
[Type] Enhancement
There was a problem hiding this comment.
Pull request overview
Updates the playback error notice pipeline to show a distinct message (and a “learn more”-style link label) specifically for episode access issues (HTTP 401/403), and passes that link label through to the Compose info bar UI.
Changes:
- Add
linkTexttoPlaybackNoticeInfoand propagate it through mini-player/full-player error bars. - Update access-denied string copy and add a new action/link-label string resource.
- Render message + link-label with different styling in
PlaybackErrorInfoBar.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/services/repositories/.../PlaybackNoticeManager.kt | Sets special message/linkText for 401/403 playback issues. |
| modules/services/repositories/.../PlaybackErrorClassifier.kt | Adds helper to classify link text resource for HTTP codes. |
| modules/services/localization/.../strings.xml | Updates access-denied copy and adds link/action label string. |
| modules/services/compose/.../PlaybackErrorInfoBar.kt | Adds linkText param and renders styled link text in the info bar. |
| modules/features/player/.../PlayerHeaderFragment.kt | Passes linkText into the full-player error bar. |
| app/src/main/java/.../MainActivity.kt | Passes linkText into the mini-player error bar. |
| fun classifyLinkTextResId(responseCode: Int): Int? { | ||
| return when (responseCode) { | ||
| 401, 403 -> LR.string.error_streaming_access_denied_action | ||
| else -> null | ||
| } | ||
| } | ||
|
|
||
| @StringRes |
There was a problem hiding this comment.
classifyLinkTextResId() is currently unused in the codebase, which adds dead code and duplicates the access-denied mapping already handled elsewhere. Either remove it, or wire it into PlaybackNoticeManager/UI creation so the link-text mapping lives in one place; if kept, add unit coverage alongside the existing PlaybackErrorClassificationTest cases for 401/403.
| fun classifyLinkTextResId(responseCode: Int): Int? { | |
| return when (responseCode) { | |
| 401, 403 -> LR.string.error_streaming_access_denied_action | |
| else -> null | |
| } | |
| } | |
| @StringRes |
| val httpCode = (playbackState.playbackIssue as? PlaybackIssue.HttpError)?.statusCode | ||
| val isAccessDenied = httpCode == 401 || httpCode == 403 | ||
| PlaybackNoticeInfo( | ||
| message = context.getString(LR.string.error_episode_not_available), | ||
| message = context.getString( | ||
| if (isAccessDenied) LR.string.error_streaming_access_denied else LR.string.error_episode_not_available, | ||
| ), | ||
| type = PlaybackNoticeType.PLAYBACK, | ||
| supportUrl = errorClassifier.classifyHelpUrl(httpCode), | ||
| linkText = if (isAccessDenied) context.getString(LR.string.error_streaming_access_denied_action) else null, | ||
| ) |
There was a problem hiding this comment.
This introduces a second 401/403 classification path (isAccessDenied + inline string lookups) in addition to PlaybackErrorClassifier. Consider delegating both the message and the optional link text to the classifier (e.g., via resource IDs) to avoid future divergence and keep HTTP-code mapping centralized.
| if (linkText != null) { | ||
| val annotatedString = buildAnnotatedString { | ||
| append("$message ") | ||
| withStyle(SpanStyle(color = linkColor, textDecoration = TextDecoration.Underline)) { | ||
| append(linkText) | ||
| } | ||
| } | ||
| Text( | ||
| text = annotatedString, | ||
| color = color, | ||
| fontSize = 14.sp, | ||
| lineHeight = 20.sp, | ||
| fontWeight = FontWeight.W500, | ||
| textAlign = TextAlign.Center, | ||
| maxLines = Int.MAX_VALUE, | ||
| overflow = TextOverflow.Ellipsis, | ||
| modifier = modifier, |
There was a problem hiding this comment.
In the linkText != null branch, Text uses regular sp sizing (so it will scale with system font size) while the non-link branch uses TextH50(disableAutoScale = true). With the fixed 48.dp bar height, larger font scales can cause clipping/truncation. Align the linked-text styling with the non-scaled typography (e.g., using nonScaledSp for fontSize/lineHeight) and set a realistic maxLines (or relax the fixed height) so overflow is handled predictably.
Description
In case of 401 and 403 errors, we have decided to show the "This episode can't be played. Learn about possible causes" text.
comms: p1775075776708869/1774342537.838119-slack-C05RR9P9RAT
Testing Instructions
patch-debug-as-prod.patch
Screenshots or Screencast
Checklist
./gradlew spotlessApplyto automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xml