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.

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

Fixes ended poll voting #5473

Merged
merged 20 commits into from
Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e4ce4ab
Adds server side vote blocking for ended polls
ericdecanini Mar 9, 2022
080844d
Removes event timestamp condition for sdk poll voting
ericdecanini Mar 10, 2022
628a160
Reverts timestamp condition but changes timing of setting closedTime
ericdecanini Mar 10, 2022
df6ae4b
Fixes warning for debugging
ericdecanini Mar 10, 2022
fe3c9cc
Reverts to fix by removing event timestamp condition
ericdecanini Mar 10, 2022
6db1c37
Reverts to fix by removing event timestamp condition
ericdecanini Mar 10, 2022
6a59007
Reverts debug isPollActive
ericdecanini Mar 10, 2022
610c67c
Refactors buildPollItem in MessageItemFactory
ericdecanini Mar 13, 2022
f24d8c2
Merge remote-tracking branch 'origin/develop' into bugfix/eric/voting…
ericdecanini Mar 13, 2022
86765c9
Adds new line at the end of PollState
ericdecanini Mar 13, 2022
06e0047
Fixes import ordering in DefaultNavigator
ericdecanini Mar 13, 2022
9806f1b
Merge remote-tracking branch 'origin/develop' into bugfix/eric/voting…
ericdecanini Mar 15, 2022
98b7d19
Adds getBestX following merge from develop
ericdecanini Mar 15, 2022
a2d18d4
Fixes PollMode import error in TimelineFragment
ericdecanini Mar 15, 2022
a46901a
Makes PollState a sealed interface
ericdecanini Mar 15, 2022
9c8f29e
Merge branch 'develop' into bugfix/eric/voting-ended-poll
ericdecanini Mar 17, 2022
d11fc06
Fixes crash due to empty constructor in CreatePollViewState
ericdecanini Mar 17, 2022
fbb6f11
Fixes remote echo of end poll not processing correctly
ericdecanini Mar 17, 2022
7449d15
Merge remote-tracking branch 'origin/develop' into bugfix/eric/voting…
ericdecanini Mar 22, 2022
edfe81c
Merge remote-tracking branch 'origin/develop' into bugfix/eric/voting…
ericdecanini Mar 24, 2022
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
1 change: 1 addition & 0 deletions changelog.d/5473.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes polls being votable after being ended
Original file line number Diff line number Diff line change
Expand Up @@ -482,46 +482,39 @@ internal class EventRelationsAggregationProcessor @Inject constructor(
roomId: String,
isLocalEcho: Boolean) {
val pollEventId = content.relatesTo?.eventId ?: return

val pollOwnerId = getPollEvent(roomId, pollEventId)?.root?.senderId
val isPollOwner = pollOwnerId == event.senderId

val powerLevelsHelper = stateEventDataSource.getStateEvent(roomId, EventType.STATE_ROOM_POWER_LEVELS, QueryStringValue.NoCondition)
?.content?.toModel<PowerLevelsContent>()
?.let { PowerLevelsHelper(it) }

if (!isPollOwner && !powerLevelsHelper?.isUserAbleToRedact(event.senderId ?: "").orFalse()) {
Timber.v("## Received poll.end event $pollEventId but user ${event.senderId} doesn't have enough power level in room $roomId")
return
}

var existing = EventAnnotationsSummaryEntity.where(realm, roomId, pollEventId).findFirst()
if (existing == null) {
var existingPoll = EventAnnotationsSummaryEntity.where(realm, roomId, pollEventId).findFirst()
if (existingPoll == null) {
Timber.v("## POLL creating new relation summary for $pollEventId")
existing = EventAnnotationsSummaryEntity.create(realm, roomId, pollEventId)
existingPoll = EventAnnotationsSummaryEntity.create(realm, roomId, pollEventId)
}

// we have it
val existingPollSummary = existing.pollResponseSummary
val existingPollSummary = existingPoll.pollResponseSummary
?: realm.createObject(PollResponseAggregatedSummaryEntity::class.java).also {
existing.pollResponseSummary = it
existingPoll.pollResponseSummary = it
}

if (existingPollSummary.closedTime != null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source of the bug is here. A local echo being processed before the remote echo meant that the remote echo never reaches past this point. closedTime thus never gets set from the remote and always turns out to be the current time millis of the local device that ended the poll. If the device has time that's out of sync with unix current time, this bug can occur

cc @onurays

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for the clarification.

Timber.v("## Received poll.end event for already ended poll $pollEventId")
return
}

val txId = event.unsignedData?.transactionId
existingPollSummary.closedTime = event.originServerTs

// is it a remote echo?
if (!isLocalEcho && existingPollSummary.sourceLocalEchoEvents.contains(txId)) {
// ok it has already been managed
Timber.v("## POLL Receiving remote echo of response eventId:$pollEventId")
existingPollSummary.sourceLocalEchoEvents.remove(txId)
existingPollSummary.sourceEvents.add(event.eventId)
return
}

existingPollSummary.closedTime = event.originServerTs
}

private fun getPollEvent(roomId: String, eventId: String): TimelineEvent? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ import im.vector.app.features.notifications.NotificationDrawerManager
import im.vector.app.features.notifications.NotificationUtils
import im.vector.app.features.permalink.NavigationInterceptor
import im.vector.app.features.permalink.PermalinkHandler
import im.vector.app.features.poll.create.PollMode
import im.vector.app.features.poll.PollMode
import im.vector.app.features.reactions.EmojiReactionPickerActivity
import im.vector.app.features.roomprofile.RoomProfileActivity
import im.vector.app.features.session.coroutineScope
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ abstract class PollItem : AbsMessageItem<PollItem.Holder>() {
var eventId: String? = null

@EpoxyAttribute
var pollSent: Boolean = false
var canVote: Boolean = false

@EpoxyAttribute
var totalVotesText: String? = null

@EpoxyAttribute
var edited: Boolean = false
@EpoxyAttribute
var edited: Boolean = false

@EpoxyAttribute
lateinit var optionViewStates: List<PollOptionViewState>
Expand All @@ -54,7 +54,6 @@ abstract class PollItem : AbsMessageItem<PollItem.Holder>() {

override fun bind(holder: Holder) {
super.bind(holder)
val relatedEventId = eventId ?: return

renderSendState(holder.view, holder.questionTextView)

Expand All @@ -73,13 +72,19 @@ abstract class PollItem : AbsMessageItem<PollItem.Holder>() {
optionViewStates.forEachIndexed { index, optionViewState ->
views.getOrNull(index)?.let {
it.render(optionViewState)
it.setOnClickListener {
callback?.onTimelineItemAction(RoomDetailAction.VoteToPoll(relatedEventId, optionViewState.optionId))
}
it.setOnClickListener { onPollItemClick(optionViewState) }
}
}
}

private fun onPollItemClick(optionViewState: PollOptionViewState) {
val relatedEventId = eventId

if (canVote && relatedEventId != null) {
callback?.onTimelineItemAction(RoomDetailAction.VoteToPoll(relatedEventId, optionViewState.optionId))
}
}

class Holder : AbsMessageItem.Holder(STUB_ID) {
val questionTextView by bind<TextView>(R.id.questionTextView)
val optionsContainer by bind<LinearLayout>(R.id.optionsContainer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ import im.vector.app.features.onboarding.OnboardingActivity
import im.vector.app.features.pin.PinActivity
import im.vector.app.features.pin.PinArgs
import im.vector.app.features.pin.PinMode
import im.vector.app.features.poll.PollMode
import im.vector.app.features.poll.create.CreatePollActivity
import im.vector.app.features.poll.create.CreatePollArgs
import im.vector.app.features.poll.create.PollMode
import im.vector.app.features.roomdirectory.RoomDirectoryActivity
import im.vector.app.features.roomdirectory.RoomDirectoryData
import im.vector.app.features.roomdirectory.createroom.CreateRoomActivity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import im.vector.app.features.location.LocationSharingMode
import im.vector.app.features.login.LoginConfig
import im.vector.app.features.media.AttachmentData
import im.vector.app.features.pin.PinMode
import im.vector.app.features.poll.create.PollMode
import im.vector.app.features.poll.PollMode
import im.vector.app.features.roomdirectory.RoomDirectoryData
import im.vector.app.features.roomdirectory.roompreview.RoomPreviewData
import im.vector.app.features.settings.VectorSettingsActivity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package im.vector.app.features.poll.create
package im.vector.app.features.poll

enum class PollMode {
CREATE,
Expand Down
27 changes: 27 additions & 0 deletions vector/src/main/java/im/vector/app/features/poll/PollState.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c) 2022 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package im.vector.app.features.poll

sealed interface PollState {
object Sending : PollState
object Ready : PollState
data class Voted(val votes: Int) : PollState
object Undisclosed : PollState
object Ended : PollState

fun isVotable() = this !is Sending && this !is Ended
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import im.vector.app.R
import im.vector.app.core.extensions.configureWith
import im.vector.app.core.platform.VectorBaseFragment
import im.vector.app.databinding.FragmentCreatePollBinding
import im.vector.app.features.poll.PollMode
import im.vector.app.features.poll.create.CreatePollViewModel.Companion.MAX_OPTIONS_COUNT
import kotlinx.parcelize.Parcelize
import org.matrix.android.sdk.api.session.room.model.message.PollType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import dagger.assisted.AssistedInject
import im.vector.app.core.di.MavericksAssistedViewModelFactory
import im.vector.app.core.di.hiltMavericksViewModelFactory
import im.vector.app.core.platform.VectorViewModel
import im.vector.app.features.poll.PollMode
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.room.model.message.MessagePollContent
import org.matrix.android.sdk.api.session.room.model.message.PollType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,18 @@
package im.vector.app.features.poll.create

import com.airbnb.mvrx.MavericksState
import im.vector.app.features.poll.PollMode
import org.matrix.android.sdk.api.session.room.model.message.PollType

data class CreatePollViewState(
val roomId: String,
val editedEventId: String?,
val mode: PollMode,
val question: String = "",
val options: List<String> = List(CreatePollViewModel.MIN_OPTIONS_COUNT) { "" },
val canCreatePoll: Boolean = false,
val canAddMoreOptions: Boolean = true,
val pollType: PollType = PollType.DISCLOSED_UNSTABLE
val roomId: String,
val editedEventId: String?,
val mode: PollMode,
val question: String = "",
val options: List<String> = List(CreatePollViewModel.MIN_OPTIONS_COUNT) { "" },
val canCreatePoll: Boolean = false,
val canAddMoreOptions: Boolean = true,
val pollType: PollType = PollType.DISCLOSED_UNSTABLE
) : MavericksState {

constructor(args: CreatePollArgs) : this(
Expand Down