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

Jump to bottom

Feature/edit e2e #384

Merged
merged 3 commits into from
Jul 18, 2019
Merged

Feature/edit e2e #384

merged 3 commits into from
Jul 18, 2019

Conversation

BillCarsonFr
Copy link
Member

Fixes #143
Fixes #314
Fixes #152

@BillCarsonFr
E2E replies
077396a 
+ Edit History / support e2e and use original event
@BillCarsonFr
Fix / edit reply was quoting wrong text
7d41352 
+ e2e reply of edit
@BillCarsonFr BillCarsonFr requested a review from bmarty July 17, 2019 14:51
bmarty
bmarty reviewed Jul 17, 2019
View reviewed changes
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

some comment

...rc/main/java/im/vector/matrix/android/internal/session/room/EventRelationsAggregationTask.kt
keysClaimed = result.claimedEd25519Key?.let { k -> mapOf("ed25519" to k) },
forwardingCurve25519KeyChain = result.forwardingCurve25519KeyChain
)
} catch (e: MXCryptoError) {
Copy link
Member

Choose a reason for hiding this comment

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

do something here?

...n/java/im/vector/riotx/features/home/room/detail/timeline/action/ViewEditHistoryViewModel.kt
keysClaimed = result.claimedEd25519Key?.let { k -> mapOf("ed25519" to k) },
forwardingCurve25519KeyChain = result.forwardingCurve25519KeyChain
)
} catch (e: MXCryptoError) {
Copy link
Member

Choose a reason for hiding this comment

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

same remark

...n/java/im/vector/riotx/features/home/room/detail/timeline/action/ViewEditHistoryViewModel.kt Outdated
}
}

if (it.eventId == eventId) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not comfortable with the apply here, because, there is 3 different eventId in the class...

@BillCarsonFr
Copy link
Member Author

BillCarsonFr commented Jul 18, 2019 •
edited
Loading

I need to create a new Issue to handle decryption error when doing incremental edit aggregation.
Currently if the keys are not known when doing edit aggregation, the replace will be ignore and lost (the message won't be marked as edited, and in some cases showing the orginal event).
The EventRelationsAggregationTask need somehow to keep track of failed to decrypt relation, and listen for new keys in order to retry

#388

@BillCarsonFr BillCarsonFr requested a review from bmarty July 18, 2019 07:32
bmarty
bmarty approved these changes Jul 18, 2019
View reviewed changes
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

LGTM

@BillCarsonFr BillCarsonFr merged commit d87ee32 into develop Jul 18, 2019
@BillCarsonFr BillCarsonFr deleted the feature/edit_e2e branch July 18, 2019 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmarty bmarty bmarty approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@BillCarsonFr @bmarty