From da4878d2647e051724b98cfa9fe5b8bf36cb389e Mon Sep 17 00:00:00 2001 From: Su TT Date: Wed, 2 Jul 2025 16:59:51 -0400 Subject: [PATCH 01/15] feat(ui):Add ErrorPanel composable to Comments Section, related UI models, and tests --- .../video/comments/CommentSectionErrorTest.kt | 64 ++++++++++ .../org/schabi/newpipe/error/ErrorUIMapper.kt | 14 +++ .../schabi/newpipe/paging/CommentsSource.kt | 3 + .../schabi/newpipe/ui/UiModel/ErrorUiModel.kt | 41 ++++++ .../ui/components/common/ErrorPanel.kt | 117 ++++++++++++++++++ .../components/common/ServiceColoredButton.kt | 54 ++++++++ .../video/comment/CommentErrorHandler.kt | 64 ++++++++++ .../video/comment/CommentSection.kt | 27 ++-- .../newpipe/viewmodels/CommentsViewModel.kt | 1 + .../schabi/newpipe/error/ErrorUiModelTest.kt | 63 ++++++++++ 10 files changed, 436 insertions(+), 12 deletions(-) create mode 100644 app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt create mode 100644 app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt create mode 100644 app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt create mode 100644 app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt create mode 100644 app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt create mode 100644 app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt create mode 100644 app/src/test/java/org/schabi/newpipe/error/ErrorUiModelTest.kt diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt new file mode 100644 index 000000000..bcf9641bd --- /dev/null +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt @@ -0,0 +1,64 @@ +package org.schabi.newpipe.ui.components.video.comments + +import android.net.http.NetworkException +import androidx.paging.LoadState +import androidx.test.ext.junit.rules.ActivityScenarioRule +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.schabi.newpipe.MainActivity +import org.schabi.newpipe.R +import org.schabi.newpipe.ui.UiModel.UnableToLoadCommentsUiModel +import org.schabi.newpipe.viewmodels.util.Resource + +@RunWith(AndroidJUnit4::class) +class CommentSectionErrorTest { + + @get:Rule + val activityRule = ActivityScenarioRule(MainActivity::class.java) + + /** + * Test Resource.Error state - when initial comment info loading fails + */ + @Test + fun testResourceErrorState_ShowsUnableToLoadCommentsUiModel() { + + val networkException = object : NetworkException("Connection attempt timed out", null) { + override fun getErrorCode(): Int = NetworkException.ERROR_CONNECTION_TIMED_OUT + override fun isImmediatelyRetryable() = true + } + val errorResource = Resource.Error(networkException) + assertEquals("Should contain the network exception", networkException, errorResource.throwable) + + val errorModel = UnableToLoadCommentsUiModel(networkException) + val spec = errorModel.spec + + assertEquals("Should have correct message resource", R.string.error_unable_to_load_comments, spec.messageRes) + assertTrue("Should show retry button", spec.showRetry) + assertTrue("Should show report button", spec.showReport) + assertFalse("Should NOT show open in browser button", spec.showOpenInBrowser) + } + + /** + * Test LoadState.Error state - when paging data loading fails + */ + @Test + fun testLoadStateErrorState_ShowsUnableToLoadCommentsUiModel() { + val pagingException = RuntimeException("Paging data loading failed") + val loadStateError = LoadState.Error(pagingException) + + assertEquals("Should contain the paging exception", pagingException, loadStateError.error) + + val errorModel = UnableToLoadCommentsUiModel(pagingException) + val spec = errorModel.spec + + assertEquals("Should have correct message resource", R.string.error_unable_to_load_comments, spec.messageRes) + assertTrue("Should show retry button", spec.showRetry) + assertTrue("Should show report button", spec.showReport) + assertFalse("Should NOT show open in browser button", spec.showOpenInBrowser) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt new file mode 100644 index 000000000..5df92ddfc --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt @@ -0,0 +1,14 @@ +package org.schabi.newpipe.error + +import org.schabi.newpipe.ui.UiModel.ErrorUiModel +import org.schabi.newpipe.ui.UiModel.GenericErrorUiModel +import org.schabi.newpipe.ui.UiModel.UnableToLoadCommentsUiModel + +fun mapThrowableToErrorUiModel(throwable: Throwable, userAction: UserAction? = null): ErrorUiModel { + if (userAction == UserAction.REQUESTED_COMMENTS) { + + return UnableToLoadCommentsUiModel(rawError = throwable) + } + // Other ErrorInfo logic and throwable + user actions + return GenericErrorUiModel(rawError = throwable) +} diff --git a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt index 669485e66..bec4911df 100644 --- a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt +++ b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt @@ -9,6 +9,7 @@ import org.schabi.newpipe.extractor.Page import org.schabi.newpipe.extractor.comments.CommentsInfo import org.schabi.newpipe.extractor.comments.CommentsInfoItem import org.schabi.newpipe.ui.components.video.comment.CommentInfo +import java.io.IOException class CommentsSource(private val commentInfo: CommentInfo) : PagingSource() { private val service = NewPipe.getService(commentInfo.serviceId) @@ -16,12 +17,14 @@ class CommentsSource(private val commentInfo: CommentInfo) : PagingSource): LoadResult { // params.key is null the first time the load() function is called, so we need to return the // first batch of already-loaded comments + return LoadResult.Error(IOException("💥 forced test error")) if (params.key == null) { return LoadResult.Page(commentInfo.comments, null, commentInfo.nextPage) } else { val info = withContext(Dispatchers.IO) { CommentsInfo.getMoreItems(service, commentInfo.url, params.key) } + return LoadResult.Page(info.items, null, info.nextPage) } } diff --git a/app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt b/app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt new file mode 100644 index 000000000..6d91c78cd --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt @@ -0,0 +1,41 @@ +package org.schabi.newpipe.ui.UiModel + +import androidx.compose.runtime.Immutable +import org.schabi.newpipe.R +import org.schabi.newpipe.ui.components.common.ErrorPanelSpec + +/** + * Each concrete case from this hierarchy represents a different failure state that the UI can render with ErrorPanel + */ +@Immutable +sealed interface ErrorUiModel { + val spec: ErrorPanelSpec + val rawError: Throwable? get() = null +} + +/** + * Concrete cases - Comments unable to load, Comments disabled, No connectivity, DNS failure, timeout etc + */ +@Immutable +data class UnableToLoadCommentsUiModel(override val rawError: Throwable?) : ErrorUiModel { + override val spec: ErrorPanelSpec = + ErrorPanelSpec( + messageRes = R.string.error_unable_to_load_comments, + showRetry = true, + showReport = true, + showOpenInBrowser = false + ) +} + +/** + * A generic ErrorUiModel for unhandled cases + */ +@Immutable +data class GenericErrorUiModel(override val rawError: Throwable?) : ErrorUiModel { + override val spec: ErrorPanelSpec = + ErrorPanelSpec( + messageRes = R.string.general_error, + showRetry = true, + showReport = true, + ) +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt new file mode 100644 index 000000000..5f16941d9 --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -0,0 +1,117 @@ +package org.schabi.newpipe.ui.components.common + +import androidx.annotation.StringRes +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.wrapContentWidth +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.tooling.preview.Preview +import org.schabi.newpipe.R +import org.schabi.newpipe.ui.theme.AppTheme +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingExtraLarge +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingLarge +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingMedium +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingSmall + +@Composable +fun ErrorPanel( + spec: ErrorPanelSpec, + onRetry: () -> Unit, + onReport: () -> Unit, + onOpenInBrowser: () -> Unit, + modifier: Modifier = Modifier + +) { + Column( + horizontalAlignment = Alignment.CenterHorizontally, + modifier = Modifier + .wrapContentWidth() + .padding(horizontal = SpacingLarge, vertical = SpacingMedium) + + ) { + + val message = stringResource(spec.messageRes) + + Text( + text = message, + style = MaterialTheme.typography.titleMedium.copy(fontWeight = FontWeight.Bold), + textAlign = TextAlign.Center + ) + + spec.serviceInfoRes?.let { infoRes -> + Spacer(Modifier.height(SpacingSmall)) + val serviceInfo = stringResource(infoRes) + Text( + text = serviceInfo, + style = MaterialTheme.typography.bodyMedium, + textAlign = TextAlign.Center + ) + } + + spec.serviceExplanationRes?.let { explanationRes -> + Spacer(Modifier.height(SpacingSmall)) + val serviceExplanation = stringResource(explanationRes) + Text( + text = serviceExplanation, + style = MaterialTheme.typography.bodyMedium, + textAlign = TextAlign.Center + ) + } + Spacer(Modifier.height(SpacingMedium)) + if (spec.showReport) { + ServiceColoredButton(onClick = onReport) { + Text(stringResource(R.string.error_snackbar_action).uppercase()) + } + } + if (spec.showRetry) { + ServiceColoredButton(onClick = onRetry) { + Text(stringResource(R.string.retry).uppercase()) + } + } + if (spec.showOpenInBrowser) { + ServiceColoredButton(onClick = onOpenInBrowser) { + Text(stringResource(R.string.open_in_browser).uppercase()) + } + } + Spacer(Modifier.height(SpacingExtraLarge)) + } +} + +data class ErrorPanelSpec( + @StringRes val messageRes: Int, + @StringRes val serviceInfoRes: Int? = null, + val serviceExplanation: String? = null, + @StringRes val serviceExplanationRes: Int? = null, + val showRetry: Boolean = false, + val showReport: Boolean = false, + val showOpenInBrowser: Boolean = false +) + +@Preview(showBackground = true, widthDp = 360, heightDp = 640) + +@Composable +fun ErrorPanelPreview() { + AppTheme { + ErrorPanel( + spec = ErrorPanelSpec( + messageRes = android.R.string.httpErrorBadUrl, + showRetry = true, + showReport = false, + showOpenInBrowser = false + ), + onRetry = {}, + onReport = {}, + onOpenInBrowser = {}, + modifier = Modifier + ) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt new file mode 100644 index 000000000..8b3c41a8a --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt @@ -0,0 +1,54 @@ +package org.schabi.newpipe.ui.components.common + +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.RowScope +import androidx.compose.foundation.layout.wrapContentWidth +import androidx.compose.material3.Button +import androidx.compose.material3.ButtonDefaults +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.RectangleShape +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import org.schabi.newpipe.ui.theme.AppTheme +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingMedium +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingSmall + +@Composable +fun ServiceColoredButton( + onClick: () -> Unit, + modifier: Modifier = Modifier, + content: @Composable() RowScope.() -> Unit, +) { + Button( + onClick = onClick, + modifier = modifier.wrapContentWidth(), + colors = ButtonDefaults.buttonColors( + containerColor = MaterialTheme.colorScheme.error, + contentColor = MaterialTheme.colorScheme.onError + ), + contentPadding = PaddingValues(horizontal = SpacingMedium, vertical = SpacingSmall), + shape = RectangleShape, + elevation = ButtonDefaults.buttonElevation( + defaultElevation = 8.dp, + + ), + ) { + content() + } +} + +@Preview +@Composable +fun ServiceColoredButtonPreview() { + AppTheme { + ServiceColoredButton( + onClick = {}, + content = { + Text("Button") + } + ) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt new file mode 100644 index 000000000..152c032fd --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt @@ -0,0 +1,64 @@ +package org.schabi.newpipe.ui.components.video.comment + +import android.util.Log +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.wrapContentHeight +import androidx.compose.runtime.Composable +import androidx.compose.runtime.SideEffect +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.input.nestedscroll.NestedScrollSource.Companion.SideEffect +import androidx.compose.ui.tooling.preview.Preview +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.UserAction +import org.schabi.newpipe.error.mapThrowableToErrorUiModel +import org.schabi.newpipe.ui.UiModel.ErrorUiModel +import org.schabi.newpipe.ui.components.common.ErrorPanel +import java.io.IOException + +@Composable +fun CommentErrorHandler( + throwable: Throwable, + userAction: UserAction, + onRetry: () -> Unit, + onReport: (ErrorInfo) -> Unit +) { + SideEffect { + Log.d("CommentErrorHandler", "⛔️ rendered for error: ${throwable.message}") + } + + val uiModel: ErrorUiModel = mapThrowableToErrorUiModel(throwable, userAction) + val errorInfo = ErrorInfo( + throwable = throwable, + userAction = userAction, + request = "" + ) + + Box( + modifier = Modifier.fillMaxSize(), + contentAlignment = Alignment.Center + ) { + ErrorPanel( + spec = uiModel.spec, + onRetry = onRetry, + onReport = { onReport(errorInfo) }, + onOpenInBrowser = {}, + modifier = Modifier + .fillMaxWidth() + .wrapContentHeight() + ) + } +} + +@Preview(showBackground = true) +@Composable +fun PreviewCommentErrorHandler() { + CommentErrorHandler( + throwable = IOException("No network"), + userAction = UserAction.REQUESTED_COMMENTS, + onRetry = {}, + onReport = {} + ) +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt index a33ffc0ca..327d30f72 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt @@ -13,6 +13,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.rememberNestedScrollInteropConnection import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.tooling.preview.Preview @@ -25,6 +26,8 @@ import androidx.paging.compose.collectAsLazyPagingItems import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flowOf import org.schabi.newpipe.R +import org.schabi.newpipe.error.ErrorUtil +import org.schabi.newpipe.error.UserAction import org.schabi.newpipe.extractor.Page import org.schabi.newpipe.extractor.comments.CommentsInfoItem import org.schabi.newpipe.extractor.stream.Description @@ -50,6 +53,7 @@ private fun CommentSection( val comments = commentsFlow.collectAsLazyPagingItems() val nestedScrollInterop = rememberNestedScrollInteropConnection() val state = rememberLazyListState() + val context = LocalContext.current LazyColumnThemedScrollbar(state = state) { LazyColumn( @@ -98,21 +102,22 @@ private fun CommentSection( ) } } - when (comments.loadState.refresh) { is LoadState.Loading -> { item { LoadingIndicator(modifier = Modifier.padding(top = 8.dp)) } } - is LoadState.Error -> { item { - // TODO use error panel instead - EmptyStateComposable(EmptyStateSpec.ErrorLoadingComments) + CommentErrorHandler( + throwable = (comments.loadState.refresh as LoadState.Error).error, + userAction = UserAction.REQUESTED_COMMENTS, + onRetry = { comments.retry() }, + onReport = { info -> ErrorUtil.openActivity(context, info) }, + ) } } - else -> { items(comments.itemCount) { Comment(comment = comments[it]!!) {} @@ -121,15 +126,13 @@ private fun CommentSection( } } } - is Resource.Error -> { item { - // TODO use error panel instead - EmptyStateComposable( - spec = EmptyStateSpec.ErrorLoadingComments, - modifier = Modifier - .fillMaxWidth() - .heightIn(min = 128.dp) + CommentErrorHandler( + throwable = uiState.throwable, + userAction = UserAction.REQUESTED_COMMENTS, + onRetry = { comments.retry() }, + onReport = { info -> ErrorUtil.openActivity(context, info) }, ) } } diff --git a/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt b/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt index 007292498..58886fc68 100644 --- a/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt +++ b/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt @@ -23,6 +23,7 @@ import org.schabi.newpipe.viewmodels.util.Resource class CommentsViewModel(savedStateHandle: SavedStateHandle) : ViewModel() { val uiState = savedStateHandle.getStateFlow(KEY_URL, "") .map { + // Resource.Error(RuntimeException("Forced error for testing")) try { Resource.Success(CommentInfo(CommentsInfo.getInfo(it))) } catch (e: Exception) { diff --git a/app/src/test/java/org/schabi/newpipe/error/ErrorUiModelTest.kt b/app/src/test/java/org/schabi/newpipe/error/ErrorUiModelTest.kt new file mode 100644 index 000000000..641f95966 --- /dev/null +++ b/app/src/test/java/org/schabi/newpipe/error/ErrorUiModelTest.kt @@ -0,0 +1,63 @@ +package org.schabi.newpipe.error + +import android.net.http.NetworkException +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.schabi.newpipe.R +import org.schabi.newpipe.ui.UiModel.GenericErrorUiModel +import org.schabi.newpipe.ui.UiModel.UnableToLoadCommentsUiModel + +class ErrorUiModelTest { + + /** + * Test that when comments fail to load, the correct error panel is rendered + */ + @Test + fun `mapThrowableToErrorUiModel with REQUESTED_COMMENTS returns UnableToLoadCommentsUiModel`() { + // val throwable = RuntimeException("Comments failed to load") + val networkException = object : NetworkException("Connection attempt timed out", null) { + override fun getErrorCode() = NetworkException.ERROR_CONNECTION_TIMED_OUT + override fun isImmediatelyRetryable() = true + } + val result = mapThrowableToErrorUiModel(networkException, UserAction.REQUESTED_COMMENTS) + assertTrue("Result should be UnableToLoadCommentsUiModel", result is UnableToLoadCommentsUiModel) + assertEquals("Raw error should be preserved for debugging", networkException, result.rawError) + } + + /** + * Test the fallback logic + */ + @Test + fun `mapThrowableToErrorUiModel with null UserAction returns GenericErrorUiModel`() { + val throwable = RuntimeException("Test error") + val result = mapThrowableToErrorUiModel(throwable, null) + + assertTrue("Should return GenericErrorUiModel", result is GenericErrorUiModel) + assertEquals("Should preserve the original throwable", throwable, result.rawError) + } + + /** + * Test that UnableToLoadCommentsUiModel maps to the correct error panel configuration + */ + @Test + fun `UnableToLoadCommentsUiModel has correct ErrorPanelSpec`() { + val throwable = RuntimeException("Test error") + val errorModel = UnableToLoadCommentsUiModel(throwable) + val spec = errorModel.spec + // Assert: Verify the spec has the correct configuration for comment loading errors + assertEquals( + "Error message should be 'Unable to load comments'", + R.string.error_unable_to_load_comments, + spec.messageRes, + ) + + assertTrue("Retry button should be shown for comment loading errors", spec.showRetry) + assertTrue("Report button should be shown for comment loading errors", spec.showReport) + assertFalse("Open in browser should NOT be shown for comment loading errors", spec.showOpenInBrowser) + + // Assert: Verify the raw error is set correctly + assertEquals("Raw error should be preserved for debugging", throwable, errorModel.rawError) + } +} From 19fb9899cd76f4221fd408c415d86358da54cc41 Mon Sep 17 00:00:00 2001 From: Su TT Date: Thu, 3 Jul 2025 14:25:36 -0400 Subject: [PATCH 02/15] Fix CommentSectionErrorTest to use named NetworkException for instrumented test compatibility --- .../components/video/comments/CommentSectionErrorTest.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt index bcf9641bd..d4cb2ab54 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt @@ -24,13 +24,14 @@ class CommentSectionErrorTest { /** * Test Resource.Error state - when initial comment info loading fails */ + class TestNetworkException : NetworkException("Connection attempt timed out", null) { + override fun getErrorCode(): Int = NetworkException.ERROR_CONNECTION_TIMED_OUT + override fun isImmediatelyRetryable() = true + } @Test fun testResourceErrorState_ShowsUnableToLoadCommentsUiModel() { - val networkException = object : NetworkException("Connection attempt timed out", null) { - override fun getErrorCode(): Int = NetworkException.ERROR_CONNECTION_TIMED_OUT - override fun isImmediatelyRetryable() = true - } + val networkException = TestNetworkException() val errorResource = Resource.Error(networkException) assertEquals("Should contain the network exception", networkException, errorResource.throwable) From 8292f729ea792284a77bb9420a5b48dde700f0cc Mon Sep 17 00:00:00 2001 From: Su TT Date: Wed, 30 Jul 2025 17:45:56 -0400 Subject: [PATCH 03/15] Updated ERefactor ErrorPanel to use ErrorInfo directly and remove UI models --- .../CommentSectionErrorIntegrationTest.kt | 119 +++++++++++++++++ .../video/comments/CommentSectionErrorTest.kt | 65 --------- .../org/schabi/newpipe/error/ErrorUIMapper.kt | 14 -- .../schabi/newpipe/paging/CommentsSource.kt | 2 +- .../schabi/newpipe/ui/UiModel/ErrorUiModel.kt | 41 ------ .../ui/components/common/ErrorPanel.kt | 123 ++++++++++-------- .../video/comment/CommentErrorHandler.kt | 64 --------- .../video/comment/CommentSection.kt | 53 ++++++-- .../newpipe/viewmodels/CommentsViewModel.kt | 1 - .../main/res/layout/fragment_video_detail.xml | 8 ++ .../schabi/newpipe/error/ErrorUiModelTest.kt | 63 --------- 11 files changed, 237 insertions(+), 316 deletions(-) create mode 100644 app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionErrorIntegrationTest.kt delete mode 100644 app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt delete mode 100644 app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt delete mode 100644 app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt delete mode 100644 app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt delete mode 100644 app/src/test/java/org/schabi/newpipe/error/ErrorUiModelTest.kt diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionErrorIntegrationTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionErrorIntegrationTest.kt new file mode 100644 index 000000000..12bcd85ac --- /dev/null +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionErrorIntegrationTest.kt @@ -0,0 +1,119 @@ +package org.schabi.newpipe.ui.components.video.comment + +import android.content.Context +import android.content.Intent +import androidx.paging.LoadState +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.UserAction +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException +import org.schabi.newpipe.ui.components.common.ErrorAction +import org.schabi.newpipe.ui.components.common.determineErrorAction +import org.schabi.newpipe.viewmodels.util.Resource +import java.io.IOException +import java.net.SocketTimeoutException + +@RunWith(AndroidJUnit4::class) +class CommentSectionErrorIntegrationTest { + + private lateinit var context: Context + + @Before + fun setUp() { + context = ApplicationProvider.getApplicationContext() + } + + // Test 1: Network error on initial load (Resource.Error) + @Test + fun testInitialCommentNetworkError() { + val expectedMessage = "Connection timeout" + val networkError = SocketTimeoutException(expectedMessage) + val resourceError = Resource.Error(networkError) + + val errorInfo = ErrorInfo( + throwable = resourceError.throwable, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + assertEquals(networkError, errorInfo.throwable) + assertEquals(ErrorAction.REPORT, determineErrorAction(errorInfo)) + assertEquals(expectedMessage, errorInfo.getExplanation()) + } + + // Test 2: Network error on paging (LoadState.Error) + @Test + fun testPagingNetworkError() { + val expectedMessage = "Paging failed" + val pagingError = IOException(expectedMessage) + val loadStateError = LoadState.Error(pagingError) + + val errorInfo = ErrorInfo( + throwable = loadStateError.error, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + assertEquals(pagingError, errorInfo.throwable) + assertEquals(ErrorAction.REPORT, determineErrorAction(errorInfo)) + assertEquals(expectedMessage, errorInfo.getExplanation()) + } + + // Test 3: ReCaptcha during comments load + @Test + fun testReCaptchaDuringComments() { + val url = "https://www.google.com/recaptcha/api/fallback?k=test" + val expectedMessage = "ReCaptcha needed" + val captcha = ReCaptchaException(expectedMessage, url) + val errorInfo = ErrorInfo( + throwable = captcha, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + assertEquals(ErrorAction.SOLVE_CAPTCHA, determineErrorAction(errorInfo)) + assertEquals(expectedMessage, errorInfo.getExplanation()) + + val intent = Intent(context, org.schabi.newpipe.error.ReCaptchaActivity::class.java).apply { + putExtra(org.schabi.newpipe.error.ReCaptchaActivity.RECAPTCHA_URL_EXTRA, url) + addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + } + assertEquals(url, intent.getStringExtra(org.schabi.newpipe.error.ReCaptchaActivity.RECAPTCHA_URL_EXTRA)) + } + + // Test 4: Retry functionality integration with ErrorPanel + @Test + fun testRetryIntegrationWithErrorPanel() { + val expectedMessage = "Network request failed" + val networkError = IOException(expectedMessage) + val loadStateError = LoadState.Error(networkError) + + val errorInfo = ErrorInfo( + throwable = loadStateError.error, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + + val errorAction = determineErrorAction(errorInfo) + assertEquals("Network errors should get REPORT action", ErrorAction.REPORT, errorAction) + var retryCallbackInvoked = false + val mockCommentRetry = { + retryCallbackInvoked = true + } + + mockCommentRetry() + assertTrue("Retry callback should be invoked when user clicks retry", retryCallbackInvoked) + + assertEquals( + "Error explanation should be available for retry scenarios", + expectedMessage, errorInfo.getExplanation() + ) + assertEquals( + "Error should maintain comment context for retry", + UserAction.REQUESTED_COMMENTS, errorInfo.userAction + ) + } +} diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt deleted file mode 100644 index d4cb2ab54..000000000 --- a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt +++ /dev/null @@ -1,65 +0,0 @@ -package org.schabi.newpipe.ui.components.video.comments - -import android.net.http.NetworkException -import androidx.paging.LoadState -import androidx.test.ext.junit.rules.ActivityScenarioRule -import androidx.test.ext.junit.runners.AndroidJUnit4 -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Rule -import org.junit.Test -import org.junit.runner.RunWith -import org.schabi.newpipe.MainActivity -import org.schabi.newpipe.R -import org.schabi.newpipe.ui.UiModel.UnableToLoadCommentsUiModel -import org.schabi.newpipe.viewmodels.util.Resource - -@RunWith(AndroidJUnit4::class) -class CommentSectionErrorTest { - - @get:Rule - val activityRule = ActivityScenarioRule(MainActivity::class.java) - - /** - * Test Resource.Error state - when initial comment info loading fails - */ - class TestNetworkException : NetworkException("Connection attempt timed out", null) { - override fun getErrorCode(): Int = NetworkException.ERROR_CONNECTION_TIMED_OUT - override fun isImmediatelyRetryable() = true - } - @Test - fun testResourceErrorState_ShowsUnableToLoadCommentsUiModel() { - - val networkException = TestNetworkException() - val errorResource = Resource.Error(networkException) - assertEquals("Should contain the network exception", networkException, errorResource.throwable) - - val errorModel = UnableToLoadCommentsUiModel(networkException) - val spec = errorModel.spec - - assertEquals("Should have correct message resource", R.string.error_unable_to_load_comments, spec.messageRes) - assertTrue("Should show retry button", spec.showRetry) - assertTrue("Should show report button", spec.showReport) - assertFalse("Should NOT show open in browser button", spec.showOpenInBrowser) - } - - /** - * Test LoadState.Error state - when paging data loading fails - */ - @Test - fun testLoadStateErrorState_ShowsUnableToLoadCommentsUiModel() { - val pagingException = RuntimeException("Paging data loading failed") - val loadStateError = LoadState.Error(pagingException) - - assertEquals("Should contain the paging exception", pagingException, loadStateError.error) - - val errorModel = UnableToLoadCommentsUiModel(pagingException) - val spec = errorModel.spec - - assertEquals("Should have correct message resource", R.string.error_unable_to_load_comments, spec.messageRes) - assertTrue("Should show retry button", spec.showRetry) - assertTrue("Should show report button", spec.showReport) - assertFalse("Should NOT show open in browser button", spec.showOpenInBrowser) - } -} diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt deleted file mode 100644 index 5df92ddfc..000000000 --- a/app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt +++ /dev/null @@ -1,14 +0,0 @@ -package org.schabi.newpipe.error - -import org.schabi.newpipe.ui.UiModel.ErrorUiModel -import org.schabi.newpipe.ui.UiModel.GenericErrorUiModel -import org.schabi.newpipe.ui.UiModel.UnableToLoadCommentsUiModel - -fun mapThrowableToErrorUiModel(throwable: Throwable, userAction: UserAction? = null): ErrorUiModel { - if (userAction == UserAction.REQUESTED_COMMENTS) { - - return UnableToLoadCommentsUiModel(rawError = throwable) - } - // Other ErrorInfo logic and throwable + user actions - return GenericErrorUiModel(rawError = throwable) -} diff --git a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt index bec4911df..21963640d 100644 --- a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt +++ b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt @@ -17,7 +17,7 @@ class CommentsSource(private val commentInfo: CommentInfo) : PagingSource): LoadResult { // params.key is null the first time the load() function is called, so we need to return the // first batch of already-loaded comments - return LoadResult.Error(IOException("💥 forced test error")) + if (params.key == null) { return LoadResult.Page(commentInfo.comments, null, commentInfo.nextPage) } else { diff --git a/app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt b/app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt deleted file mode 100644 index 6d91c78cd..000000000 --- a/app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt +++ /dev/null @@ -1,41 +0,0 @@ -package org.schabi.newpipe.ui.UiModel - -import androidx.compose.runtime.Immutable -import org.schabi.newpipe.R -import org.schabi.newpipe.ui.components.common.ErrorPanelSpec - -/** - * Each concrete case from this hierarchy represents a different failure state that the UI can render with ErrorPanel - */ -@Immutable -sealed interface ErrorUiModel { - val spec: ErrorPanelSpec - val rawError: Throwable? get() = null -} - -/** - * Concrete cases - Comments unable to load, Comments disabled, No connectivity, DNS failure, timeout etc - */ -@Immutable -data class UnableToLoadCommentsUiModel(override val rawError: Throwable?) : ErrorUiModel { - override val spec: ErrorPanelSpec = - ErrorPanelSpec( - messageRes = R.string.error_unable_to_load_comments, - showRetry = true, - showReport = true, - showOpenInBrowser = false - ) -} - -/** - * A generic ErrorUiModel for unhandled cases - */ -@Immutable -data class GenericErrorUiModel(override val rawError: Throwable?) : ErrorUiModel { - override val spec: ErrorPanelSpec = - ErrorPanelSpec( - messageRes = R.string.general_error, - showRetry = true, - showReport = true, - ) -} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt index 5f16941d9..93486f546 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -1,117 +1,132 @@ package org.schabi.newpipe.ui.components.common +import android.content.Intent import androidx.annotation.StringRes import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.wrapContentWidth import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.Preview import org.schabi.newpipe.R +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.ErrorUtil +import org.schabi.newpipe.error.ReCaptchaActivity +import org.schabi.newpipe.extractor.exceptions.AccountTerminatedException +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException +import org.schabi.newpipe.extractor.timeago.patterns.it import org.schabi.newpipe.ui.theme.AppTheme import org.schabi.newpipe.ui.theme.SizeTokens.SpacingExtraLarge -import org.schabi.newpipe.ui.theme.SizeTokens.SpacingLarge import org.schabi.newpipe.ui.theme.SizeTokens.SpacingMedium import org.schabi.newpipe.ui.theme.SizeTokens.SpacingSmall +import org.schabi.newpipe.util.external_communication.ShareUtils + +enum class ErrorAction(@StringRes val actionStringId: Int) { + REPORT(R.string.error_snackbar_action), + SOLVE_CAPTCHA(R.string.recaptcha_solve) +} + +/** + * Determines the error action type based on the throwable in ErrorInfo + * + */ +fun determineErrorAction(errorInfo: ErrorInfo): ErrorAction { + return when (errorInfo.throwable) { + is ReCaptchaException -> ErrorAction.SOLVE_CAPTCHA + is AccountTerminatedException -> ErrorAction.REPORT + else -> ErrorAction.REPORT + } +} @Composable fun ErrorPanel( - spec: ErrorPanelSpec, - onRetry: () -> Unit, - onReport: () -> Unit, - onOpenInBrowser: () -> Unit, + errorInfo: ErrorInfo, + onRetry: (() -> Unit)? = null, modifier: Modifier = Modifier ) { + val explanation = errorInfo.getExplanation() + val canOpenInBrowser = errorInfo.openInBrowserUrl != null + val errorActionType = determineErrorAction(errorInfo) + + val context = LocalContext.current + Column( horizontalAlignment = Alignment.CenterHorizontally, - modifier = Modifier - .wrapContentWidth() - .padding(horizontal = SpacingLarge, vertical = SpacingMedium) ) { - val message = stringResource(spec.messageRes) - Text( - text = message, + text = stringResource(errorInfo.messageStringId), style = MaterialTheme.typography.titleMedium.copy(fontWeight = FontWeight.Bold), textAlign = TextAlign.Center ) - spec.serviceInfoRes?.let { infoRes -> + if (explanation.isNotBlank()) { Spacer(Modifier.height(SpacingSmall)) - val serviceInfo = stringResource(infoRes) Text( - text = serviceInfo, + text = explanation, style = MaterialTheme.typography.bodyMedium, textAlign = TextAlign.Center ) } - spec.serviceExplanationRes?.let { explanationRes -> - Spacer(Modifier.height(SpacingSmall)) - val serviceExplanation = stringResource(explanationRes) - Text( - text = serviceExplanation, - style = MaterialTheme.typography.bodyMedium, - textAlign = TextAlign.Center - ) - } Spacer(Modifier.height(SpacingMedium)) - if (spec.showReport) { - ServiceColoredButton(onClick = onReport) { - Text(stringResource(R.string.error_snackbar_action).uppercase()) + when (errorActionType) { + ErrorAction.REPORT -> { + ServiceColoredButton(onClick = { + ErrorUtil.openActivity(context, errorInfo) + }) { + Text(stringResource(errorActionType.actionStringId).uppercase()) + } + } + ErrorAction.SOLVE_CAPTCHA -> { + ServiceColoredButton(onClick = { + // Starting ReCaptcha Challenge Activity + val intent = Intent(context, ReCaptchaActivity::class.java).apply { + putExtra( + ReCaptchaActivity.RECAPTCHA_URL_EXTRA, + (errorInfo.throwable as ReCaptchaException).url + ) + addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + } + context.startActivity(intent) + }) { + Text(stringResource(errorActionType.actionStringId).uppercase()) + } } } - if (spec.showRetry) { - ServiceColoredButton(onClick = onRetry) { + + onRetry?.let { + ServiceColoredButton(onClick = it) { Text(stringResource(R.string.retry).uppercase()) } } - if (spec.showOpenInBrowser) { - ServiceColoredButton(onClick = onOpenInBrowser) { + if (canOpenInBrowser) { + ServiceColoredButton(onClick = { + errorInfo.openInBrowserUrl?.let { url -> + ShareUtils.openUrlInBrowser(context, url) + } + }) { Text(stringResource(R.string.open_in_browser).uppercase()) } } + Spacer(Modifier.height(SpacingExtraLarge)) } } -data class ErrorPanelSpec( - @StringRes val messageRes: Int, - @StringRes val serviceInfoRes: Int? = null, - val serviceExplanation: String? = null, - @StringRes val serviceExplanationRes: Int? = null, - val showRetry: Boolean = false, - val showReport: Boolean = false, - val showOpenInBrowser: Boolean = false -) - @Preview(showBackground = true, widthDp = 360, heightDp = 640) @Composable fun ErrorPanelPreview() { AppTheme { - ErrorPanel( - spec = ErrorPanelSpec( - messageRes = android.R.string.httpErrorBadUrl, - showRetry = true, - showReport = false, - showOpenInBrowser = false - ), - onRetry = {}, - onReport = {}, - onOpenInBrowser = {}, - modifier = Modifier - ) } } diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt deleted file mode 100644 index 152c032fd..000000000 --- a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt +++ /dev/null @@ -1,64 +0,0 @@ -package org.schabi.newpipe.ui.components.video.comment - -import android.util.Log -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.wrapContentHeight -import androidx.compose.runtime.Composable -import androidx.compose.runtime.SideEffect -import androidx.compose.ui.Alignment -import androidx.compose.ui.Modifier -import androidx.compose.ui.input.nestedscroll.NestedScrollSource.Companion.SideEffect -import androidx.compose.ui.tooling.preview.Preview -import org.schabi.newpipe.error.ErrorInfo -import org.schabi.newpipe.error.UserAction -import org.schabi.newpipe.error.mapThrowableToErrorUiModel -import org.schabi.newpipe.ui.UiModel.ErrorUiModel -import org.schabi.newpipe.ui.components.common.ErrorPanel -import java.io.IOException - -@Composable -fun CommentErrorHandler( - throwable: Throwable, - userAction: UserAction, - onRetry: () -> Unit, - onReport: (ErrorInfo) -> Unit -) { - SideEffect { - Log.d("CommentErrorHandler", "⛔️ rendered for error: ${throwable.message}") - } - - val uiModel: ErrorUiModel = mapThrowableToErrorUiModel(throwable, userAction) - val errorInfo = ErrorInfo( - throwable = throwable, - userAction = userAction, - request = "" - ) - - Box( - modifier = Modifier.fillMaxSize(), - contentAlignment = Alignment.Center - ) { - ErrorPanel( - spec = uiModel.spec, - onRetry = onRetry, - onReport = { onReport(errorInfo) }, - onOpenInBrowser = {}, - modifier = Modifier - .fillMaxWidth() - .wrapContentHeight() - ) - } -} - -@Preview(showBackground = true) -@Composable -fun PreviewCommentErrorHandler() { - CommentErrorHandler( - throwable = IOException("No network"), - userAction = UserAction.REQUESTED_COMMENTS, - onRetry = {}, - onReport = {} - ) -} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt index 327d30f72..9a570c82c 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt @@ -1,6 +1,8 @@ package org.schabi.newpipe.ui.components.video.comment import android.content.res.Configuration +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding @@ -11,6 +13,7 @@ import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.platform.LocalContext @@ -26,11 +29,12 @@ import androidx.paging.compose.collectAsLazyPagingItems import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flowOf import org.schabi.newpipe.R -import org.schabi.newpipe.error.ErrorUtil +import org.schabi.newpipe.error.ErrorInfo import org.schabi.newpipe.error.UserAction import org.schabi.newpipe.extractor.Page import org.schabi.newpipe.extractor.comments.CommentsInfoItem import org.schabi.newpipe.extractor.stream.Description +import org.schabi.newpipe.ui.components.common.ErrorPanel import org.schabi.newpipe.ui.components.common.LazyColumnThemedScrollbar import org.schabi.newpipe.ui.components.common.LoadingIndicator import org.schabi.newpipe.ui.emptystate.EmptyStateComposable @@ -78,6 +82,7 @@ private fun CommentSection( modifier = Modifier .fillMaxWidth() .heightIn(min = 128.dp) + ) } } else if (count == 0) { @@ -109,13 +114,25 @@ private fun CommentSection( } } is LoadState.Error -> { + val error = (comments.loadState.refresh as LoadState.Error).error + val errorInfo = ErrorInfo( + throwable = error, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + item { - CommentErrorHandler( - throwable = (comments.loadState.refresh as LoadState.Error).error, - userAction = UserAction.REQUESTED_COMMENTS, - onRetry = { comments.retry() }, - onReport = { info -> ErrorUtil.openActivity(context, info) }, - ) + Box( + modifier = Modifier + .fillMaxWidth() + + ) { + ErrorPanel( + errorInfo = errorInfo, + onRetry = { comments.retry() }, + modifier = Modifier.align(Alignment.Center) + ) + } } } else -> { @@ -127,13 +144,23 @@ private fun CommentSection( } } is Resource.Error -> { + val errorInfo = ErrorInfo( + throwable = uiState.throwable, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) item { - CommentErrorHandler( - throwable = uiState.throwable, - userAction = UserAction.REQUESTED_COMMENTS, - onRetry = { comments.retry() }, - onReport = { info -> ErrorUtil.openActivity(context, info) }, - ) + Box( + modifier = Modifier + .fillMaxSize(), + contentAlignment = Alignment.Center + ) { + ErrorPanel( + errorInfo = errorInfo, + onRetry = { comments.retry() }, + modifier = Modifier.align(Alignment.Center) + ) + } } } } diff --git a/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt b/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt index 58886fc68..007292498 100644 --- a/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt +++ b/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt @@ -23,7 +23,6 @@ import org.schabi.newpipe.viewmodels.util.Resource class CommentsViewModel(savedStateHandle: SavedStateHandle) : ViewModel() { val uiState = savedStateHandle.getStateFlow(KEY_URL, "") .map { - // Resource.Error(RuntimeException("Forced error for testing")) try { Resource.Success(CommentInfo(CommentsInfo.getInfo(it))) } catch (e: Exception) { diff --git a/app/src/main/res/layout/fragment_video_detail.xml b/app/src/main/res/layout/fragment_video_detail.xml index 1a4711581..d7e2e5a3c 100644 --- a/app/src/main/res/layout/fragment_video_detail.xml +++ b/app/src/main/res/layout/fragment_video_detail.xml @@ -214,6 +214,14 @@ android:layout_marginTop="@dimen/video_item_detail_error_panel_margin" android:visibility="gone" tools:visibility="gone" /> + Date: Mon, 4 Aug 2025 16:45:12 -0400 Subject: [PATCH 04/15] Update app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt Co-authored-by: Isira Seneviratne <31027858+Isira-Seneviratne@users.noreply.github.com> --- .../org/schabi/newpipe/ui/components/common/ErrorPanel.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt index 93486f546..9fe521651 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -90,13 +90,12 @@ fun ErrorPanel( ErrorAction.SOLVE_CAPTCHA -> { ServiceColoredButton(onClick = { // Starting ReCaptcha Challenge Activity - val intent = Intent(context, ReCaptchaActivity::class.java).apply { - putExtra( + val intent = Intent(context, ReCaptchaActivity::class.java) + .putExtra( ReCaptchaActivity.RECAPTCHA_URL_EXTRA, (errorInfo.throwable as ReCaptchaException).url ) - addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - } + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) context.startActivity(intent) }) { Text(stringResource(errorActionType.actionStringId).uppercase()) From 848b8868fbd9868ea7ac155a80093c4dd08c0c5c Mon Sep 17 00:00:00 2001 From: Stt_lens <82180165+SttApollo@users.noreply.github.com> Date: Mon, 4 Aug 2025 16:51:51 -0400 Subject: [PATCH 05/15] Update app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt Co-authored-by: Isira Seneviratne <31027858+Isira-Seneviratne@users.noreply.github.com> --- .../newpipe/ui/components/video/comment/CommentSection.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt index 9a570c82c..0ad23b437 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt @@ -107,7 +107,7 @@ private fun CommentSection( ) } } - when (comments.loadState.refresh) { + when (val refresh = comments.loadState.refresh) { is LoadState.Loading -> { item { LoadingIndicator(modifier = Modifier.padding(top = 8.dp)) From 9cbcaecb92c6d0c4e737ccd712971b75abd84dc1 Mon Sep 17 00:00:00 2001 From: Stt_lens <82180165+SttApollo@users.noreply.github.com> Date: Mon, 4 Aug 2025 16:51:57 -0400 Subject: [PATCH 06/15] Update app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt Co-authored-by: Isira Seneviratne <31027858+Isira-Seneviratne@users.noreply.github.com> --- .../newpipe/ui/components/video/comment/CommentSection.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt index 0ad23b437..a774a0114 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt @@ -114,9 +114,8 @@ private fun CommentSection( } } is LoadState.Error -> { - val error = (comments.loadState.refresh as LoadState.Error).error val errorInfo = ErrorInfo( - throwable = error, + throwable = refresh.error, userAction = UserAction.REQUESTED_COMMENTS, request = "comments" ) From 2d3a4b67d74556646ea9940426f096d20aa36b96 Mon Sep 17 00:00:00 2001 From: Su TT Date: Sun, 10 Aug 2025 18:12:55 -0400 Subject: [PATCH 07/15] Convert CommentSectionErrorIntegrationTest to unit test - Moved from androidTest to test directory - Removed Android test runner dependencies - Renamed to CommentSectionErrorTest - Addresses PR feedback until Compose testing is in place --- .../CommentSectionErrorIntegrationTest.kt | 119 ------------------ .../schabi/newpipe/paging/CommentsSource.kt | 1 - .../ui/components/common/ErrorPanel.kt | 3 +- .../common/CommentSectionErrorTest.kt | 61 +++++++++ 4 files changed, 62 insertions(+), 122 deletions(-) delete mode 100644 app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionErrorIntegrationTest.kt create mode 100644 app/src/test/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionErrorIntegrationTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionErrorIntegrationTest.kt deleted file mode 100644 index 12bcd85ac..000000000 --- a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionErrorIntegrationTest.kt +++ /dev/null @@ -1,119 +0,0 @@ -package org.schabi.newpipe.ui.components.video.comment - -import android.content.Context -import android.content.Intent -import androidx.paging.LoadState -import androidx.test.core.app.ApplicationProvider -import androidx.test.ext.junit.runners.AndroidJUnit4 -import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue -import org.junit.Before -import org.junit.Test -import org.junit.runner.RunWith -import org.schabi.newpipe.error.ErrorInfo -import org.schabi.newpipe.error.UserAction -import org.schabi.newpipe.extractor.exceptions.ReCaptchaException -import org.schabi.newpipe.ui.components.common.ErrorAction -import org.schabi.newpipe.ui.components.common.determineErrorAction -import org.schabi.newpipe.viewmodels.util.Resource -import java.io.IOException -import java.net.SocketTimeoutException - -@RunWith(AndroidJUnit4::class) -class CommentSectionErrorIntegrationTest { - - private lateinit var context: Context - - @Before - fun setUp() { - context = ApplicationProvider.getApplicationContext() - } - - // Test 1: Network error on initial load (Resource.Error) - @Test - fun testInitialCommentNetworkError() { - val expectedMessage = "Connection timeout" - val networkError = SocketTimeoutException(expectedMessage) - val resourceError = Resource.Error(networkError) - - val errorInfo = ErrorInfo( - throwable = resourceError.throwable, - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - assertEquals(networkError, errorInfo.throwable) - assertEquals(ErrorAction.REPORT, determineErrorAction(errorInfo)) - assertEquals(expectedMessage, errorInfo.getExplanation()) - } - - // Test 2: Network error on paging (LoadState.Error) - @Test - fun testPagingNetworkError() { - val expectedMessage = "Paging failed" - val pagingError = IOException(expectedMessage) - val loadStateError = LoadState.Error(pagingError) - - val errorInfo = ErrorInfo( - throwable = loadStateError.error, - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - assertEquals(pagingError, errorInfo.throwable) - assertEquals(ErrorAction.REPORT, determineErrorAction(errorInfo)) - assertEquals(expectedMessage, errorInfo.getExplanation()) - } - - // Test 3: ReCaptcha during comments load - @Test - fun testReCaptchaDuringComments() { - val url = "https://www.google.com/recaptcha/api/fallback?k=test" - val expectedMessage = "ReCaptcha needed" - val captcha = ReCaptchaException(expectedMessage, url) - val errorInfo = ErrorInfo( - throwable = captcha, - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - assertEquals(ErrorAction.SOLVE_CAPTCHA, determineErrorAction(errorInfo)) - assertEquals(expectedMessage, errorInfo.getExplanation()) - - val intent = Intent(context, org.schabi.newpipe.error.ReCaptchaActivity::class.java).apply { - putExtra(org.schabi.newpipe.error.ReCaptchaActivity.RECAPTCHA_URL_EXTRA, url) - addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - } - assertEquals(url, intent.getStringExtra(org.schabi.newpipe.error.ReCaptchaActivity.RECAPTCHA_URL_EXTRA)) - } - - // Test 4: Retry functionality integration with ErrorPanel - @Test - fun testRetryIntegrationWithErrorPanel() { - val expectedMessage = "Network request failed" - val networkError = IOException(expectedMessage) - val loadStateError = LoadState.Error(networkError) - - val errorInfo = ErrorInfo( - throwable = loadStateError.error, - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - - val errorAction = determineErrorAction(errorInfo) - assertEquals("Network errors should get REPORT action", ErrorAction.REPORT, errorAction) - var retryCallbackInvoked = false - val mockCommentRetry = { - retryCallbackInvoked = true - } - - mockCommentRetry() - assertTrue("Retry callback should be invoked when user clicks retry", retryCallbackInvoked) - - assertEquals( - "Error explanation should be available for retry scenarios", - expectedMessage, errorInfo.getExplanation() - ) - assertEquals( - "Error should maintain comment context for retry", - UserAction.REQUESTED_COMMENTS, errorInfo.userAction - ) - } -} diff --git a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt index 21963640d..20d67a283 100644 --- a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt +++ b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt @@ -9,7 +9,6 @@ import org.schabi.newpipe.extractor.Page import org.schabi.newpipe.extractor.comments.CommentsInfo import org.schabi.newpipe.extractor.comments.CommentsInfoItem import org.schabi.newpipe.ui.components.video.comment.CommentInfo -import java.io.IOException class CommentsSource(private val commentInfo: CommentInfo) : PagingSource() { private val service = NewPipe.getService(commentInfo.serviceId) diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt index 9fe521651..5224c2c2f 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -21,7 +21,6 @@ import org.schabi.newpipe.error.ErrorUtil import org.schabi.newpipe.error.ReCaptchaActivity import org.schabi.newpipe.extractor.exceptions.AccountTerminatedException import org.schabi.newpipe.extractor.exceptions.ReCaptchaException -import org.schabi.newpipe.extractor.timeago.patterns.it import org.schabi.newpipe.ui.theme.AppTheme import org.schabi.newpipe.ui.theme.SizeTokens.SpacingExtraLarge import org.schabi.newpipe.ui.theme.SizeTokens.SpacingMedium @@ -48,8 +47,8 @@ fun determineErrorAction(errorInfo: ErrorInfo): ErrorAction { @Composable fun ErrorPanel( errorInfo: ErrorInfo, + modifier: Modifier = Modifier, onRetry: (() -> Unit)? = null, - modifier: Modifier = Modifier ) { val explanation = errorInfo.getExplanation() diff --git a/app/src/test/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt b/app/src/test/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt new file mode 100644 index 000000000..42e45a3c5 --- /dev/null +++ b/app/src/test/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt @@ -0,0 +1,61 @@ +package org.schabi.newpipe.ui.components.common + +import androidx.paging.LoadState +import org.junit.Assert +import org.junit.Test +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.UserAction +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException +import java.io.IOException +import java.net.SocketTimeoutException + +class CommentSectionErrorTest { + + // Test 1: Network error on initial load (Resource.Error) + @Test + fun testInitialCommentNetworkError() { + val expectedMessage = "Connection timeout" + val networkError = SocketTimeoutException(expectedMessage) + + val errorInfo = ErrorInfo( + throwable = networkError, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + Assert.assertEquals(networkError, errorInfo.throwable) + Assert.assertEquals(ErrorAction.REPORT, determineErrorAction(errorInfo)) + Assert.assertEquals(expectedMessage, errorInfo.getExplanation()) + } + + // Test 2: Network error on paging (LoadState.Error) + @Test + fun testPagingNetworkError() { + val expectedMessage = "Paging failed" + val pagingError = IOException(expectedMessage) + val loadStateError = LoadState.Error(pagingError) + + val errorInfo = ErrorInfo( + throwable = loadStateError.error, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + Assert.assertEquals(pagingError, errorInfo.throwable) + Assert.assertEquals(ErrorAction.REPORT, determineErrorAction(errorInfo)) + Assert.assertEquals(expectedMessage, errorInfo.getExplanation()) + } + + // Test 3: ReCaptcha during comments load + @Test + fun testReCaptchaDuringComments() { + val url = "https://www.google.com/recaptcha/api/fallback?k=test" + val expectedMessage = "ReCaptcha needed" + val captcha = ReCaptchaException(expectedMessage, url) + val errorInfo = ErrorInfo( + throwable = captcha, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + Assert.assertEquals(ErrorAction.SOLVE_CAPTCHA, determineErrorAction(errorInfo)) + Assert.assertEquals(expectedMessage, errorInfo.getExplanation()) + } +} From 5ba95a2c37b790aecf81c8e36caf6520b4e0f7d1 Mon Sep 17 00:00:00 2001 From: Su TT Date: Mon, 11 Aug 2025 11:13:59 -0400 Subject: [PATCH 08/15] Add ErrorPanelPreview --- .../schabi/newpipe/ui/components/common/ErrorPanel.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt index 5224c2c2f..de6dd096e 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -59,7 +59,7 @@ fun ErrorPanel( Column( horizontalAlignment = Alignment.CenterHorizontally, - + modifier = modifier ) { Text( @@ -126,5 +126,12 @@ fun ErrorPanel( @Composable fun ErrorPanelPreview() { AppTheme { + ErrorPanel( + errorInfo = ErrorInfo( + throwable = Exception("Network error"), + userAction = org.schabi.newpipe.error.UserAction.UI_ERROR, + request = "Preview request" + ) + ) } } From fab0d35269787a252901a274dad0739f71fea309 Mon Sep 17 00:00:00 2001 From: Su TT Date: Wed, 24 Sep 2025 16:47:21 -0400 Subject: [PATCH 09/15] Update ErrorPanel and retest --- .../common/CommentSectionErrorTest.kt | 61 ++++++++++ .../ui/components/common/ErrorPanel.kt | 110 +++++++----------- .../common/CommentSectionErrorTest.kt | 61 ---------- 3 files changed, 100 insertions(+), 132 deletions(-) create mode 100644 app/src/androidTest/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt delete mode 100644 app/src/test/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt new file mode 100644 index 000000000..010cab7a7 --- /dev/null +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt @@ -0,0 +1,61 @@ +package org.schabi.newpipe.ui.components.common + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert +import org.junit.Test +import org.junit.runner.RunWith +import org.schabi.newpipe.R +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.UserAction +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException +import java.io.IOException +import java.net.SocketTimeoutException + +@RunWith(AndroidJUnit4::class) +class CommentSectionErrorTest { + private val context: Context by lazy { ApplicationProvider.getApplicationContext() } + // Test 1: Network error on initial load (Resource.Error) + @Test + fun testInitialCommentNetworkError() { + val errorInfo = ErrorInfo( + throwable = SocketTimeoutException("Connection timeout"), + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + Assert.assertEquals(context.getString(R.string.network_error), errorInfo.getMessage(context)) + Assert.assertTrue(errorInfo.isReportable) + Assert.assertTrue(errorInfo.isRetryable) + Assert.assertNull(errorInfo.recaptchaUrl) + } + + // Test 2: Network error on paging (LoadState.Error) + @Test + fun testPagingNetworkError() { + val errorInfo = ErrorInfo( + throwable = IOException("Paging failed"), + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + Assert.assertEquals(context.getString(R.string.network_error), errorInfo.getMessage(context)) + Assert.assertTrue(errorInfo.isReportable) + Assert.assertTrue(errorInfo.isRetryable) + Assert.assertNull(errorInfo.recaptchaUrl) + } + + // Test 3: ReCaptcha during comments load + @Test + fun testReCaptchaDuringComments() { + val url = "https://www.google.com/recaptcha/api/fallback?k=test" + val errorInfo = ErrorInfo( + throwable = ReCaptchaException("ReCaptcha needed", url), + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + Assert.assertEquals(context.getString(R.string.recaptcha_request_toast), errorInfo.getMessage(context)) + Assert.assertEquals(url, errorInfo.recaptchaUrl) + Assert.assertTrue(errorInfo.isReportable) + Assert.assertTrue(errorInfo.isRetryable) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt index de6dd096e..3a2771a6f 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -1,7 +1,6 @@ package org.schabi.newpipe.ui.components.common import android.content.Intent -import androidx.annotation.StringRes import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.height @@ -11,6 +10,7 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.LocalInspectionMode import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextAlign @@ -19,31 +19,10 @@ import org.schabi.newpipe.R import org.schabi.newpipe.error.ErrorInfo import org.schabi.newpipe.error.ErrorUtil import org.schabi.newpipe.error.ReCaptchaActivity -import org.schabi.newpipe.extractor.exceptions.AccountTerminatedException -import org.schabi.newpipe.extractor.exceptions.ReCaptchaException import org.schabi.newpipe.ui.theme.AppTheme -import org.schabi.newpipe.ui.theme.SizeTokens.SpacingExtraLarge -import org.schabi.newpipe.ui.theme.SizeTokens.SpacingMedium -import org.schabi.newpipe.ui.theme.SizeTokens.SpacingSmall +import org.schabi.newpipe.ui.theme.SizeTokens import org.schabi.newpipe.util.external_communication.ShareUtils -enum class ErrorAction(@StringRes val actionStringId: Int) { - REPORT(R.string.error_snackbar_action), - SOLVE_CAPTCHA(R.string.recaptcha_solve) -} - -/** - * Determines the error action type based on the throwable in ErrorInfo - * - */ -fun determineErrorAction(errorInfo: ErrorInfo): ErrorAction { - return when (errorInfo.throwable) { - is ReCaptchaException -> ErrorAction.SOLVE_CAPTCHA - is AccountTerminatedException -> ErrorAction.REPORT - else -> ErrorAction.REPORT - } -} - @Composable fun ErrorPanel( errorInfo: ErrorInfo, @@ -51,11 +30,13 @@ fun ErrorPanel( onRetry: (() -> Unit)? = null, ) { - val explanation = errorInfo.getExplanation() - val canOpenInBrowser = errorInfo.openInBrowserUrl != null - val errorActionType = determineErrorAction(errorInfo) - val context = LocalContext.current + val isPreview = LocalInspectionMode.current + val messageText = if (isPreview) { + stringResource(R.string.error_snackbar_message) + } else { + errorInfo.getMessage(context) + } Column( horizontalAlignment = Alignment.CenterHorizontally, @@ -63,61 +44,48 @@ fun ErrorPanel( ) { Text( - text = stringResource(errorInfo.messageStringId), + text = messageText, style = MaterialTheme.typography.titleMedium.copy(fontWeight = FontWeight.Bold), textAlign = TextAlign.Center ) - if (explanation.isNotBlank()) { - Spacer(Modifier.height(SpacingSmall)) - Text( - text = explanation, - style = MaterialTheme.typography.bodyMedium, - textAlign = TextAlign.Center - ) - } - - Spacer(Modifier.height(SpacingMedium)) - when (errorActionType) { - ErrorAction.REPORT -> { - ServiceColoredButton(onClick = { - ErrorUtil.openActivity(context, errorInfo) - }) { - Text(stringResource(errorActionType.actionStringId).uppercase()) - } - } - ErrorAction.SOLVE_CAPTCHA -> { - ServiceColoredButton(onClick = { - // Starting ReCaptcha Challenge Activity - val intent = Intent(context, ReCaptchaActivity::class.java) - .putExtra( - ReCaptchaActivity.RECAPTCHA_URL_EXTRA, - (errorInfo.throwable as ReCaptchaException).url - ) - .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - context.startActivity(intent) - }) { - Text(stringResource(errorActionType.actionStringId).uppercase()) - } - } - } - - onRetry?.let { - ServiceColoredButton(onClick = it) { - Text(stringResource(R.string.retry).uppercase()) - } - } - if (canOpenInBrowser) { + Spacer(Modifier.height(SizeTokens.SpacingMedium)) + if (errorInfo.isReportable) { ServiceColoredButton(onClick = { - errorInfo.openInBrowserUrl?.let { url -> - ShareUtils.openUrlInBrowser(context, url) + ErrorUtil.openActivity(context, errorInfo) + }) { + Text(stringResource(R.string.error_snackbar_action).uppercase()) + } + } + + errorInfo.recaptchaUrl?.let { recaptchaUrl -> + ServiceColoredButton(onClick = { + val intent = Intent(context, ReCaptchaActivity::class.java) + .putExtra(ReCaptchaActivity.RECAPTCHA_URL_EXTRA, recaptchaUrl) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + context.startActivity(intent) + }) { + Text(stringResource(R.string.recaptcha_solve).uppercase()) + } + } + + if (errorInfo.isRetryable) { + onRetry?.let { + ServiceColoredButton(onClick = it) { + Text(stringResource(R.string.retry).uppercase()) } + } + } + + errorInfo.openInBrowserUrl?.let { url -> + ServiceColoredButton(onClick = { + ShareUtils.openUrlInBrowser(context, url) }) { Text(stringResource(R.string.open_in_browser).uppercase()) } } - Spacer(Modifier.height(SpacingExtraLarge)) + Spacer(Modifier.height(SizeTokens.SpacingExtraLarge)) } } diff --git a/app/src/test/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt b/app/src/test/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt deleted file mode 100644 index 42e45a3c5..000000000 --- a/app/src/test/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt +++ /dev/null @@ -1,61 +0,0 @@ -package org.schabi.newpipe.ui.components.common - -import androidx.paging.LoadState -import org.junit.Assert -import org.junit.Test -import org.schabi.newpipe.error.ErrorInfo -import org.schabi.newpipe.error.UserAction -import org.schabi.newpipe.extractor.exceptions.ReCaptchaException -import java.io.IOException -import java.net.SocketTimeoutException - -class CommentSectionErrorTest { - - // Test 1: Network error on initial load (Resource.Error) - @Test - fun testInitialCommentNetworkError() { - val expectedMessage = "Connection timeout" - val networkError = SocketTimeoutException(expectedMessage) - - val errorInfo = ErrorInfo( - throwable = networkError, - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - Assert.assertEquals(networkError, errorInfo.throwable) - Assert.assertEquals(ErrorAction.REPORT, determineErrorAction(errorInfo)) - Assert.assertEquals(expectedMessage, errorInfo.getExplanation()) - } - - // Test 2: Network error on paging (LoadState.Error) - @Test - fun testPagingNetworkError() { - val expectedMessage = "Paging failed" - val pagingError = IOException(expectedMessage) - val loadStateError = LoadState.Error(pagingError) - - val errorInfo = ErrorInfo( - throwable = loadStateError.error, - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - Assert.assertEquals(pagingError, errorInfo.throwable) - Assert.assertEquals(ErrorAction.REPORT, determineErrorAction(errorInfo)) - Assert.assertEquals(expectedMessage, errorInfo.getExplanation()) - } - - // Test 3: ReCaptcha during comments load - @Test - fun testReCaptchaDuringComments() { - val url = "https://www.google.com/recaptcha/api/fallback?k=test" - val expectedMessage = "ReCaptcha needed" - val captcha = ReCaptchaException(expectedMessage, url) - val errorInfo = ErrorInfo( - throwable = captcha, - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - Assert.assertEquals(ErrorAction.SOLVE_CAPTCHA, determineErrorAction(errorInfo)) - Assert.assertEquals(expectedMessage, errorInfo.getExplanation()) - } -} From 3ab1322425fe49c76fb2938221eed71ef01a049e Mon Sep 17 00:00:00 2001 From: Su TT Date: Mon, 29 Sep 2025 13:48:03 -0400 Subject: [PATCH 10/15] test,ui: move comment error tests to error; remove unused ComposeView --- .gitignore | 3 +++ .../ErrorInfoCommentsTest.kt} | 6 ++---- .../ui/components/video/comment/CommentSection.kt | 2 -- app/src/main/res/layout/fragment_video_detail.xml | 9 --------- 4 files changed, 5 insertions(+), 15 deletions(-) rename app/src/androidTest/java/org/schabi/newpipe/{ui/components/common/CommentSectionErrorTest.kt => error/ErrorInfoCommentsTest.kt} (93%) diff --git a/.gitignore b/.gitignore index 7bccc3132..a5219720c 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,6 @@ app/release/ bin/ .vscode/ *.code-workspace + +# logs +*.log diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt similarity index 93% rename from app/src/androidTest/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt rename to app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt index 010cab7a7..454964b44 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/ui/components/common/CommentSectionErrorTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt @@ -1,4 +1,4 @@ -package org.schabi.newpipe.ui.components.common +package org.schabi.newpipe.error import android.content.Context import androidx.test.core.app.ApplicationProvider @@ -7,14 +7,12 @@ import org.junit.Assert import org.junit.Test import org.junit.runner.RunWith import org.schabi.newpipe.R -import org.schabi.newpipe.error.ErrorInfo -import org.schabi.newpipe.error.UserAction import org.schabi.newpipe.extractor.exceptions.ReCaptchaException import java.io.IOException import java.net.SocketTimeoutException @RunWith(AndroidJUnit4::class) -class CommentSectionErrorTest { +class ErrorInfoCommentsTest { private val context: Context by lazy { ApplicationProvider.getApplicationContext() } // Test 1: Network error on initial load (Resource.Error) @Test diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt index a774a0114..23f13a342 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt @@ -16,7 +16,6 @@ import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll -import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.rememberNestedScrollInteropConnection import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.tooling.preview.Preview @@ -57,7 +56,6 @@ private fun CommentSection( val comments = commentsFlow.collectAsLazyPagingItems() val nestedScrollInterop = rememberNestedScrollInteropConnection() val state = rememberLazyListState() - val context = LocalContext.current LazyColumnThemedScrollbar(state = state) { LazyColumn( diff --git a/app/src/main/res/layout/fragment_video_detail.xml b/app/src/main/res/layout/fragment_video_detail.xml index d7e2e5a3c..abf1509b1 100644 --- a/app/src/main/res/layout/fragment_video_detail.xml +++ b/app/src/main/res/layout/fragment_video_detail.xml @@ -214,15 +214,6 @@ android:layout_marginTop="@dimen/video_item_detail_error_panel_margin" android:visibility="gone" tools:visibility="gone" /> - - Date: Fri, 5 Sep 2025 17:16:43 +0200 Subject: [PATCH 11/15] Unify getString for compatibility (read the method's javadoc for why) --- .../org/schabi/newpipe/error/ErrorInfo.kt | 9 ++-- .../MediaBrowserPlaybackPreparer.kt | 4 +- .../settings/viewmodel/SettingsViewModel.kt | 7 ++-- .../org/schabi/newpipe/util/Localization.java | 41 +++++++++++++++++++ .../schabi/newpipe/util/NavigationHelper.java | 7 ++-- 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt index 609fbb336..a3c0aa993 100644 --- a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt +++ b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt @@ -28,6 +28,7 @@ import org.schabi.newpipe.extractor.exceptions.YoutubeMusicPremiumContentExcepti import org.schabi.newpipe.ktx.isNetworkRelated import org.schabi.newpipe.player.mediasource.FailedMediaSource import org.schabi.newpipe.player.resolver.PlaybackResolver +import org.schabi.newpipe.util.Localization import java.net.UnknownHostException /** @@ -147,13 +148,11 @@ class ErrorInfo private constructor( private vararg val formatArgs: String, ) : Parcelable { fun getString(context: Context): String { + // use Localization.compatGetString() just in case context is not AppCompatActivity return if (formatArgs.isEmpty()) { - // use ContextCompat.getString() just in case context is not AppCompatActivity - ContextCompat.getString(context, stringRes) + Localization.compatGetString(context, stringRes) } else { - // ContextCompat.getString() with formatArgs does not exist, so we just - // replicate its source code but with formatArgs - ContextCompat.getContextForLanguage(context).getString(stringRes, *formatArgs) + Localization.compatGetString(context, stringRes, *formatArgs) } } } diff --git a/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt b/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt index 4815965a3..fbd506456 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt +++ b/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt @@ -6,7 +6,6 @@ import android.os.Bundle import android.os.ResultReceiver import android.support.v4.media.session.PlaybackStateCompat import android.util.Log -import androidx.core.content.ContextCompat import androidx.core.net.toUri import com.google.android.exoplayer2.Player import com.google.android.exoplayer2.ext.mediasession.MediaSessionConnector.PlaybackPreparer @@ -29,6 +28,7 @@ import org.schabi.newpipe.player.playqueue.PlaylistPlayQueue import org.schabi.newpipe.player.playqueue.SinglePlayQueue import org.schabi.newpipe.util.ChannelTabHelper import org.schabi.newpipe.util.ExtractorHelper +import org.schabi.newpipe.util.Localization import org.schabi.newpipe.util.NavigationHelper import java.util.function.BiConsumer import java.util.function.Consumer @@ -111,7 +111,7 @@ class MediaBrowserPlaybackPreparer( //region Errors private fun onUnsupportedError() { setMediaSessionError.accept( - ContextCompat.getString(context, R.string.content_not_supported), + Localization.compatGetString(context, R.string.content_not_supported), PlaybackStateCompat.ERROR_CODE_NOT_SUPPORTED ) } diff --git a/app/src/main/java/org/schabi/newpipe/settings/viewmodel/SettingsViewModel.kt b/app/src/main/java/org/schabi/newpipe/settings/viewmodel/SettingsViewModel.kt index ae3520c94..1e48fef5e 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/viewmodel/SettingsViewModel.kt +++ b/app/src/main/java/org/schabi/newpipe/settings/viewmodel/SettingsViewModel.kt @@ -3,13 +3,13 @@ package org.schabi.newpipe.settings.viewmodel import android.app.Application import android.content.Context import android.content.SharedPreferences -import androidx.core.content.ContextCompat import androidx.lifecycle.AndroidViewModel import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import org.schabi.newpipe.R +import org.schabi.newpipe.util.Localization import javax.inject.Inject @HiltViewModel @@ -20,11 +20,12 @@ class SettingsViewModel @Inject constructor( private var _settingsLayoutRedesignPref: Boolean get() = preferenceManager.getBoolean( - ContextCompat.getString(getApplication(), R.string.settings_layout_redesign_key), false + Localization.compatGetString(getApplication(), R.string.settings_layout_redesign_key), + false ) set(value) { preferenceManager.edit().putBoolean( - ContextCompat.getString(getApplication(), R.string.settings_layout_redesign_key), + Localization.compatGetString(getApplication(), R.string.settings_layout_redesign_key), value ).apply() } diff --git a/app/src/main/java/org/schabi/newpipe/util/Localization.java b/app/src/main/java/org/schabi/newpipe/util/Localization.java index 890981e90..f5bcc40d3 100644 --- a/app/src/main/java/org/schabi/newpipe/util/Localization.java +++ b/app/src/main/java/org/schabi/newpipe/util/Localization.java @@ -18,6 +18,7 @@ import androidx.annotation.Nullable; import androidx.annotation.PluralsRes; import androidx.annotation.StringRes; import androidx.appcompat.app.AppCompatDelegate; +import androidx.core.content.ContextCompat; import androidx.core.math.MathUtils; import androidx.core.os.LocaleListCompat; import androidx.preference.PreferenceManager; @@ -71,6 +72,46 @@ public final class Localization { private Localization() { } + /** + * Gets a string like you would normally do with {@link Context#getString}, except that when + * Context is not an AppCompatActivity the correct locale is still used. The latter step uses + * {@link ContextCompat#getString}, which might fail if the Locale system service is not + * available (e.g. inside of Compose previews). In that case this method falls back to plain old + * {@link Context#getString}. + *

This method also supports format args (see {@link #compatGetString(Context, int, + * Object...)}, unlike {@link ContextCompat#getString}.

+ * + * @param context any Android context, even the App context + * @param resId the string resource to resolve + * @return the resolved string + */ + public static String compatGetString(final Context context, @StringRes final int resId) { + try { + return ContextCompat.getString(context, resId); + } catch (final Throwable e) { + return context.getString(resId); + } + } + + /** + * @see #compatGetString(Context, int) + * @param context any Android context, even the App context + * @param resId the string resource to resolve + * @param formatArgs the formatting arguments + * @return the resolved string + */ + public static String compatGetString(final Context context, + @StringRes final int resId, + final Object... formatArgs) { + try { + // ContextCompat.getString() with formatArgs does not exist, so we just + // replicate its source code but with formatArgs + return ContextCompat.getContextForLanguage(context).getString(resId, formatArgs); + } catch (final Throwable e) { + return context.getString(resId, formatArgs); + } + } + @NonNull public static String concatenateStrings(final String... strings) { return concatenateStrings(DOT_SEPARATOR, Arrays.asList(strings)); diff --git a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java index cc260d254..0a7906b8d 100644 --- a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java @@ -641,10 +641,9 @@ public final class NavigationHelper { public static void openSettings(final Context context) { final Class settingsClass = PreferenceManager.getDefaultSharedPreferences(context) - .getBoolean( - ContextCompat.getString(context, R.string.settings_layout_redesign_key), - false - ) ? SettingsV2Activity.class : SettingsActivity.class; + .getBoolean(Localization.compatGetString(context, + R.string.settings_layout_redesign_key), false) + ? SettingsV2Activity.class : SettingsActivity.class; final Intent intent = new Intent(context, settingsClass); context.startActivity(intent); From 8856e97c62442e3de1abc1fc09594a004a5bcc49 Mon Sep 17 00:00:00 2001 From: Stypox Date: Fri, 5 Sep 2025 17:17:23 +0200 Subject: [PATCH 12/15] Reorder buttons in error panel and don't allow reporting recaptchas --- .../org/schabi/newpipe/error/ErrorInfo.kt | 4 +- .../ui/components/common/ErrorPanel.kt | 56 +++++++++---------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt index a3c0aa993..45ab8daa0 100644 --- a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt +++ b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt @@ -3,7 +3,6 @@ package org.schabi.newpipe.error import android.content.Context import android.os.Parcelable import androidx.annotation.StringRes -import androidx.core.content.ContextCompat import com.google.android.exoplayer2.ExoPlaybackException import com.google.android.exoplayer2.upstream.HttpDataSource import com.google.android.exoplayer2.upstream.Loader @@ -275,6 +274,9 @@ class ErrorInfo private constructor( // we don't have an exception, so this is a manually built error, which likely // indicates that it's important and is thus reportable null -> true + // a recaptcha was detected, and the user needs to solve it, there is no use in + // letting users report it + is ReCaptchaException -> false // the service explicitly said that content is not available (e.g. age restrictions, // video deleted, etc.), there is no use in letting users report it is ContentNotAvailableException -> false diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt index 3a2771a6f..74de30ea5 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -1,9 +1,8 @@ package org.schabi.newpipe.ui.components.common import android.content.Intent +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.height import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -15,12 +14,14 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp import org.schabi.newpipe.R import org.schabi.newpipe.error.ErrorInfo import org.schabi.newpipe.error.ErrorUtil import org.schabi.newpipe.error.ReCaptchaActivity +import org.schabi.newpipe.error.UserAction +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException import org.schabi.newpipe.ui.theme.AppTheme -import org.schabi.newpipe.ui.theme.SizeTokens import org.schabi.newpipe.util.external_communication.ShareUtils @Composable @@ -28,7 +29,6 @@ fun ErrorPanel( errorInfo: ErrorInfo, modifier: Modifier = Modifier, onRetry: (() -> Unit)? = null, - ) { val context = LocalContext.current val isPreview = LocalInspectionMode.current @@ -39,29 +39,24 @@ fun ErrorPanel( } Column( + verticalArrangement = Arrangement.spacedBy(12.dp), horizontalAlignment = Alignment.CenterHorizontally, - modifier = modifier + modifier = modifier, ) { - Text( text = messageText, style = MaterialTheme.typography.titleMedium.copy(fontWeight = FontWeight.Bold), textAlign = TextAlign.Center ) - Spacer(Modifier.height(SizeTokens.SpacingMedium)) - if (errorInfo.isReportable) { - ServiceColoredButton(onClick = { - ErrorUtil.openActivity(context, errorInfo) - }) { - Text(stringResource(R.string.error_snackbar_action).uppercase()) - } - } - - errorInfo.recaptchaUrl?.let { recaptchaUrl -> + if (errorInfo.recaptchaUrl != null) { ServiceColoredButton(onClick = { + // Starting ReCaptcha Challenge Activity val intent = Intent(context, ReCaptchaActivity::class.java) - .putExtra(ReCaptchaActivity.RECAPTCHA_URL_EXTRA, recaptchaUrl) + .putExtra( + ReCaptchaActivity.RECAPTCHA_URL_EXTRA, + errorInfo.recaptchaUrl + ) .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) context.startActivity(intent) }) { @@ -77,29 +72,32 @@ fun ErrorPanel( } } - errorInfo.openInBrowserUrl?.let { url -> - ServiceColoredButton(onClick = { - ShareUtils.openUrlInBrowser(context, url) - }) { - Text(stringResource(R.string.open_in_browser).uppercase()) + if (errorInfo.isReportable) { + ServiceColoredButton(onClick = { ErrorUtil.openActivity(context, errorInfo) }) { + Text(stringResource(R.string.error_snackbar_action).uppercase()) } } - Spacer(Modifier.height(SizeTokens.SpacingExtraLarge)) + errorInfo.openInBrowserUrl?.let { url -> + ServiceColoredButton(onClick = { ShareUtils.openUrlInBrowser(context, url) }) { + Text(stringResource(R.string.open_in_browser).uppercase()) + } + } } } -@Preview(showBackground = true, widthDp = 360, heightDp = 640) - +@Preview(showBackground = true, widthDp = 360, heightDp = 640, backgroundColor = 0xffffffff) @Composable fun ErrorPanelPreview() { AppTheme { ErrorPanel( errorInfo = ErrorInfo( - throwable = Exception("Network error"), - userAction = org.schabi.newpipe.error.UserAction.UI_ERROR, - request = "Preview request" - ) + throwable = ReCaptchaException("An error", "https://example.com"), + userAction = UserAction.REQUESTED_STREAM, + request = "Preview request", + openInBrowserUrl = "https://example.com", + ), + onRetry = {}, ) } } From d9ddc07d4b7c614a78cfbfb9c53c7895cf9471b0 Mon Sep 17 00:00:00 2001 From: Clippy <22662897+absurdlylongusername@users.noreply.github.com> Date: Wed, 1 Oct 2025 17:24:02 +0100 Subject: [PATCH 13/15] Fix failing recaptcha mapping test --- .../java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt index 454964b44..4ced9dbc7 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt @@ -53,7 +53,7 @@ class ErrorInfoCommentsTest { ) Assert.assertEquals(context.getString(R.string.recaptcha_request_toast), errorInfo.getMessage(context)) Assert.assertEquals(url, errorInfo.recaptchaUrl) - Assert.assertTrue(errorInfo.isReportable) + Assert.assertFalse(errorInfo.isReportable) Assert.assertTrue(errorInfo.isRetryable) } } From 9d3ac1b94fbf90659f06d7a3dc2aa02db956ea1d Mon Sep 17 00:00:00 2001 From: Su TT Date: Sat, 4 Oct 2025 15:33:00 -0400 Subject: [PATCH 14/15] Add Compose UI tests covering ErrorPanel and comment section flows --- app/build.gradle | 4 + .../newpipe/error/ErrorInfoCommentsTest.kt | 59 --- .../schabi/newpipe/error/ErrorInfoTest.java | 62 --- .../org/schabi/newpipe/error/ErrorInfoTest.kt | 128 +++++++ .../ui/components/common/ErrorPanelTest.kt | 126 ++++++ .../comment/CommentSectionInstrumentedTest.kt | 358 ++++++++++++++++++ .../ui/components/common/ErrorPanel.kt | 4 +- gradle/libs.versions.toml | 2 + 8 files changed, 621 insertions(+), 122 deletions(-) delete mode 100644 app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt delete mode 100644 app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoTest.java create mode 100644 app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoTest.kt create mode 100644 app/src/androidTest/java/org/schabi/newpipe/ui/components/common/ErrorPanelTest.kt create mode 100644 app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionInstrumentedTest.kt diff --git a/app/build.gradle b/app/build.gradle index ec7bc3776..59ca2b6a8 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -342,6 +342,10 @@ dependencies { androidTestImplementation libs.androidx.runner androidTestImplementation libs.androidx.room.testing androidTestImplementation libs.assertj.core + androidTestImplementation platform(libs.androidx.compose.bom) + androidTestImplementation libs.androidx.compose.ui.test.junit4 + debugImplementation libs.androidx.compose.ui.test.manifest + } static String getGitWorkingBranch() { diff --git a/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt deleted file mode 100644 index 4ced9dbc7..000000000 --- a/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt +++ /dev/null @@ -1,59 +0,0 @@ -package org.schabi.newpipe.error - -import android.content.Context -import androidx.test.core.app.ApplicationProvider -import androidx.test.ext.junit.runners.AndroidJUnit4 -import org.junit.Assert -import org.junit.Test -import org.junit.runner.RunWith -import org.schabi.newpipe.R -import org.schabi.newpipe.extractor.exceptions.ReCaptchaException -import java.io.IOException -import java.net.SocketTimeoutException - -@RunWith(AndroidJUnit4::class) -class ErrorInfoCommentsTest { - private val context: Context by lazy { ApplicationProvider.getApplicationContext() } - // Test 1: Network error on initial load (Resource.Error) - @Test - fun testInitialCommentNetworkError() { - val errorInfo = ErrorInfo( - throwable = SocketTimeoutException("Connection timeout"), - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - Assert.assertEquals(context.getString(R.string.network_error), errorInfo.getMessage(context)) - Assert.assertTrue(errorInfo.isReportable) - Assert.assertTrue(errorInfo.isRetryable) - Assert.assertNull(errorInfo.recaptchaUrl) - } - - // Test 2: Network error on paging (LoadState.Error) - @Test - fun testPagingNetworkError() { - val errorInfo = ErrorInfo( - throwable = IOException("Paging failed"), - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - Assert.assertEquals(context.getString(R.string.network_error), errorInfo.getMessage(context)) - Assert.assertTrue(errorInfo.isReportable) - Assert.assertTrue(errorInfo.isRetryable) - Assert.assertNull(errorInfo.recaptchaUrl) - } - - // Test 3: ReCaptcha during comments load - @Test - fun testReCaptchaDuringComments() { - val url = "https://www.google.com/recaptcha/api/fallback?k=test" - val errorInfo = ErrorInfo( - throwable = ReCaptchaException("ReCaptcha needed", url), - userAction = UserAction.REQUESTED_COMMENTS, - request = "comments" - ) - Assert.assertEquals(context.getString(R.string.recaptcha_request_toast), errorInfo.getMessage(context)) - Assert.assertEquals(url, errorInfo.recaptchaUrl) - Assert.assertFalse(errorInfo.isReportable) - Assert.assertTrue(errorInfo.isRetryable) - } -} diff --git a/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoTest.java b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoTest.java deleted file mode 100644 index 892d1df0f..000000000 --- a/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoTest.java +++ /dev/null @@ -1,62 +0,0 @@ -package org.schabi.newpipe.error; - -import android.os.Parcel; - -import androidx.test.ext.junit.runners.AndroidJUnit4; -import androidx.test.filters.LargeTest; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.schabi.newpipe.R; -import org.schabi.newpipe.extractor.ServiceList; -import org.schabi.newpipe.extractor.exceptions.ParsingException; - -import java.util.Arrays; -import java.util.Objects; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -/** - * Instrumented tests for {@link ErrorInfo}. - */ -@RunWith(AndroidJUnit4.class) -@LargeTest -public class ErrorInfoTest { - - /** - * @param errorInfo the error info to access - * @return the private field errorInfo.message.stringRes using reflection - */ - private int getMessageFromErrorInfo(final ErrorInfo errorInfo) - throws NoSuchFieldException, IllegalAccessException { - final var message = ErrorInfo.class.getDeclaredField("message"); - message.setAccessible(true); - final var messageValue = (ErrorInfo.Companion.ErrorMessage) message.get(errorInfo); - - final var stringRes = ErrorInfo.Companion.ErrorMessage.class.getDeclaredField("stringRes"); - stringRes.setAccessible(true); - return (int) Objects.requireNonNull(stringRes.get(messageValue)); - } - - @Test - public void errorInfoTestParcelable() throws NoSuchFieldException, IllegalAccessException { - final ErrorInfo info = new ErrorInfo(new ParsingException("Hello"), - UserAction.USER_REPORT, "request", ServiceList.YouTube.getServiceId()); - // Obtain a Parcel object and write the parcelable object to it: - final Parcel parcel = Parcel.obtain(); - info.writeToParcel(parcel, 0); - parcel.setDataPosition(0); - final ErrorInfo infoFromParcel = (ErrorInfo) ErrorInfo.CREATOR.createFromParcel(parcel); - - assertTrue(Arrays.toString(infoFromParcel.getStackTraces()) - .contains(ErrorInfoTest.class.getSimpleName())); - assertEquals(UserAction.USER_REPORT, infoFromParcel.getUserAction()); - assertEquals(ServiceList.YouTube.getServiceInfo().getName(), - infoFromParcel.getServiceName()); - assertEquals("request", infoFromParcel.getRequest()); - assertEquals(R.string.parsing_error, getMessageFromErrorInfo(infoFromParcel)); - - parcel.recycle(); - } -} diff --git a/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoTest.kt b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoTest.kt new file mode 100644 index 000000000..2dee463ef --- /dev/null +++ b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoTest.kt @@ -0,0 +1,128 @@ +package org.schabi.newpipe.error + +import android.content.Context +import android.os.Parcel +import android.os.Parcelable +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.LargeTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.schabi.newpipe.R +import org.schabi.newpipe.extractor.ServiceList +import org.schabi.newpipe.extractor.exceptions.ParsingException +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException +import java.io.IOException +import java.net.SocketTimeoutException + +/** + * Instrumented tests for {@link ErrorInfo}. + */ +@RunWith(AndroidJUnit4::class) +@LargeTest +class ErrorInfoTest { + private val context: Context by lazy { ApplicationProvider.getApplicationContext() } + + /** + * @param errorInfo the error info to access + * @return the private field errorInfo.message.stringRes using reflection + */ + @Throws(NoSuchFieldException::class, IllegalAccessException::class) + private fun getMessageFromErrorInfo(errorInfo: ErrorInfo): Int { + val message = ErrorInfo::class.java.getDeclaredField("message") + message.isAccessible = true + val messageValue = message.get(errorInfo) as ErrorInfo.Companion.ErrorMessage + + val stringRes = ErrorInfo.Companion.ErrorMessage::class.java.getDeclaredField("stringRes") + stringRes.isAccessible = true + return stringRes.get(messageValue) as Int + } + + @Test + @Throws(NoSuchFieldException::class, IllegalAccessException::class) + fun errorInfoTestParcelable() { + val info = ErrorInfo( + ParsingException("Hello"), + UserAction.USER_REPORT, + "request", + ServiceList.YouTube.serviceId + ) + // Obtain a Parcel object and write the parcelable object to it: + val parcel = Parcel.obtain() + info.writeToParcel(parcel, 0) + parcel.setDataPosition(0) + val creatorField = ErrorInfo::class.java.getDeclaredField("CREATOR") + val creator = creatorField.get(null) + check(creator is Parcelable.Creator<*>) + val infoFromParcel = requireNotNull( + creator.createFromParcel(parcel) as? ErrorInfo + ) + assertTrue( + infoFromParcel.stackTraces.contentToString() + .contains(ErrorInfoTest::class.java.simpleName) + ) + assertEquals(UserAction.USER_REPORT, infoFromParcel.userAction) + assertEquals( + ServiceList.YouTube.serviceInfo.name, + infoFromParcel.getServiceName() + ) + assertEquals("request", infoFromParcel.request) + assertEquals(R.string.parsing_error, getMessageFromErrorInfo(infoFromParcel)) + + parcel.recycle() + } + + /** + * Test: Network error on initial load (Resource.Error) + */ + + @Test + fun testInitialCommentNetworkError() { + val errorInfo = ErrorInfo( + throwable = SocketTimeoutException("Connection timeout"), + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + assertEquals(context.getString(R.string.network_error), errorInfo.getMessage(context)) + assertTrue(errorInfo.isReportable) + assertTrue(errorInfo.isRetryable) + assertNull(errorInfo.recaptchaUrl) + } + + /** + * Test: Network error on paging (LoadState.Error) + */ + @Test + fun testPagingNetworkError() { + val errorInfo = ErrorInfo( + throwable = IOException("Paging failed"), + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + assertEquals(context.getString(R.string.network_error), errorInfo.getMessage(context)) + assertTrue(errorInfo.isReportable) + assertTrue(errorInfo.isRetryable) + assertNull(errorInfo.recaptchaUrl) + } + + /** + * Test: ReCaptcha during comments load + */ + @Test + fun testReCaptchaDuringComments() { + val url = "https://www.google.com/recaptcha/api/fallback?k=test" + val errorInfo = ErrorInfo( + throwable = ReCaptchaException("ReCaptcha needed", url), + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + assertEquals(context.getString(R.string.recaptcha_request_toast), errorInfo.getMessage(context)) + assertEquals(url, errorInfo.recaptchaUrl) + assertFalse(errorInfo.isReportable) + assertTrue(errorInfo.isRetryable) + } +} diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/common/ErrorPanelTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/common/ErrorPanelTest.kt new file mode 100644 index 000000000..a6627ee66 --- /dev/null +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/common/ErrorPanelTest.kt @@ -0,0 +1,126 @@ +package org.schabi.newpipe.ui.components.common + +import androidx.activity.ComponentActivity +import androidx.annotation.StringRes +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.schabi.newpipe.R +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.UserAction +import org.schabi.newpipe.extractor.exceptions.ContentNotAvailableException +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException +import org.schabi.newpipe.ui.theme.AppTheme +import java.net.UnknownHostException + +@RunWith(AndroidJUnit4::class) +class ErrorPanelTest { + @get:Rule + val composeRule = createAndroidComposeRule() + + private fun setErrorPanel(errorInfo: ErrorInfo, onRetry: (() -> Unit)? = null) { + composeRule.setContent { + AppTheme { + ErrorPanel(errorInfo = errorInfo, onRetry = onRetry) + } + } + } + private fun text(@StringRes id: Int) = composeRule.activity.getString(id) + + /** + * Test Network Error + */ + @Test + fun testNetworkErrorShowsRetryWithoutReportButton() { + val networkErrorInfo = ErrorInfo( + throwable = UnknownHostException("offline"), + userAction = UserAction.REQUESTED_STREAM, + request = "https://example.com/watch?v=foo" + ) + + setErrorPanel(networkErrorInfo, onRetry = {}) + composeRule.onNodeWithText(text(R.string.network_error)).assertIsDisplayed() + composeRule.onNodeWithText(text(R.string.retry), ignoreCase = true).assertIsDisplayed() + composeRule.onNodeWithText(text(R.string.error_snackbar_action), ignoreCase = true) + .assertDoesNotExist() + composeRule.onNodeWithText(text(R.string.recaptcha_solve), ignoreCase = true) + .assertDoesNotExist() + } + + /** + * Test Unexpected Error, Shows Report and Retry buttons + */ + @Test + fun unexpectedErrorShowsReportAndRetryButtons() { + val unexpectedErrorInfo = ErrorInfo( + throwable = RuntimeException("Unexpected error"), + userAction = UserAction.REQUESTED_STREAM, + request = "https://example.com/watch?v=bar" + ) + + setErrorPanel(unexpectedErrorInfo, onRetry = {}) + composeRule.onNodeWithText(text(R.string.error_snackbar_message)).assertIsDisplayed() + composeRule.onNodeWithText(text(R.string.retry), ignoreCase = true).assertIsDisplayed() + composeRule.onNodeWithText(text(R.string.error_snackbar_action), ignoreCase = true) + .assertIsDisplayed() + } + + /** + * Test Recaptcha Error shows solve, retry and open in browser buttons + */ + @Test + fun recaptchaErrorShowsSolveAndRetryOpenInBrowserButtons() { + var retryClicked = false + val recaptchaErrorInfo = ErrorInfo( + throwable = ReCaptchaException( + "Recaptcha required", + "https://example.com/captcha" + ), + userAction = UserAction.REQUESTED_STREAM, + request = "https://example.com/watch?v=baz", + openInBrowserUrl = "https://example.com/watch?v=baz" + ) + + setErrorPanel( + errorInfo = recaptchaErrorInfo, + onRetry = { retryClicked = true } + + ) + composeRule.onNodeWithText(text(R.string.recaptcha_solve), ignoreCase = true) + .assertIsDisplayed() + composeRule.onNodeWithText(text(R.string.retry), ignoreCase = true) + .assertIsDisplayed() + .performClick() + composeRule.onNodeWithText(text(R.string.open_in_browser), ignoreCase = true) + .assertIsDisplayed() + composeRule.onNodeWithText(text(R.string.error_snackbar_action), ignoreCase = true) + .assertDoesNotExist() + assert(retryClicked) { "onRetry callback should have been invoked" } + } + + /** + * Test Content Not Available Error hides retry button + */ + @Test + fun testNonRetryableErrorHidesRetryAndReportButtons() { + val contentNotAvailable = ErrorInfo( + throwable = ContentNotAvailableException("Video has been removed"), + userAction = UserAction.REQUESTED_STREAM, + request = "https://example.com/watch?v=qux" + ) + + setErrorPanel(contentNotAvailable) + + composeRule.onNodeWithText(text(R.string.content_not_available)) + .assertIsDisplayed() + composeRule.onNodeWithText(text(R.string.retry), ignoreCase = true) + .assertDoesNotExist() + composeRule.onNodeWithText(text(R.string.error_snackbar_action), ignoreCase = true) + .assertDoesNotExist() + } +} diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionInstrumentedTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionInstrumentedTest.kt new file mode 100644 index 000000000..eed80ead4 --- /dev/null +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comment/CommentSectionInstrumentedTest.kt @@ -0,0 +1,358 @@ +package org.schabi.newpipe.ui.components.video.comment + +import androidx.annotation.StringRes +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.heightIn +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.input.nestedscroll.nestedScroll +import androidx.compose.ui.platform.rememberNestedScrollInteropConnection +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.res.pluralStringResource +import androidx.compose.ui.test.ExperimentalTestApi +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.hasClickAction +import androidx.compose.ui.test.hasText +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performScrollTo +import androidx.compose.ui.test.performScrollToNode +import androidx.compose.ui.unit.dp +import androidx.paging.LoadState +import androidx.paging.LoadStates +import androidx.paging.PagingData +import androidx.paging.compose.collectAsLazyPagingItems +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.schabi.newpipe.R +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.UserAction +import org.schabi.newpipe.extractor.Page +import org.schabi.newpipe.extractor.comments.CommentsInfoItem +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException +import org.schabi.newpipe.extractor.stream.Description +import org.schabi.newpipe.ui.components.common.ErrorPanel +import org.schabi.newpipe.ui.components.common.LazyColumnThemedScrollbar +import org.schabi.newpipe.ui.components.common.LoadingIndicator +import org.schabi.newpipe.ui.emptystate.EmptyStateComposable +import org.schabi.newpipe.ui.emptystate.EmptyStateSpec +import org.schabi.newpipe.ui.theme.AppTheme +import org.schabi.newpipe.viewmodels.util.Resource +import java.net.UnknownHostException + +class CommentSectionInstrumentedTest { + + @get:Rule + val composeRule = createAndroidComposeRule() + + private val uiStateFlow = MutableStateFlow>(Resource.Loading) + private val pagingFlow = MutableStateFlow(PagingData.empty()) + private fun string(@StringRes resId: Int) = composeRule.activity.getString(resId) + + @Before + fun setUp() { + composeRule.setContent { + AppTheme { + TestCommentSection(uiStateFlow = uiStateFlow, commentsFlow = pagingFlow) + } + } + } + + private fun successState(commentCount: Int) = Resource.Success( + CommentInfo( + serviceId = 0, + url = "", + comments = emptyList(), + nextPage = null, + commentCount = commentCount, + isCommentsDisabled = false + ) + ) + + @Test + fun commentListLoadsAndScrolls() { + val comments = (1..25).map { index -> + CommentsInfoItem( + commentText = Description("Comment $index", Description.PLAIN_TEXT), + uploaderName = "Uploader $index", + replies = Page(""), + replyCount = 0 + ) + } + uiStateFlow.value = successState(comments.size) + pagingFlow.value = PagingData.from(comments) + composeRule.waitForIdle() + composeRule.onNodeWithText("Comment 1").assertIsDisplayed() + composeRule.onNodeWithTag("comment_list") + .performScrollToNode(hasText("Comment 25")) + composeRule.onNodeWithText("Comment 25").assertIsDisplayed() + } + + @OptIn(ExperimentalTestApi::class) + @Test + fun pagingErrorShowsErrorPanelAndAllowsRetry() { + uiStateFlow.value = successState(10) + pagingFlow.value = PagingData.from( + data = emptyList(), + sourceLoadStates = LoadStates( + refresh = LoadState.Error(ReCaptchaException("captcha required", "https://example.com")), + prepend = LoadState.NotLoading(true), + append = LoadState.NotLoading(true) + ) + ) + composeRule.waitForIdle() + + val solveMatcher = hasText(string(R.string.recaptcha_solve), ignoreCase = true) + .and(hasClickAction()) + val retryMatcher = hasText(string(R.string.retry), ignoreCase = true) + .and(hasClickAction()) + + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodes(solveMatcher).fetchSemanticsNodes().isNotEmpty() + } + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodes(retryMatcher).fetchSemanticsNodes().isNotEmpty() + } + + composeRule.onNode(retryMatcher) + .performScrollTo() + .performClick() + + val recoveredComment = CommentsInfoItem( + commentText = Description("Recovered comment", Description.PLAIN_TEXT), + uploaderName = "Uploader", + replies = Page(""), + replyCount = 0 + ) + + uiStateFlow.value = successState(1) + pagingFlow.value = PagingData.from( + data = listOf(recoveredComment), + sourceLoadStates = LoadStates( + refresh = LoadState.NotLoading(false), + prepend = LoadState.NotLoading(true), + append = LoadState.NotLoading(true) + ) + ) + composeRule.waitForIdle() + + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodes(hasText("Recovered comment")) + .fetchSemanticsNodes() + .isNotEmpty() + } + composeRule.onNodeWithText("Recovered comment").assertIsDisplayed() + + composeRule.onNode(solveMatcher).assertDoesNotExist() + composeRule.onNode(retryMatcher).assertDoesNotExist() + } + + @OptIn(ExperimentalTestApi::class) + @Test + fun resourceErrorShowsErrorPanelAndRetry() { + uiStateFlow.value = Resource.Error(UnknownHostException("offline")) + composeRule.waitForIdle() + + composeRule.onNodeWithText(string(R.string.network_error)).assertIsDisplayed() + val retryMatcher = hasText(string(R.string.retry), ignoreCase = true) + .and(hasClickAction()) + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodes(retryMatcher).fetchSemanticsNodes().isNotEmpty() + } + + composeRule.onNode(retryMatcher) + .performScrollTo() + .performClick() + + val recoveredComment = CommentsInfoItem( + commentText = Description("Recovered comment", Description.PLAIN_TEXT), + uploaderName = "Uploader", + replies = Page(""), + replyCount = 0 + ) + + uiStateFlow.value = successState(1) + pagingFlow.value = PagingData.from( + data = listOf(recoveredComment), + sourceLoadStates = LoadStates( + refresh = LoadState.NotLoading(false), + prepend = LoadState.NotLoading(true), + append = LoadState.NotLoading(true) + ) + ) + composeRule.waitForIdle() + + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodes(hasText("Recovered comment")) + .fetchSemanticsNodes() + .isNotEmpty() + } + composeRule.onNodeWithText("Recovered comment").assertIsDisplayed() + + composeRule.onNodeWithText(string(R.string.network_error)) + .assertDoesNotExist() + composeRule.onNode(retryMatcher).assertDoesNotExist() + } + + @OptIn(ExperimentalTestApi::class) + @Test + fun retryAfterErrorRecoversList() { + uiStateFlow.value = Resource.Error(RuntimeException("boom")) + composeRule.waitForIdle() + + val retryMatcher = hasText(string(R.string.retry), ignoreCase = true) + .and(hasClickAction()) + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodes(retryMatcher).fetchSemanticsNodes().isNotEmpty() + } + composeRule.onNode(retryMatcher) + .performScrollTo() + .performClick() + + val firstComment = CommentsInfoItem( + commentText = Description("First comment", Description.PLAIN_TEXT), + uploaderName = "Uploader", + replies = Page(""), + replyCount = 0 + ) + + uiStateFlow.value = successState(1) + pagingFlow.value = PagingData.from( + data = listOf(firstComment), + sourceLoadStates = LoadStates( + refresh = LoadState.NotLoading(false), + prepend = LoadState.NotLoading(true), + append = LoadState.NotLoading(true) + ) + ) + composeRule.waitForIdle() + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodes(hasText("First comment")) + .fetchSemanticsNodes() + .isNotEmpty() + } + composeRule.onNodeWithText("First comment").assertIsDisplayed() + + composeRule.onNodeWithText(string(R.string.network_error)) + .assertDoesNotExist() + composeRule.onNode(retryMatcher).assertDoesNotExist() + } +} + +@Composable +private fun TestCommentSection( + uiStateFlow: StateFlow>, + commentsFlow: Flow> +) { + val uiState by uiStateFlow.collectAsState() + val comments = commentsFlow.collectAsLazyPagingItems() + val nestedScrollInterop = rememberNestedScrollInteropConnection() + val listState = rememberLazyListState() + val COMMENT_LIST_TAG = "comment_list" + + LazyColumnThemedScrollbar(state = listState) { + LazyColumn( + modifier = Modifier + .testTag(COMMENT_LIST_TAG) + .nestedScroll(nestedScrollInterop), + state = listState + ) { + when (uiState) { + is Resource.Loading -> item { + LoadingIndicator(modifier = Modifier.padding(top = 8.dp)) + } + is Resource.Success -> { + val commentInfo = (uiState as Resource.Success).data + val count = commentInfo.commentCount + + when { + commentInfo.isCommentsDisabled -> item { + EmptyStateComposable( + spec = EmptyStateSpec.DisabledComments, + modifier = Modifier + .fillMaxWidth() + .heightIn(min = 128.dp) + ) + } + count == 0 -> item { + EmptyStateComposable( + spec = EmptyStateSpec.NoComments, + modifier = Modifier + .fillMaxWidth() + .heightIn(min = 128.dp) + ) + } + else -> { + if (count >= 0) { + item { + Text( + modifier = Modifier + .padding(start = 12.dp, end = 12.dp, bottom = 4.dp), + text = pluralStringResource(R.plurals.comments, count, count), + maxLines = 1, + style = MaterialTheme.typography.titleMedium + ) + } + } + when (val refresh = comments.loadState.refresh) { + is LoadState.Loading -> item { + LoadingIndicator(modifier = Modifier.padding(top = 8.dp)) + } + is LoadState.Error -> item { + Box( + modifier = Modifier.fillMaxWidth() + ) { + ErrorPanel( + errorInfo = ErrorInfo( + throwable = refresh.error, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ), + onRetry = { comments.retry() }, + modifier = Modifier.align(Alignment.Center) + ) + } + } + else -> items(comments.itemCount) { index -> + Comment(comment = comments[index]!!) {} + } + } + } + } + } + is Resource.Error -> item { + Box( + modifier = Modifier.fillMaxSize(), + contentAlignment = Alignment.Center + ) { + ErrorPanel( + errorInfo = ErrorInfo( + throwable = (uiState as Resource.Error).throwable, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ), + onRetry = { comments.retry() }, + modifier = Modifier.align(Alignment.Center) + ) + } + } + } + } + } +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt index 74de30ea5..943c59853 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -1,6 +1,7 @@ package org.schabi.newpipe.ui.components.common import android.content.Intent +import android.content.res.Configuration import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.material3.MaterialTheme @@ -86,7 +87,8 @@ fun ErrorPanel( } } -@Preview(showBackground = true, widthDp = 360, heightDp = 640, backgroundColor = 0xffffffff) +@Preview(name = "Light mode", uiMode = Configuration.UI_MODE_NIGHT_NO) +@Preview(name = "Dark mode", uiMode = Configuration.UI_MODE_NIGHT_YES) @Composable fun ErrorPanelPreview() { AppTheme { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index bea2550ad..3d12d559e 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -88,6 +88,8 @@ androidx-compose-adaptive = { group = "androidx.compose.material3.adaptive", nam androidx-compose-bom = { group = "androidx.compose", name = "compose-bom", version.ref = "jetpack-compose" } androidx-compose-material-icons-extended = { group = "androidx.compose.material", name = "material-icons-extended" } androidx-compose-material3 = { group = "androidx.compose.material3", name = "material3" } +androidx-compose-ui-test-junit4 = { group = "androidx.compose.ui", name = "ui-test-junit4" } +androidx-compose-ui-test-manifest = { group = "androidx.compose.ui", name = "ui-test-manifest" } androidx-compose-ui-text = { group = "androidx.compose.ui", name = "ui-text" } androidx-compose-ui-tooling = { group = "androidx.compose.ui", name = "ui-tooling" } androidx-compose-ui-tooling-preview = { group = "androidx.compose.ui", name = "ui-tooling-preview" } From 25e96d584a9d1a9e4dae8a1f160d60bec1642140 Mon Sep 17 00:00:00 2001 From: Su TT Date: Mon, 6 Oct 2025 11:11:13 -0400 Subject: [PATCH 15/15] Address reviewer changes - make previews private and remove white space --- .../org/schabi/newpipe/ui/components/common/ErrorPanel.kt | 2 +- .../newpipe/ui/components/common/ServiceColoredButton.kt | 6 ++++-- .../newpipe/ui/components/video/comment/CommentSection.kt | 1 - 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt index 943c59853..666d1759d 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -90,7 +90,7 @@ fun ErrorPanel( @Preview(name = "Light mode", uiMode = Configuration.UI_MODE_NIGHT_NO) @Preview(name = "Dark mode", uiMode = Configuration.UI_MODE_NIGHT_YES) @Composable -fun ErrorPanelPreview() { +private fun ErrorPanelPreview() { AppTheme { ErrorPanel( errorInfo = ErrorInfo( diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt index 8b3c41a8a..59a37066c 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt @@ -1,5 +1,6 @@ package org.schabi.newpipe.ui.components.common +import android.content.res.Configuration import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.RowScope import androidx.compose.foundation.layout.wrapContentWidth @@ -40,9 +41,10 @@ fun ServiceColoredButton( } } -@Preview +@Preview(name = "Light mode", uiMode = Configuration.UI_MODE_NIGHT_NO) +@Preview(name = "Dark mode", uiMode = Configuration.UI_MODE_NIGHT_YES) @Composable -fun ServiceColoredButtonPreview() { +private fun ServiceColoredButtonPreview() { AppTheme { ServiceColoredButton( onClick = {}, diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt index 23f13a342..f0a293784 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt @@ -122,7 +122,6 @@ private fun CommentSection( Box( modifier = Modifier .fillMaxWidth() - ) { ErrorPanel( errorInfo = errorInfo,