From a0548dfdc48abbbfdceb6b02b41b78a050f612b7 Mon Sep 17 00:00:00 2001 From: jonnyandrew Date: Tue, 28 Nov 2023 13:25:18 +0000 Subject: [PATCH] Add option to delete a poll while editing the poll (#1895) --------- Co-authored-by: ElementBot --- changelog.d/1895.feature | 1 + .../poll/impl/create/CreatePollEvents.kt | 1 + .../poll/impl/create/CreatePollPresenter.kt | 29 +++++++-- .../poll/impl/create/CreatePollState.kt | 5 +- .../impl/create/CreatePollStateProvider.kt | 40 +++++++++--- .../poll/impl/create/CreatePollView.kt | 17 ++++- .../features/poll/impl/data/PollRepository.kt | 7 +++ .../impl/create/CreatePollPresenterTest.kt | 63 +++++++++++++++++-- ...ollView-Day-0_0_null_6,NEXUS_5,1.0,en].png | 4 +- ...ollView-Day-0_0_null_7,NEXUS_5,1.0,en].png | 3 + ...lView-Night-0_1_null_6,NEXUS_5,1.0,en].png | 4 +- ...lView-Night-0_1_null_7,NEXUS_5,1.0,en].png | 3 + 12 files changed, 153 insertions(+), 24 deletions(-) create mode 100644 changelog.d/1895.feature create mode 100644 tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Day-0_0_null_7,NEXUS_5,1.0,en].png create mode 100644 tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Night-0_1_null_7,NEXUS_5,1.0,en].png diff --git a/changelog.d/1895.feature b/changelog.d/1895.feature new file mode 100644 index 0000000000..a683847fce --- /dev/null +++ b/changelog.d/1895.feature @@ -0,0 +1 @@ +Add option to delete a poll while editing the poll \ No newline at end of file diff --git a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollEvents.kt b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollEvents.kt index b763edffa1..5831a73850 100644 --- a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollEvents.kt +++ b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollEvents.kt @@ -20,6 +20,7 @@ import io.element.android.libraries.matrix.api.poll.PollKind sealed interface CreatePollEvents { data object Save : CreatePollEvents + data class Delete(val confirmed: Boolean) : CreatePollEvents data class SetQuestion(val question: String) : CreatePollEvents data class SetAnswer(val index: Int, val text: String) : CreatePollEvents data object AddAnswer : CreatePollEvents diff --git a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollPresenter.kt b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollPresenter.kt index 58a32584e0..67faaa43da 100644 --- a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollPresenter.kt +++ b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollPresenter.kt @@ -67,7 +67,8 @@ class CreatePollPresenter @AssistedInject constructor( var question: String by rememberSaveable { mutableStateOf("") } var answers: List by rememberSaveable { mutableStateOf(listOf("", "")) } var pollKind: PollKind by rememberSaveable(saver = pollKindSaver) { mutableStateOf(PollKind.Disclosed) } - var showConfirmation: Boolean by rememberSaveable { mutableStateOf(false) } + var showBackConfirmation: Boolean by rememberSaveable { mutableStateOf(false) } + var showDeleteConfirmation: Boolean by rememberSaveable { mutableStateOf(false) } LaunchedEffect(Unit) { if (mode is CreatePollMode.EditPoll) { @@ -114,6 +115,22 @@ class CreatePollPresenter @AssistedInject constructor( Timber.d("Cannot create poll") } } + is CreatePollEvents.Delete -> { + if (mode !is CreatePollMode.EditPoll) { + return + } + + if (!event.confirmed) { + showDeleteConfirmation = true + return + } + + scope.launch { + showDeleteConfirmation = false + repository.deletePoll(mode.eventId) + navigateUp() + } + } is CreatePollEvents.AddAnswer -> { answers = answers + "" } @@ -137,12 +154,15 @@ class CreatePollPresenter @AssistedInject constructor( CreatePollEvents.ConfirmNavBack -> { val shouldConfirm = question.isNotBlank() || answers.any { it.isNotBlank() } if (shouldConfirm) { - showConfirmation = true + showBackConfirmation = true } else { navigateUp() } } - is CreatePollEvents.HideConfirmation -> showConfirmation = false + is CreatePollEvents.HideConfirmation -> { + showBackConfirmation = false + showDeleteConfirmation = false + } } } @@ -156,7 +176,8 @@ class CreatePollPresenter @AssistedInject constructor( question = question, answers = immutableAnswers, pollKind = pollKind, - showConfirmation = showConfirmation, + showBackConfirmation = showBackConfirmation, + showDeleteConfirmation = showDeleteConfirmation, eventSink = ::handleEvents, ) } diff --git a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollState.kt b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollState.kt index 23069e1d78..d3248be8a5 100644 --- a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollState.kt +++ b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollState.kt @@ -26,13 +26,16 @@ data class CreatePollState( val question: String, val answers: ImmutableList, val pollKind: PollKind, - val showConfirmation: Boolean, + val showBackConfirmation: Boolean, + val showDeleteConfirmation: Boolean, val eventSink: (CreatePollEvents) -> Unit, ) { enum class Mode { New, Edit, } + + val canDelete: Boolean = mode == Mode.Edit } data class Answer( diff --git a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollStateProvider.kt b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollStateProvider.kt index 0df5293b19..d6d843eff9 100644 --- a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollStateProvider.kt +++ b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollStateProvider.kt @@ -34,7 +34,8 @@ class CreatePollStateProvider : PreviewParameterProvider { Answer("", false) ), pollKind = PollKind.Disclosed, - showConfirmation = false, + showBackConfirmation = false, + showDeleteConfirmation = false, ), aCreatePollState( mode = CreatePollState.Mode.New, @@ -45,7 +46,8 @@ class CreatePollStateProvider : PreviewParameterProvider { Answer("Italian \uD83C\uDDEE\uD83C\uDDF9", false), Answer("Chinese \uD83C\uDDE8\uD83C\uDDF3", false), ), - showConfirmation = false, + showBackConfirmation = false, + showDeleteConfirmation = false, pollKind = PollKind.Undisclosed, ), aCreatePollState( @@ -57,7 +59,8 @@ class CreatePollStateProvider : PreviewParameterProvider { Answer("Italian \uD83C\uDDEE\uD83C\uDDF9", false), Answer("Chinese \uD83C\uDDE8\uD83C\uDDF3", false), ), - showConfirmation = true, + showBackConfirmation = true, + showDeleteConfirmation = false, pollKind = PollKind.Undisclosed, ), aCreatePollState( @@ -71,7 +74,8 @@ class CreatePollStateProvider : PreviewParameterProvider { Answer("Brazilian \uD83C\uDDE7\uD83C\uDDF7", true), Answer("French \uD83C\uDDEB\uD83C\uDDF7", true), ), - showConfirmation = false, + showBackConfirmation = false, + showDeleteConfirmation = false, pollKind = PollKind.Undisclosed, ), aCreatePollState( @@ -101,7 +105,8 @@ class CreatePollStateProvider : PreviewParameterProvider { Answer("19", true), Answer("20", true), ), - showConfirmation = false, + showBackConfirmation = false, + showDeleteConfirmation = false, pollKind = PollKind.Undisclosed, ), aCreatePollState( @@ -124,7 +129,8 @@ class CreatePollStateProvider : PreviewParameterProvider { false ), ), - showConfirmation = false, + showBackConfirmation = false, + showDeleteConfirmation = false, pollKind = PollKind.Undisclosed, ), aCreatePollState( @@ -137,7 +143,21 @@ class CreatePollStateProvider : PreviewParameterProvider { Answer("", false) ), pollKind = PollKind.Disclosed, - showConfirmation = false, + showDeleteConfirmation = false, + showBackConfirmation = false, + ), + aCreatePollState( + mode = CreatePollState.Mode.Edit, + canCreate = false, + canAddAnswer = true, + question = "", + answers = persistentListOf( + Answer("", false), + Answer("", false) + ), + pollKind = PollKind.Disclosed, + showDeleteConfirmation = true, + showBackConfirmation = false, ), ) } @@ -148,7 +168,8 @@ private fun aCreatePollState( canAddAnswer: Boolean, question: String, answers: PersistentList, - showConfirmation: Boolean, + showBackConfirmation: Boolean, + showDeleteConfirmation: Boolean, pollKind: PollKind ): CreatePollState { return CreatePollState( @@ -157,7 +178,8 @@ private fun aCreatePollState( canAddAnswer = canAddAnswer, question = question, answers = answers, - showConfirmation = showConfirmation, + showBackConfirmation = showBackConfirmation, + showDeleteConfirmation = showDeleteConfirmation, pollKind = pollKind, eventSink = {} ) diff --git a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollView.kt b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollView.kt index 6520450d20..a1c50332cb 100644 --- a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollView.kt +++ b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/create/CreatePollView.kt @@ -76,11 +76,19 @@ fun CreatePollView( val navBack = { state.eventSink(CreatePollEvents.ConfirmNavBack) } BackHandler(onBack = navBack) - if (state.showConfirmation) ConfirmationDialog( + if (state.showBackConfirmation) ConfirmationDialog( content = stringResource(id = R.string.screen_create_poll_cancel_confirmation_content_android), onSubmitClicked = { state.eventSink(CreatePollEvents.NavBack) }, onDismiss = { state.eventSink(CreatePollEvents.HideConfirmation) } ) + if (state.showDeleteConfirmation) { + ConfirmationDialog( + title = stringResource(id = R.string.screen_edit_poll_delete_confirmation_title), + content = stringResource(id = R.string.screen_edit_poll_delete_confirmation), + onSubmitClicked = { state.eventSink(CreatePollEvents.Delete(confirmed = true)) }, + onDismiss = { state.eventSink(CreatePollEvents.HideConfirmation) } + ) + } val questionFocusRequester = remember { FocusRequester() } val answerFocusRequester = remember { FocusRequester() } LaunchedEffect(Unit) { @@ -191,6 +199,13 @@ fun CreatePollView( onChange = { state.eventSink(CreatePollEvents.SetPollKind(if (it) PollKind.Undisclosed else PollKind.Disclosed)) }, ), ) + if (state.canDelete) { + ListItem( + headlineContent = { Text(text = stringResource(id = CommonStrings.action_delete_poll)) }, + style = ListItemStyle.Destructive, + onClick = { state.eventSink(CreatePollEvents.Delete(confirmed = false)) }, + ) + } } } } diff --git a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/data/PollRepository.kt b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/data/PollRepository.kt index d40c5a1df4..0b9aa5aee0 100644 --- a/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/data/PollRepository.kt +++ b/features/poll/impl/src/main/kotlin/io/element/android/features/poll/impl/data/PollRepository.kt @@ -59,4 +59,11 @@ class PollRepository @Inject constructor( pollKind = pollKind, ) } + + suspend fun deletePoll( + pollStartId: EventId, + ): Result = + room.redactEvent( + eventId = pollStartId, + ) } diff --git a/features/poll/impl/src/test/kotlin/io/element/android/features/poll/impl/create/CreatePollPresenterTest.kt b/features/poll/impl/src/test/kotlin/io/element/android/features/poll/impl/create/CreatePollPresenterTest.kt index 69046ab808..dae6b604f6 100644 --- a/features/poll/impl/src/test/kotlin/io/element/android/features/poll/impl/create/CreatePollPresenterTest.kt +++ b/features/poll/impl/src/test/kotlin/io/element/android/features/poll/impl/create/CreatePollPresenterTest.kt @@ -371,7 +371,7 @@ class CreatePollPresenterTest { }.test { val initial = awaitItem() Truth.assertThat(navUpInvocationsCount).isEqualTo(0) - Truth.assertThat(initial.showConfirmation).isFalse() + Truth.assertThat(initial.showBackConfirmation).isFalse() initial.eventSink(CreatePollEvents.ConfirmNavBack) Truth.assertThat(navUpInvocationsCount).isEqualTo(1) } @@ -386,12 +386,59 @@ class CreatePollPresenterTest { val initial = awaitItem() initial.eventSink(CreatePollEvents.SetQuestion("Non blank")) Truth.assertThat(navUpInvocationsCount).isEqualTo(0) - Truth.assertThat(awaitItem().showConfirmation).isFalse() + Truth.assertThat(awaitItem().showBackConfirmation).isFalse() initial.eventSink(CreatePollEvents.ConfirmNavBack) Truth.assertThat(navUpInvocationsCount).isEqualTo(0) - Truth.assertThat(awaitItem().showConfirmation).isTrue() + Truth.assertThat(awaitItem().showBackConfirmation).isTrue() initial.eventSink(CreatePollEvents.HideConfirmation) - Truth.assertThat(awaitItem().showConfirmation).isFalse() + Truth.assertThat(awaitItem().showBackConfirmation).isFalse() + } + } + + @Test + fun `delete confirms`() = runTest { + val presenter = createCreatePollPresenter(mode = CreatePollMode.EditPoll(pollEventId)) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + awaitDefaultItem() + awaitPollLoaded().eventSink(CreatePollEvents.Delete(confirmed = false)) + awaitDeleteConfirmation() + Truth.assertThat(fakeMatrixRoom.redactEventEventIdParam).isNull() + } + } + + @Test + fun `delete can be cancelled`() = runTest { + val presenter = createCreatePollPresenter(mode = CreatePollMode.EditPoll(pollEventId)) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + awaitDefaultItem() + awaitPollLoaded().eventSink(CreatePollEvents.Delete(confirmed = false)) + Truth.assertThat(fakeMatrixRoom.redactEventEventIdParam).isNull() + awaitDeleteConfirmation().eventSink(CreatePollEvents.HideConfirmation) + awaitPollLoaded().apply { + Truth.assertThat(showDeleteConfirmation).isFalse() + } + Truth.assertThat(fakeMatrixRoom.redactEventEventIdParam).isNull() + } + } + + @Test + fun `delete can be confirmed`() = runTest { + val presenter = createCreatePollPresenter(mode = CreatePollMode.EditPoll(pollEventId)) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + awaitDefaultItem() + awaitPollLoaded().eventSink(CreatePollEvents.Delete(confirmed = false)) + Truth.assertThat(fakeMatrixRoom.redactEventEventIdParam).isNull() + awaitDeleteConfirmation().eventSink(CreatePollEvents.Delete(confirmed = true)) + awaitPollLoaded().apply { + Truth.assertThat(showDeleteConfirmation).isFalse() + } + Truth.assertThat(fakeMatrixRoom.redactEventEventIdParam).isEqualTo(pollEventId) } } @@ -402,7 +449,13 @@ class CreatePollPresenterTest { Truth.assertThat(question).isEmpty() Truth.assertThat(answers).isEqualTo(listOf(Answer("", false), Answer("", false))) Truth.assertThat(pollKind).isEqualTo(PollKind.Disclosed) - Truth.assertThat(showConfirmation).isFalse() + Truth.assertThat(showBackConfirmation).isFalse() + Truth.assertThat(showDeleteConfirmation).isFalse() + } + + private suspend fun TurbineTestContext.awaitDeleteConfirmation() = + awaitItem().apply { + Truth.assertThat(showDeleteConfirmation).isTrue() } private suspend fun TurbineTestContext.awaitPollLoaded( diff --git a/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Day-0_0_null_6,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Day-0_0_null_6,NEXUS_5,1.0,en].png index 4880eb0cf8..dd72111031 100644 --- a/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Day-0_0_null_6,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Day-0_0_null_6,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:3f9afba1a28a22dface99d2359ca4a29134b3f8c9768ac71f7a492ffb07af356 -size 33406 +oid sha256:8e545dca45645bf7874d183697250ca6f8f916121e5a72a07409917a62d1dc94 +size 35440 diff --git a/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Day-0_0_null_7,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Day-0_0_null_7,NEXUS_5,1.0,en].png new file mode 100644 index 0000000000..6f8432a888 --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Day-0_0_null_7,NEXUS_5,1.0,en].png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:362bcdd3b7e32613da7b9f1a6a2dc23a097e8e9b671d185220d9a243326c5b4f +size 37556 diff --git a/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Night-0_1_null_6,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Night-0_1_null_6,NEXUS_5,1.0,en].png index c5b3d52d79..6b1952a412 100644 --- a/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Night-0_1_null_6,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Night-0_1_null_6,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:e51bd3544012bde205e05b3b3116a0a6a682b043ac9270a91a4bff3eaa6c91fd -size 31627 +oid sha256:0ccb4cce1db9083561047204aa570727c870a2d1974534fbfc8482b3eaf4fa53 +size 33628 diff --git a/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Night-0_1_null_7,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Night-0_1_null_7,NEXUS_5,1.0,en].png new file mode 100644 index 0000000000..e6c69623e8 --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/ui_S_t[f.poll.impl.create_CreatePollView_null_CreatePollView-Night-0_1_null_7,NEXUS_5,1.0,en].png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:9fd2419fbcfbf5df4847e47416f3717c1176c5e9e5b529503916f6413c47e5cf +size 32712