Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Only show resolve post if actionable #252

Merged
merged 8 commits into from
Jan 9, 2025
Merged

Bugfix: Only show resolve post if actionable #252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
832579b
Use IStandalonePost in ThreadUi
FelberMartin Dec 20, 2024
118f769
Adjust tests to use new setupThreadUI signature
FelberMartin Dec 20, 2024
5ddcf1b
Fix; adjust IAnswerPost to include parentAuthorId
FelberMartin Dec 21, 2024
3d1d961
Adjusted tests
FelberMartin Dec 21, 2024
6c6aef5
Fix compilation
FelberMartin Dec 21, 2024
bba5065
Fix implementation bug
FelberMartin Dec 21, 2024
9b5e561
Cleaned up tests; default all post actions rights to false
FelberMartin Dec 21, 2024
85c35f0
Merge branch 'develop' into bugfix/communication/only-show-resolve-po…
FelberMartin Dec 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...nformatics/www1/artemis/native_app/feature/metis/conversation/ui/ConversationViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ internal open class ConversationViewModel(
* It updates the post accordingly.
*/
fun toggleResolvePost(
parentPost: PostPojo,
parentPost: IStandalonePost,
post: AnswerPostPojo
): Deferred<MetisModificationFailure?> {
return viewModelScope.async(coroutineContext) {
Expand Down Expand Up @@ -490,7 +490,7 @@ internal open class ConversationViewModel(
}

fun editAnswerPost(
parentPost: PostPojo,
parentPost: IStandalonePost,
post: AnswerPostPojo,
newText: String
): Deferred<MetisModificationFailure?> {
Expand Down
6 changes: 4 additions & 2 deletions ...cs/www1/artemis/native_app/feature/metis/conversation/ui/post/post_actions/PostActions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.remember
import androidx.compose.ui.platform.LocalClipboardManager
import androidx.compose.ui.text.AnnotatedString
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.IAnswerPost
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.IBasePost

data class PostActions(
Expand Down Expand Up @@ -52,10 +53,11 @@ fun rememberPostActions(
}

val doesPostExistOnServer = post.serverPostId != null
val isPostAuthor = post.authorId == clientId
val isParentPostAuthor = post is IAnswerPost && post.parentAuthorId == clientId
val hasResolvePostRights =
postActionFlags.isAtLeastTutorInCourse || post.authorId == clientId
postActionFlags.isAtLeastTutorInCourse || isParentPostAuthor
val hasPinPostRights = postActionFlags.isAbleToPin
val isPostAuthor = post.authorId == clientId

PostActions(
requestEditPost = if (doesPostExistOnServer && isPostAuthor) onRequestEdit else null,
Expand Down
9 changes: 5 additions & 4 deletions ...www1/artemis/native_app/feature/metis/conversation/ui/thread/ConversationThreadUseCase.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import de.tum.informatics.www1.artemis.native_app.core.datastore.AccountService
import de.tum.informatics.www1.artemis.native_app.core.datastore.ServerConfigurationService
import de.tum.informatics.www1.artemis.native_app.core.datastore.authToken
import de.tum.informatics.www1.artemis.native_app.core.device.NetworkStatusProvider
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisContext
import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.service.network.MetisService
import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.service.storage.MetisStorageService
import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.ui.DataStatus
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisContext
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.StandalonePostId
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.IStandalonePost
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.StandalonePost
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.db.entities.BasePostingEntity
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.db.pojo.PostPojo
Expand Down Expand Up @@ -56,7 +57,7 @@ internal class ConversationThreadUseCase(
/**
* The post data state flow as loading from the server.
*/
val post: StateFlow<DataState<PostPojo>> = postId.flatMapLatest { postId ->
val post: StateFlow<DataState<IStandalonePost>> = postId.flatMapLatest { postId ->
when (postId) {
is StandalonePostId.ClientSideId -> metisStorageService
.getStandalonePost(postId.clientSideId)
Expand Down Expand Up @@ -100,8 +101,8 @@ internal class ConversationThreadUseCase(
private suspend fun handleServerLoadedStandalonePost(
metisContext: MetisContext,
standalonePostDataState: DataState<StandalonePost>
): Flow<DataState<PostPojo>> {
val failureFlow: Flow<DataState<PostPojo>> =
): Flow<DataState<out IStandalonePost>> {
val failureFlow: Flow<DataState<IStandalonePost>> =
flowOf(DataState.Failure(RuntimeException("Something went wrong while loading the post.")))

return when (standalonePostDataState) {
Expand Down
11 changes: 6 additions & 5 deletions ...informatics/www1/artemis/native_app/feature/metis/conversation/ui/thread/MetisThreadUi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.ui.
import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.ui.reply.ReplyTextField
import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.ui.shared.isReplyEnabled
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.IBasePost
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.IStandalonePost
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.conversation.Conversation
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.db.pojo.AnswerPostPojo
import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.db.pojo.PostPojo
Expand All @@ -62,7 +63,7 @@ import kotlinx.coroutines.Deferred
import org.koin.compose.koinInject

internal const val TEST_TAG_THREAD_LIST = "TEST_TAG_THREAD_LIST"
internal fun testTagForAnswerPost(answerPostId: String) = "answerPost$answerPostId"
internal fun testTagForAnswerPost(answerPostId: String?) = "answerPost$answerPostId"

/**
* Displays a single post with its replies.
Expand All @@ -73,7 +74,7 @@ internal fun MetisThreadUi(
listContentPadding: PaddingValues,
viewModel: ConversationViewModel
) {
val postDataState: DataState<PostPojo> by viewModel.threadUseCase.post.collectAsState()
val postDataState: DataState<IStandalonePost> by viewModel.threadUseCase.post.collectAsState()
val clientId: Long by viewModel.clientIdOrDefault.collectAsState()

val serverUrl by viewModel.serverUrl.collectAsState()
Expand Down Expand Up @@ -154,7 +155,7 @@ internal fun MetisThreadUi(
modifier: Modifier,
courseId: Long,
clientId: Long,
postDataState: DataState<PostPojo>,
postDataState: DataState<IStandalonePost>,
conversationDataState: DataState<Conversation>,
postActionFlags: PostActionFlags,
listContentPadding: PaddingValues,
Expand Down Expand Up @@ -254,7 +255,7 @@ internal fun MetisThreadUi(
private fun PostAndRepliesList(
modifier: Modifier,
state: LazyListState,
post: PostPojo,
post: IStandalonePost,
postActionFlags: PostActionFlags,
listContentPadding: PaddingValues,
clientId: Long,
Expand Down Expand Up @@ -329,7 +330,7 @@ private fun PostAndRepliesList(

itemsIndexed(
post.orderedAnswerPostings,
key = { _, post -> post.postId }) { index, answerPost ->
key = { index, post -> post.clientPostId ?: index }) { index, answerPost ->
val postActions = rememberPostActions(answerPost)

PostWithBottomSheet(
Expand Down
20 changes: 12 additions & 8 deletions ...n/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/BaseChatUITest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ abstract class BaseChatUITest : BaseComposeTest() {
val answers = (0..2).map { index ->
AnswerPostPojo(
parentPostId = "client-id",
parentAuthorIdCache = AnswerPostPojo.ParentAuthorIdCache(clientId),
postId = "answer-client-id-$index",
resolvesPost = false,
basePostingCache = AnswerPostPojo.BasePostingCache(
Expand Down Expand Up @@ -78,11 +79,12 @@ abstract class BaseChatUITest : BaseComposeTest() {
}

fun setupThreadUi(
post: PostPojo,
onResolvePost: ((IBasePost) -> Deferred<MetisModificationFailure>)?,
onPinPost: ((IBasePost) -> Deferred<MetisModificationFailure>)?,
hasModerationRights: Boolean = true,
isAbleToPin: Boolean = true
post: IStandalonePost,
onResolvePost: ((IBasePost) -> Deferred<MetisModificationFailure>)? = { CompletableDeferred() },
onPinPost: ((IBasePost) -> Deferred<MetisModificationFailure>)? = { CompletableDeferred() },
isAbleToPin: Boolean = false,
isAtLeastTutorInCourse: Boolean = false,
hasModerationRights: Boolean = false,
) {
composeTestRule.setContent {
MetisThreadUi(
Expand All @@ -93,7 +95,7 @@ abstract class BaseChatUITest : BaseComposeTest() {
conversationDataState = DataState.Success(conversation),
postActionFlags = PostActionFlags(
isAbleToPin = isAbleToPin,
isAtLeastTutorInCourse = false,
isAtLeastTutorInCourse = isAtLeastTutorInCourse,
hasModerationRights = hasModerationRights,
),
listContentPadding = PaddingValues(),
Expand All @@ -116,6 +118,8 @@ abstract class BaseChatUITest : BaseComposeTest() {
fun setupChatUi(
posts: List<IStandalonePost>,
currentUser: User = User(id = clientId),
isAbleToPin: Boolean = false,
isAtLeastTutorInCourse: Boolean = false,
hasModerationRights: Boolean = false,
onPinPost: (IStandalonePost) -> Deferred<MetisModificationFailure> = { CompletableDeferred() }
) {
Expand All @@ -127,8 +131,8 @@ abstract class BaseChatUITest : BaseComposeTest() {
posts = PostsDataState.Loaded.WithList(list, PostsDataState.NotLoading),
clientId = currentUser.id,
postActionFlags = PostActionFlags(
isAbleToPin = true,
isAtLeastTutorInCourse = false,
isAbleToPin = isAbleToPin,
isAtLeastTutorInCourse = isAtLeastTutorInCourse,
hasModerationRights = hasModerationRights,
),
listContentPadding = PaddingValues(),
Expand Down
37 changes: 23 additions & 14 deletions ...cs/www1/artemis/native_app/feature/metis/conversation/ConversationAnswerMessagesUITest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,13 @@ class ConversationAnswerMessagesUITest : BaseChatUITest() {
fun `test GIVEN post is not resolved WHEN resolving the post THEN the post is resolved with the first answer post`() {
var resolvedPost: IBasePost? = null

setupThreadUi(post, { post ->
resolvedPost = post
CompletableDeferred()
}, { CompletableDeferred() })
setupThreadUi(
post = post,
onResolvePost = { post ->
resolvedPost = post
CompletableDeferred()
}
)

composeTestRule.onNodeWithText(answers[0].content!!, useUnmergedTree = true)
.performSemanticsAction(SemanticsActions.OnLongClick)
Expand All @@ -54,10 +57,13 @@ class ConversationAnswerMessagesUITest : BaseChatUITest() {
fun `test GIVEN post is not resolved WHEN resolving the post THEN the post is resolved with the third answer post`() {
var resolvedPost: IBasePost? = null

setupThreadUi(post, { post ->
resolvedPost = post
CompletableDeferred()
}, { CompletableDeferred() })
setupThreadUi(
post = post,
onResolvePost = { post ->
resolvedPost = post
CompletableDeferred()
}
)

composeTestRule.onNodeWithText(answers[2].content!!, useUnmergedTree = true)
.performSemanticsAction(SemanticsActions.OnLongClick)
Expand All @@ -81,10 +87,13 @@ class ConversationAnswerMessagesUITest : BaseChatUITest() {

var unresolvedPost: IBasePost? = null

setupThreadUi(resolvedPost, { post ->
unresolvedPost = post
CompletableDeferred()
}, { CompletableDeferred() })
setupThreadUi(
post = resolvedPost,
onResolvePost = { post ->
unresolvedPost = post
CompletableDeferred()
},
)

composeTestRule.onNodeWithText(answers[resolvingIndex].content!!, useUnmergedTree = true)
.performSemanticsAction(SemanticsActions.OnLongClick)
Expand All @@ -98,7 +107,7 @@ class ConversationAnswerMessagesUITest : BaseChatUITest() {

@Test
fun `test GIVEN the post is not resolved and no answer post is resolving THEN the post is shown as not resolved and no answer post is shown as resolving`() {
setupThreadUi(post, { CompletableDeferred() }, { CompletableDeferred() })
setupThreadUi(post)

composeTestRule.onNodeWithText(post.content).assertExists()
for (answer in answers) {
Expand All @@ -121,7 +130,7 @@ class ConversationAnswerMessagesUITest : BaseChatUITest() {
answers = modifiedAnswers
)

setupThreadUi(resolvedPost, { CompletableDeferred() }, { CompletableDeferred() })
setupThreadUi(resolvedPost)

val resolvesAssertion = hasAnyChild(hasText(context.getString(R.string.post_resolves)))

Expand Down
28 changes: 16 additions & 12 deletions ...ormatics/www1/artemis/native_app/feature/metis/conversation/ConversationMessagesUITest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,13 @@ class ConversationMessagesUITest : BaseChatUITest() {
fun `test GIVEN post is not pinned in thread WHEN pinning the post THEN the correct post gets pinned in thread`() {
var changedPost: IBasePost? = null

setupThreadUi(posts[0], { CompletableDeferred() }, { post ->
changedPost = post
CompletableDeferred()
})
setupThreadUi(
post = posts[0],
onPinPost = { post ->
changedPost = post
CompletableDeferred()
}
)

composeTestRule.onNodeWithTag(
testTagForPost(posts[0].standalonePostId),
Expand All @@ -94,10 +97,12 @@ class ConversationMessagesUITest : BaseChatUITest() {
val modifiedPosts = posts.toMutableList()
modifiedPosts[0] = modifiedPosts[0].copy(displayPriority = DisplayPriority.PINNED)

setupThreadUi(modifiedPosts[0], { CompletableDeferred() }, { post ->
changedPost = post
CompletableDeferred()
})
setupThreadUi(
post = modifiedPosts[0],
onPinPost = { post ->
changedPost = post
CompletableDeferred()
})

composeTestRule.onNodeWithTag(
testTagForPost(posts[0].standalonePostId),
Expand All @@ -119,7 +124,7 @@ class ConversationMessagesUITest : BaseChatUITest() {

@Test
fun `test GIVEN the post is not pinned in thread THEN the post is not shown as pinned in thread`() {
setupThreadUi(posts[0], { CompletableDeferred() }, { CompletableDeferred() })
setupThreadUi(posts[0])

testPinnedLabelInvisibility()
}
Expand All @@ -136,9 +141,8 @@ class ConversationMessagesUITest : BaseChatUITest() {
@Test
fun `test GIVEN the post is pinned in thread THEN the post is shown as pinned in thread`() {
setupThreadUi(
posts[0].copy(displayPriority = DisplayPriority.PINNED),
{ CompletableDeferred() },
{ CompletableDeferred() })
posts[0].copy(displayPriority = DisplayPriority.PINNED)
)

testPinnedLabelVisibility()
}
Expand Down
1 change: 1 addition & 0 deletions ...rtemis/native_app/feature/metis/conversation/service/storage/impl/MetisStorageBaseTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ abstract class MetisStorageBaseTest {
internal val localAnswerPojo = AnswerPostPojo(
parentPostId = parentClientPostId,
postId = answerClientPostId,
parentAuthorIdCache = AnswerPostPojo.ParentAuthorIdCache(author.id),
resolvesPost = false,
basePostingCache = AnswerPostPojo.BasePostingCache(
serverPostId = 0,
Expand Down
Loading
Loading