fix: avoid redundant card activity reload and assert ordering

app/src/androidTest/java/space/hackenslacker/kanbn4droid/app/CardDetailFlowTest.kt

@@ -180,8 +180,9 @@ class CardDetailFlowTest {
         onView(withText(R.string.add)).perform(click())
 
         awaitCondition {
-            fakeDataSource.addCommentCalls.get() == 1 && fakeDataSource.listActivitiesCalls.get() >= 2
+            fakeDataSource.addCommentCalls.get() == 1
         }
+        assertEquals(1, fakeDataSource.listActivitiesCalls.get())
         assertTrue(observedEvents.any { it is CardDetailUiEvent.ShowSnackbar })
     }
 
@@ -196,6 +197,7 @@ class CardDetailFlowTest {
         onView(withText(containsString("Someone commented"))).check(matches(isDisplayed()))
         onView(withText(containsString("Someone updated this card"))).check(matches(isDisplayed()))
         onView(withText(containsString("body"))).check(matches(isDisplayed()))
+        assertEquals(listOf("a-new", "a-old"), observedStates.last().activities.map { it.id })
     }
 
     @Test
@@ -360,15 +362,20 @@ class CardDetailFlowTest {
             return if (listActivitiesResults.isNotEmpty()) listActivitiesResults.removeFirst() else DataSourceResult.Success(activities)
         }
 
-        override suspend fun addComment(cardId: String, comment: String): DataSourceResult<Unit> {
+        override suspend fun addComment(cardId: String, comment: String): DataSourceResult<List<CardActivity>> {
             addCommentCalls.incrementAndGet()
             val result = if (addCommentResults.isNotEmpty()) addCommentResults.removeFirst() else DataSourceResult.Success(Unit)
             if (result is DataSourceResult.Success) {
                 activities = listOf(
                     CardActivity("comment-${addCommentCalls.get()}", "comment", comment, System.currentTimeMillis()),
                 ) + activities
+                return DataSourceResult.Success(activities)
+            }
+            return when (result) {
+                is DataSourceResult.GenericError -> result
+                DataSourceResult.SessionExpired -> DataSourceResult.SessionExpired
+                is DataSourceResult.Success -> DataSourceResult.Success(activities)
             }
-            return result
         }
     }
 }

app/src/main/java/space/hackenslacker/kanbn4droid/app/carddetail/CardDetailActivity.kt

@@ -97,7 +97,13 @@ class CardDetailActivity : AppCompatActivity() {
         val id = cardId
         val fakeFactory = testDataSourceFactory
         if (fakeFactory != null) {
-            CardDetailViewModel.Factory(id, fakeFactory.invoke(id))
+            CardDetailViewModel.Factory(
+                cardId = id,
+                dataSource = fakeFactory.invoke(id),
+                titleRequiredMessage = getString(R.string.card_title_required),
+                commentRequiredMessage = getString(R.string.card_detail_comment_required),
+                commentAddedMessage = getString(R.string.card_detail_comment_added),
+            )
         } else {
             val repository = CardDetailRepository(
                 sessionStore = sessionStore,
@@ -107,6 +113,9 @@ class CardDetailActivity : AppCompatActivity() {
             CardDetailViewModel.Factory(
                 cardId = id,
                 dataSource = CardDetailRepositoryDataSource(repository, repository::loadCard),
+                titleRequiredMessage = getString(R.string.card_title_required),
+                commentRequiredMessage = getString(R.string.card_detail_comment_required),
+                commentAddedMessage = getString(R.string.card_detail_comment_added),
             )
         }
     }

app/src/main/java/space/hackenslacker/kanbn4droid/app/carddetail/CardDetailViewModel.kt

@@ -58,7 +58,7 @@ interface CardDetailDataSource {
     suspend fun updateDescription(cardId: String, description: String): DataSourceResult<Unit>
     suspend fun updateDueDate(cardId: String, dueDate: LocalDate?): DataSourceResult<Unit>
     suspend fun listActivities(cardId: String): DataSourceResult<List<CardActivity>>
-    suspend fun addComment(cardId: String, comment: String): DataSourceResult<Unit>
+    suspend fun addComment(cardId: String, comment: String): DataSourceResult<List<CardActivity>>
 }
 
 internal class CardDetailRepositoryDataSource(
@@ -85,10 +85,10 @@ internal class CardDetailRepositoryDataSource(
         return repository.listActivities(cardId).toDataSourceResult()
     }
 
-    override suspend fun addComment(cardId: String, comment: String): DataSourceResult<Unit> {
+    override suspend fun addComment(cardId: String, comment: String): DataSourceResult<List<CardActivity>> {
         val result = repository.addComment(cardId, comment)
         return when (result) {
-            is CardDetailRepository.Result.Success -> DataSourceResult.Success(Unit)
+            is CardDetailRepository.Result.Success -> DataSourceResult.Success(result.value)
             is CardDetailRepository.Result.Failure.SessionExpired -> DataSourceResult.SessionExpired
             is CardDetailRepository.Result.Failure.Generic -> {
                 DataSourceResult.GenericError(result.message)
@@ -101,6 +101,9 @@ class CardDetailViewModel(
     private val cardId: String,
     private val repository: CardDetailDataSource,
     private val descriptionDebounceMillis: Long = 800L,
+    private val titleRequiredMessage: String = "Card title is required",
+    private val commentRequiredMessage: String = "Comment is required",
+    private val commentAddedMessage: String = "Comment added",
 ) : ViewModel() {
     private val _uiState = MutableStateFlow(CardDetailUiState())
     val uiState: StateFlow<CardDetailUiState> = _uiState.asStateFlow()
@@ -284,7 +287,7 @@ class CardDetailViewModel(
 
         val payload = _uiState.value.commentDraft.trim()
         if (payload.isBlank()) {
-            _uiState.update { it.copy(commentErrorMessage = "Comment is required") }
+            _uiState.update { it.copy(commentErrorMessage = commentRequiredMessage) }
             return
         }
 
@@ -299,10 +302,12 @@ class CardDetailViewModel(
                             isCommentDialogOpen = false,
                             commentDraft = "",
                             commentDialogMode = CommentDialogMode.EDIT,
+                            activities = result.value,
+                            activitiesErrorMessage = null,
+                            isActivitiesLoading = false,
                         )
                     }
-                    _events.emit(CardDetailUiEvent.ShowSnackbar("Comment added"))
+                    _events.emit(CardDetailUiEvent.ShowSnackbar(commentAddedMessage))
-                    loadActivities()
                 }
 
                 is DataSourceResult.SessionExpired -> {
@@ -342,7 +347,7 @@ class CardDetailViewModel(
         }
         val normalized = rawTitle.trim()
         if (normalized.isBlank()) {
-            _uiState.update { it.copy(titleErrorMessage = "Card title is required") }
+            _uiState.update { it.copy(titleErrorMessage = titleRequiredMessage) }
             return
         }
         if (!fromRetry && normalized == persistedTitle) {
@@ -567,11 +572,20 @@ class CardDetailViewModel(
     class Factory(
         private val cardId: String,
         private val dataSource: CardDetailDataSource,
+        private val titleRequiredMessage: String = "Card title is required",
+        private val commentRequiredMessage: String = "Comment is required",
+        private val commentAddedMessage: String = "Comment added",
     ) : ViewModelProvider.Factory {
         @Suppress("UNCHECKED_CAST")
         override fun <T : ViewModel> create(modelClass: Class<T>): T {
             if (modelClass.isAssignableFrom(CardDetailViewModel::class.java)) {
-                return CardDetailViewModel(cardId = cardId, repository = dataSource) as T
+                return CardDetailViewModel(
+                    cardId = cardId,
+                    repository = dataSource,
+                    titleRequiredMessage = titleRequiredMessage,
+                    commentRequiredMessage = commentRequiredMessage,
+                    commentAddedMessage = commentAddedMessage,
+                ) as T
             }
             throw IllegalArgumentException("Unknown ViewModel class: ${modelClass.name}")
         }

app/src/main/res/values/strings.xml

@@ -85,4 +85,6 @@
     <string name="card_detail_timeline_action_commented">commented</string>
     <string name="card_detail_timeline_action_updated">updated this card</string>
     <string name="add">Add</string>
+    <string name="card_detail_comment_required">Comment is required</string>
+    <string name="card_detail_comment_added">Comment added</string>
 </resources>

app/src/test/java/space/hackenslacker/kanbn4droid/app/carddetail/CardDetailViewModelTest.kt

@@ -227,10 +227,9 @@ class CardDetailViewModelTest {
             listActivitiesResults = ArrayDeque(
                 listOf(
                     DataSourceResult.Success(emptyList()),
-                    DataSourceResult.Success(sampleActivitiesShuffled()),
                 ),
             ),
-            addCommentResult = DataSourceResult.Success(Unit),
+            addCommentResult = DataSourceResult.Success(sampleActivitiesShuffled()),
         )
         val viewModel = loadedViewModel(this, repository)
         val eventDeferred = async { viewModel.events.first { it is CardDetailUiEvent.ShowSnackbar } }
@@ -243,7 +242,7 @@ class CardDetailViewModelTest {
         assertFalse(viewModel.uiState.value.isCommentDialogOpen)
         assertEquals("", viewModel.uiState.value.commentDraft)
         assertEquals(1, repository.addCommentCalls)
-        assertEquals(2, repository.listActivitiesCalls)
+        assertEquals(1, repository.listActivitiesCalls)
         assertEquals(listOf("a-mid", "a-new", "a-old"), viewModel.uiState.value.activities.map { it.id })
         assertTrue(eventDeferred.await() is CardDetailUiEvent.ShowSnackbar)
     }
@@ -365,7 +364,7 @@ class CardDetailViewModelTest {
         var updateTitleResult: DataSourceResult<Unit> = DataSourceResult.Success(Unit),
         var updateDescriptionResult: DataSourceResult<Unit> = DataSourceResult.Success(Unit),
         var updateDueDateResult: DataSourceResult<Unit> = DataSourceResult.Success(Unit),
-        var addCommentResult: DataSourceResult<Unit> = DataSourceResult.Success(Unit),
+        var addCommentResult: DataSourceResult<List<CardActivity>> = DataSourceResult.Success(emptyList()),
         var updateDescriptionGate: CompletableDeferred<Unit>? = null,
         var updateDescriptionGates: ArrayDeque<CompletableDeferred<Unit>> = ArrayDeque(),
     ) : CardDetailDataSource {
@@ -416,7 +415,7 @@ class CardDetailViewModelTest {
             return listActivitiesResult
         }
 
-        override suspend fun addComment(cardId: String, comment: String): DataSourceResult<Unit> {
+        override suspend fun addComment(cardId: String, comment: String): DataSourceResult<List<CardActivity>> {
             addCommentCalls += 1
             addCommentPayloads += comment
             return addCommentResult