diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/PollTablesTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/PollTablesTest.kt index dea0771dce..f4a8b5271b 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/PollTablesTest.kt +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/PollTablesTest.kt @@ -2,6 +2,7 @@ package org.thoughtcrime.securesms.database import androidx.test.ext.junit.runners.AndroidJUnit4 import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test @@ -100,7 +101,7 @@ class PollTablesTest { val voteCount = SignalDatabase.polls.insertVote(poll, pollOption) assertEquals(1, voteCount) - assertEquals(listOf(0), SignalDatabase.polls.getVotes(poll.id, false)) + assertEquals(listOf(0), SignalDatabase.polls.getVotes(poll.id, false, voteCount)) } @Test @@ -110,23 +111,25 @@ class PollTablesTest { val pollOption = poll.pollOptions.first() val voteCount = SignalDatabase.polls.removeVote(poll, pollOption) - SignalDatabase.polls.markPendingAsRemoved(poll.id, Recipient.self().id.toLong(), voteCount, 1) + SignalDatabase.polls.markPendingAsRemoved(poll.id, Recipient.self().id.toLong(), voteCount, 1, pollOption.id) assertEquals(1, voteCount) - val status = SignalDatabase.polls.getPollVoteStateForGivenVote(poll.id, voteCount) - assertEquals(PollTables.VoteState.REMOVED, status) + val votes = SignalDatabase.polls.getVotes(poll.id, false, voteCount) + assertTrue(votes.isEmpty()) } @Test - fun givenAVote_whenISetPollOptionId_thenOptionIdIsUpdated() { - SignalDatabase.polls.insertPoll("how do you feel about unit testing?", false, listOf("yay", "ok", "nay"), 1, 1) + fun givenAPendingVote_whenIRevertThatVote_thenItGoesToMostRecentResolvedState() { + SignalDatabase.polls.insertPoll("how do you feel about unit testing?", true, listOf("yay", "ok", "nay"), 1, 1) val poll = SignalDatabase.polls.getPoll(1)!! val option = poll.pollOptions.first() SignalDatabase.polls.insertVotes(poll.id, listOf(option.id), Recipient.self().id.toLong(), 5, MessageId(1)) - SignalDatabase.polls.setPollVoteStateForGivenVote(poll.id, Recipient.self().id.toLong(), 5, 1, true) - val status = SignalDatabase.polls.getPollVoteStateForGivenVote(poll.id, 5) + SignalDatabase.polls.markPendingAsAdded(poll.id, Recipient.self().id.toLong(), 5, 1, option.id) + SignalDatabase.polls.removeVote(poll, option) - assertEquals(PollTables.VoteState.ADDED, status) + SignalDatabase.polls.removePendingVote(poll.id, option.id, 6, 1) + val votes = SignalDatabase.polls.getVotes(1, true, 6) + assertEquals(listOf(0), votes) } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/PollComponent.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/PollComponent.kt index f678dc65a3..22682c51da 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/PollComponent.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/PollComponent.kt @@ -13,6 +13,8 @@ import androidx.compose.animation.fadeIn import androidx.compose.animation.fadeOut import androidx.compose.animation.togetherWith import androidx.compose.foundation.background +import androidx.compose.foundation.clickable +import androidx.compose.foundation.interaction.MutableInteractionSource import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -26,6 +28,7 @@ import androidx.compose.foundation.layout.wrapContentHeight import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.ButtonDefaults import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -34,12 +37,14 @@ import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.hapticfeedback.HapticFeedbackType import androidx.compose.ui.platform.ComposeView import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalHapticFeedback import androidx.compose.ui.res.colorResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.res.vectorResource import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import org.signal.core.ui.compose.Buttons @@ -50,6 +55,7 @@ import org.thoughtcrime.securesms.R import org.thoughtcrime.securesms.components.compose.RoundCheckbox import org.thoughtcrime.securesms.polls.PollOption import org.thoughtcrime.securesms.polls.PollRecord +import org.thoughtcrime.securesms.polls.VoteState import org.thoughtcrime.securesms.polls.Voter import org.thoughtcrime.securesms.util.DynamicTheme import org.thoughtcrime.securesms.util.VibrateUtil @@ -150,33 +156,80 @@ private fun PollOption( ) { if (!hasEnded) { AnimatedContent( - targetState = option.isPending, + targetState = option.voteState, transitionSpec = { - val enterTransition = fadeIn(tween(delayMillis = 500, durationMillis = 500)) + val delayMs = if (option.voteState == VoteState.PENDING_REMOVE || option.voteState == VoteState.PENDING_ADD) 500 else 0 + val enterTransition = fadeIn(tween(delayMillis = delayMs, durationMillis = 500)) val exitTransition = fadeOut(tween(durationMillis = 500)) enterTransition.togetherWith(exitTransition) .using(SizeTransform(clip = false)) } - ) { inProgress -> - if (inProgress) { - CircularProgressIndicator( - modifier = Modifier.padding(top = 4.dp, end = 8.dp).size(24.dp), - strokeWidth = 1.5.dp, - color = pollColors.checkbox - ) - } else { - RoundCheckbox( - checked = option.isSelected, - onCheckedChange = { checked -> - if (VibrateUtil.isHapticFeedbackEnabled(context)) { - haptics.performHapticFeedback(if (checked) HapticFeedbackType.ToggleOn else HapticFeedbackType.ToggleOff) - } - onToggleVote(option, checked) - }, - modifier = Modifier.padding(top = 4.dp, end = 8.dp).height(24.dp), - outlineColor = pollColors.checkbox, - checkedColor = pollColors.checkboxBackground - ) + ) { voteState -> + when (voteState) { + VoteState.PENDING_ADD -> { + Box( + modifier = Modifier + .clickable( + onClick = { + if (VibrateUtil.isHapticFeedbackEnabled(context)) { + haptics.performHapticFeedback(HapticFeedbackType.ToggleOff) + } + onToggleVote(option, false) + }, + interactionSource = remember { MutableInteractionSource() }, + indication = null, + onClickLabel = stringResource(R.string.SignalCheckbox_accessibility_on_click_label), + enabled = true + ) + ) { + CircularProgressIndicator( + modifier = Modifier.padding(top = 4.dp, end = 8.dp).size(24.dp), + strokeWidth = 1.5.dp, + color = pollColors.checkbox + ) + Icon( + imageVector = ImageVector.vectorResource(R.drawable.symbol_check_compact_bold_16), + contentDescription = stringResource(R.string.SignalCheckbox_accessibility_unchecked_description), + tint = pollColors.checkbox, + modifier = Modifier.padding(top = 4.dp, end = 8.dp).align(Alignment.Center) + ) + } + } + VoteState.PENDING_REMOVE -> { + CircularProgressIndicator( + modifier = Modifier.padding(top = 4.dp, end = 8.dp).size(24.dp) + .clickable( + onClick = { + if (VibrateUtil.isHapticFeedbackEnabled(context)) { + haptics.performHapticFeedback(HapticFeedbackType.ToggleOn) + } + onToggleVote(option, true) + }, + interactionSource = remember { MutableInteractionSource() }, + indication = null, + onClickLabel = stringResource(R.string.SignalCheckbox_accessibility_on_click_label), + enabled = true + ), + strokeWidth = 1.5.dp, + color = pollColors.checkbox + ) + } + VoteState.ADDED, + VoteState.REMOVED, + VoteState.NONE -> { + RoundCheckbox( + checked = voteState == VoteState.ADDED, + onCheckedChange = { checked -> + if (VibrateUtil.isHapticFeedbackEnabled(context)) { + haptics.performHapticFeedback(if (checked) HapticFeedbackType.ToggleOn else HapticFeedbackType.ToggleOff) + } + onToggleVote(option, checked) + }, + modifier = Modifier.padding(top = 4.dp, end = 8.dp).height(24.dp), + outlineColor = pollColors.checkbox, + checkedColor = pollColors.checkboxBackground + ) + } } } } @@ -190,7 +243,7 @@ private fun PollOption( color = pollColors.text ) - if (hasEnded && option.isSelected) { + if (hasEnded && option.voteState == VoteState.ADDED) { RoundCheckbox( checked = true, onCheckedChange = {}, @@ -290,7 +343,7 @@ private fun PollPreview() { id = 1, question = "How do you feel about compose previews?", pollOptions = listOf( - PollOption(1, "yay", listOf(Voter(1, 1)), isSelected = true), + PollOption(1, "yay", listOf(Voter(1, 1)), voteState = VoteState.ADDED), PollOption(2, "ok", listOf(Voter(1, 1), Voter(2, 1))), PollOption(3, "nay", listOf(Voter(1, 1), Voter(2, 1), Voter(3, 1))) ), @@ -313,8 +366,8 @@ private fun EmptyPollPreview() { question = "How do you feel about multiple compose previews?", pollOptions = listOf( PollOption(1, "yay", emptyList()), - PollOption(2, "ok", emptyList(), isSelected = true), - PollOption(3, "nay", emptyList(), isSelected = true) + PollOption(2, "ok", emptyList()), + PollOption(3, "nay", emptyList()) ), allowMultipleVotes = true, hasEnded = false, @@ -335,7 +388,7 @@ private fun FinishedPollPreview() { question = "How do you feel about finished compose previews?", pollOptions = listOf( PollOption(1, "yay", listOf(Voter(1, 1))), - PollOption(2, "ok", emptyList(), isSelected = true), + PollOption(2, "ok", emptyList()), PollOption(3, "nay", emptyList()) ), allowMultipleVotes = false, diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/clicklisteners/PollVotesFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/clicklisteners/PollVotesFragment.kt index c32562fd82..22cebb4bb6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/clicklisteners/PollVotesFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/clicklisteners/PollVotesFragment.kt @@ -220,7 +220,7 @@ private fun PollResultsScreenPreview() { question = "How do you feel about finished compose previews?", pollOptions = listOf( PollOption(1, "Yay", listOf(Voter(1, 1), Voter(12, 1), Voter(3, 1))), - PollOption(2, "Ok", listOf(Voter(2, 1), Voter(4, 1)), isSelected = true), + PollOption(2, "Ok", listOf(Voter(2, 1), Voter(4, 1))), PollOption(3, "Nay", emptyList()) ), allowMultipleVotes = false, diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt index e2a2c6a6d9..ff068fed70 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt @@ -659,7 +659,8 @@ class ConversationViewModel( val pollVoteJob = PollVoteJob.create( messageId = poll.messageId, voteCount = voteCount, - isRemoval = !isChecked + isRemoval = !isChecked, + optionId = pollOption.id ) if (pollVoteJob != null) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/PollTables.kt b/app/src/main/java/org/thoughtcrime/securesms/database/PollTables.kt index b7c83fd442..b75e16f71f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/PollTables.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/PollTables.kt @@ -6,6 +6,7 @@ import androidx.core.content.contentValuesOf import org.signal.core.util.SqlUtil import org.signal.core.util.delete import org.signal.core.util.exists +import org.signal.core.util.forEach import org.signal.core.util.groupBy import org.signal.core.util.insertInto import org.signal.core.util.logging.Log @@ -30,6 +31,7 @@ import org.thoughtcrime.securesms.polls.Poll import org.thoughtcrime.securesms.polls.PollOption import org.thoughtcrime.securesms.polls.PollRecord import org.thoughtcrime.securesms.polls.PollVote +import org.thoughtcrime.securesms.polls.VoteState import org.thoughtcrime.securesms.polls.Voter import org.thoughtcrime.securesms.recipients.Recipient import org.thoughtcrime.securesms.recipients.RecipientId @@ -143,8 +145,7 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT $VOTER_ID INTEGER NOT NULL REFERENCES ${RecipientTable.TABLE_NAME} (${RecipientTable.ID}) ON DELETE CASCADE, $VOTE_COUNT INTEGER, $DATE_RECEIVED INTEGER DEFAULT 0, - $VOTE_STATE INTEGER DEFAULT 0, - UNIQUE($POLL_ID, $VOTER_ID, $POLL_OPTION_ID) ON CONFLICT REPLACE + $VOTE_STATE INTEGER DEFAULT 0 ) """ @@ -224,6 +225,8 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT /** * Inserts a vote in a poll and increases the vote count by 1. * Status is marked as [VoteState.PENDING_ADD] here and then once it successfully sends, it will get updated to [VoteState.ADDED] in [markPendingAsAdded] + * If the latest state is already pending for this vote, it will just update the pending state. + * However, if there is a resolved state eg [VoteState.ADDED], we add a new entry so that if the pending entry fails, we can revert back to a known state. */ fun insertVote(poll: PollRecord, pollOption: PollOption): Int { val self = Recipient.self().id.toLong() @@ -231,17 +234,36 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT writableDatabase.withinTransaction { db -> voteCount = getCurrentPollVoteCount(poll.id, self) + 1 - val contentValues = ContentValues().apply { - put(PollVoteTable.POLL_ID, poll.id) - put(PollVoteTable.POLL_OPTION_ID, pollOption.id) - put(PollVoteTable.VOTER_ID, self) - put(PollVoteTable.VOTE_COUNT, voteCount) - put(PollVoteTable.VOTE_STATE, VoteState.PENDING_ADD.value) - } - db.insertInto(PollVoteTable.TABLE_NAME) - .values(contentValues) - .run(SQLiteDatabase.CONFLICT_REPLACE) + if (isPending(poll.id, pollOption.id, self)) { + db.update(PollVoteTable.TABLE_NAME) + .values( + PollVoteTable.VOTE_STATE to VoteState.PENDING_ADD.value, + PollVoteTable.VOTE_COUNT to voteCount + ) + .where( + """ + ${PollVoteTable.POLL_ID} = ? AND + ${PollVoteTable.POLL_OPTION_ID} = ? AND + ${PollVoteTable.VOTER_ID} = ? AND + (${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_ADD.value} OR ${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_REMOVE.value}) + """.trimIndent(), + poll.id, + pollOption.id, + self + ) + .run() + } else { + db.insertInto(PollVoteTable.TABLE_NAME) + .values( + PollVoteTable.POLL_ID to poll.id, + PollVoteTable.POLL_OPTION_ID to pollOption.id, + PollVoteTable.VOTER_ID to self, + PollVoteTable.VOTE_COUNT to voteCount, + PollVoteTable.VOTE_STATE to VoteState.PENDING_ADD.value + ) + .run(SQLiteDatabase.CONFLICT_REPLACE) + } } AppDependencies.databaseObserver.notifyMessageUpdateObservers(MessageId(poll.messageId)) @@ -252,25 +274,43 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT * Once a vote is sent to at least one person, we can update the [VoteState.PENDING_ADD] state to [VoteState.ADDED]. * If the poll only allows one vote, it also clears out any old votes. */ - fun markPendingAsAdded(pollId: Long, voterId: Long, voteCount: Int, messageId: Long) { + fun markPendingAsAdded(pollId: Long, voterId: Long, voteCount: Int, messageId: Long, optionId: Long) { val poll = SignalDatabase.polls.getPollFromId(pollId) if (poll == null) { Log.w(TAG, "Cannot find poll anymore $pollId") return } - writableDatabase.updateWithOnConflict( - PollVoteTable.TABLE_NAME, - contentValuesOf(PollVoteTable.VOTE_STATE to VoteState.ADDED.value), - "${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.VOTER_ID} = ? AND ${PollVoteTable.VOTE_COUNT} = ? AND ${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_ADD.value}", - SqlUtil.buildArgs(pollId, voterId, voteCount), - SQLiteDatabase.CONFLICT_REPLACE - ) - - if (!poll.allowMultipleVotes) { - writableDatabase.delete(PollVoteTable.TABLE_NAME) - .where("${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.VOTER_ID} = ? AND ${PollVoteTable.VOTE_COUNT} < ?", poll.id, Recipient.self().id, voteCount) + writableDatabase.withinTransaction { db -> + // Clear out any old voting history for this specific voter option combination + db.delete(PollVoteTable.TABLE_NAME) + .where( + """ + ${PollVoteTable.POLL_ID} = ? AND + ${PollVoteTable.POLL_OPTION_ID} = ? AND + ${PollVoteTable.VOTER_ID} = ? AND + ${PollVoteTable.VOTE_COUNT} < ? + """.trimIndent(), + pollId, + optionId, + voterId, + voteCount + ) .run() + + db.updateWithOnConflict( + PollVoteTable.TABLE_NAME, + contentValuesOf(PollVoteTable.VOTE_STATE to VoteState.ADDED.value), + "${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.VOTER_ID} = ? AND ${PollVoteTable.VOTE_COUNT} = ? AND ${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_ADD.value}", + SqlUtil.buildArgs(pollId, voterId, voteCount), + SQLiteDatabase.CONFLICT_REPLACE + ) + + if (!poll.allowMultipleVotes) { + db.delete(PollVoteTable.TABLE_NAME) + .where("${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.VOTER_ID} = ? AND ${PollVoteTable.VOTE_COUNT} < ?", poll.id, Recipient.self().id, voteCount) + .run() + } } AppDependencies.databaseObserver.notifyMessageUpdateObservers(MessageId(messageId)) } @@ -278,6 +318,8 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT /** * Removes vote from a poll. This also increases the vote count because removal of a vote, is technically a type of vote. * Status is marked as [VoteState.PENDING_REMOVE] here and then once it successfully sends, it will get updated to [VoteState.REMOVED] in [markPendingAsRemoved] + * If the latest state is already a pending state for this vote, it will just update the pending state. + * However, if there is a resolved state eg [VoteState.REMOVED], we add a new entry so that if the pending entry fails, we can revert back to a known state. */ fun removeVote(poll: PollRecord, pollOption: PollOption): Int { val self = Recipient.self().id.toLong() @@ -285,15 +327,30 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT writableDatabase.withinTransaction { db -> voteCount = getCurrentPollVoteCount(poll.id, self) + 1 - db.insertInto(PollVoteTable.TABLE_NAME) - .values( - PollVoteTable.POLL_ID to poll.id, - PollVoteTable.POLL_OPTION_ID to pollOption.id, - PollVoteTable.VOTER_ID to self, - PollVoteTable.VOTE_COUNT to voteCount, - PollVoteTable.VOTE_STATE to VoteState.PENDING_REMOVE.value - ) - .run(SQLiteDatabase.CONFLICT_REPLACE) + if (isPending(poll.id, pollOption.id, self)) { + db.update(PollVoteTable.TABLE_NAME) + .values( + PollVoteTable.VOTE_STATE to VoteState.PENDING_REMOVE.value, + PollVoteTable.VOTE_COUNT to voteCount + ) + .where( + "${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.POLL_OPTION_ID} = ? AND ${PollVoteTable.VOTER_ID} = ? AND (${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_ADD.value} OR ${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_REMOVE.value})", + poll.id, + pollOption.id, + self + ) + .run() + } else { + db.insertInto(PollVoteTable.TABLE_NAME) + .values( + PollVoteTable.POLL_ID to poll.id, + PollVoteTable.POLL_OPTION_ID to pollOption.id, + PollVoteTable.VOTER_ID to self, + PollVoteTable.VOTE_COUNT to voteCount, + PollVoteTable.VOTE_STATE to VoteState.PENDING_REMOVE.value + ) + .run() + } } AppDependencies.databaseObserver.notifyMessageUpdateObservers(MessageId(poll.messageId)) @@ -303,8 +360,24 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT /** * Once a vote is sent to at least one person, we can update the [VoteState.PENDING_REMOVE] state to [VoteState.REMOVED]. */ - fun markPendingAsRemoved(pollId: Long, voterId: Long, voteCount: Int, messageId: Long) { + fun markPendingAsRemoved(pollId: Long, voterId: Long, voteCount: Int, messageId: Long, optionId: Long) { writableDatabase.withinTransaction { db -> + // Clear out any old voting history for this specific voter option combination + db.delete(PollVoteTable.TABLE_NAME) + .where( + """ + ${PollVoteTable.POLL_ID} = ? AND + ${PollVoteTable.VOTER_ID} = ? AND + ${PollVoteTable.POLL_OPTION_ID} = ? AND + ${PollVoteTable.VOTE_COUNT} < ? + """.trimIndent(), + pollId, + voterId, + optionId, + voteCount + ) + .run() + db.updateWithOnConflict( PollVoteTable.TABLE_NAME, contentValuesOf(PollVoteTable.VOTE_STATE to VoteState.REMOVED.value), @@ -317,17 +390,21 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT } /** - * For a given poll, returns the option indexes that the person has voted for + * For a given poll, returns the option indexes that the person has voted for. + * Because we store both pending and resolved states of votes, we need to make + * sure that when getting votes, we only get the latest vote. We do this by + * sorting our query by vote count (higher vote counts are the more recent) and + * then for each option, only getting the latest such vote if applicable. */ - fun getVotes(pollId: Long, allowMultipleVotes: Boolean): List { + fun getVotes(pollId: Long, allowMultipleVotes: Boolean, voteCount: Int): List { val voteQuery = if (allowMultipleVotes) { - "(${PollVoteTable.VOTE_STATE} = ${VoteState.ADDED.value} OR ${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_ADD.value})" + "${PollVoteTable.VOTE_COUNT} <= ?" } else { - "${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_ADD.value}" + "${PollVoteTable.VOTE_COUNT} = ?" } - return readableDatabase - .select(PollOptionTable.OPTION_ORDER) + val votesStates = readableDatabase + .select(PollOptionTable.OPTION_ORDER, PollVoteTable.VOTE_STATE) .from("${PollVoteTable.TABLE_NAME} LEFT JOIN ${PollOptionTable.TABLE_NAME} ON ${PollVoteTable.TABLE_NAME}.${PollVoteTable.POLL_OPTION_ID} = ${PollOptionTable.TABLE_NAME}.${PollOptionTable.ID}") .where( """ @@ -337,10 +414,24 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT $voteQuery """, pollId, - Recipient.self().id.toLong() + Recipient.self().id.toLong(), + voteCount ) + .orderBy("${PollVoteTable.VOTE_COUNT} DESC") .run() - .readToList { cursor -> cursor.requireInt(PollOptionTable.OPTION_ORDER) } + .groupBy { cursor -> + cursor.requireInt(PollOptionTable.OPTION_ORDER) to cursor.requireInt(PollVoteTable.VOTE_STATE) + } + + val votes = votesStates.filter { + if (allowMultipleVotes) { + it.value.first() == VoteState.PENDING_ADD.value || it.value.first() == VoteState.ADDED.value + } else { + it.value.first() == VoteState.PENDING_ADD.value + } + } + + return votes.keys.toList() } /** @@ -365,33 +456,27 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT } /** - * Returns the [VoteState] for a given voting session (as indicated by voteCount) + * When a vote job fails, delete the pending vote so that it falls back to the most recently resolved (not pending) + * [VoteState] prior to this voting session. If the pending vote was the only entry, its deletion is equal to having not voted. */ - fun getPollVoteStateForGivenVote(pollId: Long, voteCount: Int): VoteState { - val value = readableDatabase - .select(PollVoteTable.VOTE_STATE) - .from(PollVoteTable.TABLE_NAME) - .where("${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.VOTER_ID} = ? AND ${PollVoteTable.VOTE_COUNT} = ?", pollId, Recipient.self().id.toLong(), voteCount) - .run() - .readToSingleInt() - return VoteState.fromValue(value) - } + fun removePendingVote(pollId: Long, optionId: Long, voteCount: Int, messageId: Long) { + Log.w(TAG, "Pending vote failed, reverting vote at $voteCount") - /** - * Sets the [VoteState] for a given voting session (as indicated by voteCount) - */ - fun setPollVoteStateForGivenVote(pollId: Long, voterId: Long, voteCount: Int, messageId: Long, undoRemoval: Boolean) { - val state = if (undoRemoval) VoteState.ADDED.value else VoteState.REMOVED.value writableDatabase.withinTransaction { db -> - db.updateWithOnConflict( - PollVoteTable.TABLE_NAME, - contentValuesOf( - PollVoteTable.VOTE_STATE to state - ), - "${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.VOTER_ID} = ? AND ${PollVoteTable.VOTE_COUNT} = ?", - SqlUtil.buildArgs(pollId, voterId, voteCount), - SQLiteDatabase.CONFLICT_REPLACE - ) + db.delete(PollVoteTable.TABLE_NAME) + .where( + """ + ${PollVoteTable.POLL_ID} = ? AND + ${PollVoteTable.POLL_OPTION_ID} = ? AND + ${PollVoteTable.VOTER_ID} = ? AND + ${PollVoteTable.VOTE_COUNT} = ? + """.trimIndent(), + pollId, + optionId, + Recipient.self().id.toLong(), + voteCount + ) + .run() } AppDependencies.databaseObserver.notifyMessageUpdateObservers(MessageId(messageId)) } @@ -537,10 +622,19 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT .readToMap { cursor -> val pollId = cursor.requireLong(PollTable.ID) val pollVotes = getPollVotes(pollId) - val pendingVotes = getPendingVotes(pollId) + val (pendingAdds, pendingRemoves) = getPendingVotes(pollId) val pollOptions = getPollOptions(pollId).map { option -> val voters = pollVotes[option.key] ?: emptyList() - PollOption(id = option.key, text = option.value, voters = voters, isSelected = voters.any { it.id == self }, isPending = pendingVotes.contains(option.key)) + val voteState = if (pendingAdds.contains(option.key)) { + VoteState.PENDING_ADD + } else if (pendingRemoves.contains(option.key)) { + VoteState.PENDING_REMOVE + } else if (voters.any { it.id == self }) { + VoteState.ADDED + } else { + VoteState.NONE + } + PollOption(id = option.key, text = option.value, voters = voters, voteState = voteState) } val poll = PollRecord( id = pollId, @@ -606,6 +700,23 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT .readToSingleBoolean() } + private fun isPending(pollId: Long, optionId: Long, voterId: Long): Boolean { + return readableDatabase + .exists(PollVoteTable.TABLE_NAME) + .where( + """ + ${PollVoteTable.POLL_ID} = ? AND + ${PollVoteTable.POLL_OPTION_ID} = ? AND + ${PollVoteTable.VOTER_ID} = ? AND + (${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_ADD.value} OR ${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_REMOVE.value}) + """, + pollId, + optionId, + voterId + ) + .run() + } + private fun getPollOptions(pollId: Long): Map { return readableDatabase .select(PollOptionTable.ID, PollOptionTable.OPTION_TEXT) @@ -621,22 +732,33 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT return readableDatabase .select(PollVoteTable.POLL_OPTION_ID, PollVoteTable.VOTER_ID, PollVoteTable.VOTE_COUNT) .from(PollVoteTable.TABLE_NAME) - .where("${PollVoteTable.POLL_ID} = ? AND (${PollVoteTable.VOTE_STATE} = ${VoteState.ADDED.value} OR ${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_REMOVE.value})", pollId) + .where("${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.VOTE_STATE} = ${VoteState.ADDED.value}", pollId) .run() .groupBy { cursor -> cursor.requireLong(PollVoteTable.POLL_OPTION_ID) to Voter(id = cursor.requireLong(PollVoteTable.VOTER_ID), voteCount = cursor.requireInt(PollVoteTable.VOTE_COUNT)) } } - private fun getPendingVotes(pollId: Long): List { - return readableDatabase - .select(PollVoteTable.POLL_OPTION_ID) + private fun getPendingVotes(pollId: Long): Pair, List> { + val pendingAdds = mutableListOf() + val pendingRemoves = mutableListOf() + readableDatabase + .select(PollVoteTable.POLL_OPTION_ID, PollVoteTable.VOTE_STATE) .from(PollVoteTable.TABLE_NAME) - .where("${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.VOTER_ID} = ? AND (${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_ADD.value} OR ${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_REMOVE.value})", pollId, Recipient.self().id) + .where( + "${PollVoteTable.POLL_ID} = ? AND ${PollVoteTable.VOTER_ID} = ? AND (${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_ADD.value} OR ${PollVoteTable.VOTE_STATE} = ${VoteState.PENDING_REMOVE.value})", + pollId, + Recipient.self().id + ) .run() - .readToList { cursor -> - cursor.requireLong(PollVoteTable.POLL_OPTION_ID) + .forEach { cursor -> + if (cursor.requireInt(PollVoteTable.VOTE_STATE) == VoteState.PENDING_ADD.value) { + pendingAdds += cursor.requireLong(PollVoteTable.POLL_OPTION_ID) + } else { + pendingRemoves += cursor.requireLong(PollVoteTable.POLL_OPTION_ID) + } } + return Pair(pendingAdds, pendingRemoves) } private fun getPollOptionText(pollId: Long): List { @@ -670,25 +792,4 @@ class PollTables(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT ) } } - - enum class VoteState(val value: Int) { - /** We have no information on the vote state */ - NONE(0), - - /** Vote is in the process of being removed */ - PENDING_REMOVE(1), - - /** Vote is in the process of being added */ - PENDING_ADD(2), - - /** Vote was removed */ - REMOVED(3), - - /** Vote was added */ - ADDED(4); - - companion object { - fun fromValue(value: Int) = VoteState.entries.first { it.value == value } - } - } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt index e70e6d751e..00870b7df4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt @@ -149,6 +149,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V291_NullOutRemoteK import org.thoughtcrime.securesms.database.helpers.migration.V292_AddPollTables import org.thoughtcrime.securesms.database.helpers.migration.V294_RemoveLastResortKeyTupleColumnConstraintMigration import org.thoughtcrime.securesms.database.helpers.migration.V295_AddLastRestoreKeyTypeTableIfMissingMigration +import org.thoughtcrime.securesms.database.helpers.migration.V296_RemovePollVoteConstraint import org.thoughtcrime.securesms.database.SQLiteDatabase as SignalSqliteDatabase /** @@ -304,10 +305,11 @@ object SignalDatabaseMigrations { 292 to V292_AddPollTables, // 293 to V293_LastResortKeyTupleTableMigration, - removed due to crashing on some devices. 294 to V294_RemoveLastResortKeyTupleColumnConstraintMigration, - 295 to V295_AddLastRestoreKeyTypeTableIfMissingMigration + 295 to V295_AddLastRestoreKeyTypeTableIfMissingMigration, + 296 to V296_RemovePollVoteConstraint ) - const val DATABASE_VERSION = 295 + const val DATABASE_VERSION = 296 @JvmStatic fun migrate(context: Application, db: SignalSqliteDatabase, oldVersion: Int, newVersion: Int) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V296_RemovePollVoteConstraint.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V296_RemovePollVoteConstraint.kt new file mode 100644 index 0000000000..32739c036a --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V296_RemovePollVoteConstraint.kt @@ -0,0 +1,57 @@ +/* + * Copyright 2025 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.database.helpers.migration + +import android.app.Application +import org.thoughtcrime.securesms.database.SQLiteDatabase + +/** + * Removes the UNIQUE constraint from the poll vote table + */ +@Suppress("ClassName") +object V296_RemovePollVoteConstraint : SignalDatabaseMigration { + override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { + db.execSQL("DROP INDEX IF EXISTS poll_vote_poll_id_index") + db.execSQL("DROP INDEX IF EXISTS poll_vote_poll_option_id_index") + db.execSQL("DROP INDEX IF EXISTS poll_vote_voter_id_index") + + db.execSQL( + """ + CREATE TABLE poll_vote_tmp ( + _id INTEGER PRIMARY KEY AUTOINCREMENT, + poll_id INTEGER NOT NULL REFERENCES poll (_id) ON DELETE CASCADE, + poll_option_id INTEGER DEFAULT NULL REFERENCES poll_option (_id) ON DELETE CASCADE, + voter_id INTEGER NOT NULL REFERENCES recipient (_id) ON DELETE CASCADE, + vote_count INTEGER, + date_received INTEGER DEFAULT 0, + vote_state INTEGER DEFAULT 0 + ) + """ + ) + + db.execSQL( + """ + INSERT INTO poll_vote_tmp + SELECT + _id, + poll_id, + poll_option_id, + voter_id, + vote_count, + date_received, + vote_state + FROM poll_vote + """ + ) + + db.execSQL("DROP TABLE poll_vote") + db.execSQL("ALTER TABLE poll_vote_tmp RENAME TO poll_vote") + + db.execSQL("CREATE INDEX poll_vote_poll_id_index ON poll_vote (poll_id)") + db.execSQL("CREATE INDEX poll_vote_poll_option_id_index ON poll_vote (poll_option_id)") + db.execSQL("CREATE INDEX poll_vote_voter_id_index ON poll_vote (voter_id)") + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PollVoteJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/PollVoteJob.kt index f3599dc257..7b82d43e9a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PollVoteJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PollVoteJob.kt @@ -1,7 +1,6 @@ package org.thoughtcrime.securesms.jobs import org.signal.core.util.logging.Log -import org.thoughtcrime.securesms.database.PollTables import org.thoughtcrime.securesms.database.SignalDatabase import org.thoughtcrime.securesms.database.model.MessageId import org.thoughtcrime.securesms.groups.GroupId @@ -29,6 +28,7 @@ class PollVoteJob( private val initialRecipientCount: Int, private val voteCount: Int, private val isRemoval: Boolean, + private val optionId: Long, parameters: Parameters ) : Job(parameters) { @@ -36,7 +36,7 @@ class PollVoteJob( const val KEY: String = "PollVoteJob" private val TAG = Log.tag(PollVoteJob::class.java) - fun create(messageId: Long, voteCount: Int, isRemoval: Boolean): PollVoteJob? { + fun create(messageId: Long, voteCount: Int, isRemoval: Boolean, optionId: Long): PollVoteJob? { val message = SignalDatabase.messages.getMessageRecordOrNull(messageId) if (message == null) { Log.w(TAG, "Unable to find corresponding message") @@ -57,6 +57,7 @@ class PollVoteJob( initialRecipientCount = recipients.size, voteCount = voteCount, isRemoval = isRemoval, + optionId = optionId, parameters = Parameters.Builder() .setQueue(conversationRecipient.id.toQueueKey()) .addConstraint(NetworkConstraint.KEY) @@ -68,7 +69,7 @@ class PollVoteJob( } override fun serialize(): ByteArray { - return PollVoteJobData(messageId, recipientIds, initialRecipientCount, voteCount, isRemoval).encode() + return PollVoteJobData(messageId, recipientIds, initialRecipientCount, voteCount, isRemoval, optionId).encode() } override fun getFactoryKey(): String { @@ -126,7 +127,7 @@ class PollVoteJob( } private fun deliver(conversationRecipient: Recipient, destinations: List, targetAuthor: Recipient, targetSentTimestamp: Long, poll: PollRecord): List { - val votes = SignalDatabase.polls.getVotes(poll.id, poll.allowMultipleVotes) + val votes = SignalDatabase.polls.getVotes(poll.id, poll.allowMultipleVotes, voteCount) val dataMessageBuilder = newBuilder() .withTimestamp(System.currentTimeMillis()) @@ -169,14 +170,16 @@ class PollVoteJob( pollId = poll.id, voterId = Recipient.self().id.toLong(), voteCount = voteCount, - messageId = poll.messageId + messageId = poll.messageId, + optionId = optionId ) } else { SignalDatabase.polls.markPendingAsAdded( pollId = poll.id, voterId = Recipient.self().id.toLong(), voteCount = voteCount, - messageId = poll.messageId + messageId = poll.messageId, + optionId = optionId ) } } @@ -198,17 +201,7 @@ class PollVoteJob( return } - val voteState = SignalDatabase.polls.getPollVoteStateForGivenVote(pollId, voteCount) - - if (isRemoval && voteState == PollTables.VoteState.PENDING_REMOVE) { - Log.w(TAG, "Vote removal failed so we are adding it back") - SignalDatabase.polls.setPollVoteStateForGivenVote(pollId, Recipient.self().id.toLong(), voteCount, messageId, isRemoval) - } else if (!isRemoval && voteState == PollTables.VoteState.PENDING_ADD) { - Log.w(TAG, "Voting failed so we are removing it") - SignalDatabase.polls.setPollVoteStateForGivenVote(pollId, Recipient.self().id.toLong(), voteCount, messageId, isRemoval) - } else { - Log.w(TAG, "Voting state does not match what we'd expect, so ignoring.") - } + SignalDatabase.polls.removePendingVote(pollId, optionId, voteCount, messageId) } private fun buildPollVote( @@ -235,6 +228,7 @@ class PollVoteJob( initialRecipientCount = data.initialRecipientCount, voteCount = data.voteCount, isRemoval = data.isRemoval, + optionId = data.optionId, parameters = parameters ) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/polls/PollOption.kt b/app/src/main/java/org/thoughtcrime/securesms/polls/PollOption.kt index a238d74ad2..ab6c9680ae 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/polls/PollOption.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/polls/PollOption.kt @@ -11,6 +11,5 @@ data class PollOption( val id: Long, val text: String, val voters: List, - val isSelected: Boolean = false, - val isPending: Boolean = false + val voteState: VoteState = VoteState.NONE ) : Parcelable diff --git a/app/src/main/java/org/thoughtcrime/securesms/polls/VoteState.kt b/app/src/main/java/org/thoughtcrime/securesms/polls/VoteState.kt new file mode 100644 index 0000000000..45a203bf81 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/polls/VoteState.kt @@ -0,0 +1,27 @@ +package org.thoughtcrime.securesms.polls + +/** + * Tracks general state information when a user votes in a poll. Vote states are specific to an option in a poll + * eg. in a poll with three options, each option can have a different states like (PENDING_ADD, ADDED, NONE) + */ +enum class VoteState(val value: Int) { + + /** We have no information on the vote state */ + NONE(0), + + /** Vote is in the process of being removed */ + PENDING_REMOVE(1), + + /** Vote is in the process of being added */ + PENDING_ADD(2), + + /** Vote was removed */ + REMOVED(3), + + /** Vote was added */ + ADDED(4); + + companion object { + fun fromValue(value: Int) = VoteState.entries.first { it.value == value } + } +} diff --git a/app/src/main/protowire/JobData.proto b/app/src/main/protowire/JobData.proto index e0afe63946..e17d8f186e 100644 --- a/app/src/main/protowire/JobData.proto +++ b/app/src/main/protowire/JobData.proto @@ -250,4 +250,5 @@ message PollVoteJobData { uint32 initialRecipientCount = 3; uint32 voteCount = 4; bool isRemoval = 5; + uint64 optionId = 6; } \ No newline at end of file diff --git a/app/src/spinner/java/org/thoughtcrime/securesms/SpinnerApplicationContext.kt b/app/src/spinner/java/org/thoughtcrime/securesms/SpinnerApplicationContext.kt index 00096cc390..59dc92d953 100644 --- a/app/src/spinner/java/org/thoughtcrime/securesms/SpinnerApplicationContext.kt +++ b/app/src/spinner/java/org/thoughtcrime/securesms/SpinnerApplicationContext.kt @@ -20,6 +20,7 @@ import org.thoughtcrime.securesms.database.LogDatabase import org.thoughtcrime.securesms.database.MegaphoneDatabase import org.thoughtcrime.securesms.database.MessageBitmaskColumnTransformer import org.thoughtcrime.securesms.database.MessageRangesTransformer +import org.thoughtcrime.securesms.database.PollTransformer import org.thoughtcrime.securesms.database.ProfileKeyCredentialTransformer import org.thoughtcrime.securesms.database.QueryMonitor import org.thoughtcrime.securesms.database.RecipientTransformer @@ -70,7 +71,8 @@ class SpinnerApplicationContext : ApplicationContext() { MessageRangesTransformer, KyberKeyTransformer, RecipientTransformer, - AttachmentTransformer + AttachmentTransformer, + PollTransformer ) ), "jobmanager" to DatabaseConfig(db = { JobDatabase.getInstance(this).sqlCipherDatabase }, columnTransformers = listOf(TimestampTransformer)), diff --git a/app/src/spinner/java/org/thoughtcrime/securesms/database/PollTransformer.kt b/app/src/spinner/java/org/thoughtcrime/securesms/database/PollTransformer.kt new file mode 100644 index 0000000000..6fb6767cd1 --- /dev/null +++ b/app/src/spinner/java/org/thoughtcrime/securesms/database/PollTransformer.kt @@ -0,0 +1,17 @@ +package org.thoughtcrime.securesms.database + +import android.database.Cursor +import org.signal.core.util.requireInt +import org.signal.spinner.ColumnTransformer +import org.thoughtcrime.securesms.polls.VoteState + +object PollTransformer : ColumnTransformer { + override fun matches(tableName: String?, columnName: String): Boolean { + return columnName == PollTables.PollVoteTable.VOTE_STATE && (tableName == null || tableName == PollTables.PollVoteTable.TABLE_NAME) + } + + override fun transform(tableName: String?, columnName: String, cursor: Cursor): String? { + val voteState = VoteState.fromValue(cursor.requireInt(PollTables.PollVoteTable.VOTE_STATE)) + return "${cursor.requireInt(PollTables.PollVoteTable.VOTE_STATE)}

$voteState" + } +}