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

Threads: added support to read receipts (MSC3771) #1617

Merged
merged 6 commits into from
Nov 4, 2022

Conversation

gileluard
Copy link
Contributor

@gileluard gileluard marked this pull request as draft October 24, 2022 20:30
@gileluard gileluard requested a review from ismailgulek October 25, 2022 07:55
Copy link
Contributor

@ismailgulek ismailgulek left a comment

Choose a reason for hiding this comment

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

Had some inline comments, otherwise lgtm

MatrixSDK/Data/MXRoom.h Show resolved Hide resolved
MatrixSDK/JSONModels/MXEvent.m Outdated Show resolved Hide resolved
MatrixSDK/Data/Store/MXFileStore/MXFileStore.m Outdated Show resolved Hide resolved
MatrixSDK/Data/Store/MXMemoryStore/MXMemoryStore.h Outdated Show resolved Hide resolved
MatrixSDK/JSONModels/MXEvent.h Outdated Show resolved Hide resolved
MatrixSDKTests/MXReceiptDataIntegrationTests.swift Outdated Show resolved Hide resolved
MatrixSDK/Contrib/Swift/MXRestClient.swift Show resolved Hide resolved
}
}

[list addObject:threadId ?: [NSNull null]];
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we use kMXEventTimelineMain instead of NSNull?

Copy link
Contributor Author

@gileluard gileluard Nov 1, 2022

Choose a reason for hiding this comment

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

We still must support RR without thread ID (unthreaded RR) sent by old clients. In this case we have to set the thread ID as nil so the RR will be stored in all thread and main timelines as per MSC.

@gileluard gileluard marked this pull request as ready for review November 1, 2022 21:49
@gileluard gileluard requested a review from ismailgulek November 2, 2022 08:35
Copy link
Contributor

@ismailgulek ismailgulek left a comment

Choose a reason for hiding this comment

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

A tiny comment inline, otherwise LGTM!

MatrixSDK/Data/MXRoom.h Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Base: 16.16% // Head: 16.07% // Decreases project coverage by -0.08% ⚠️

Coverage data is based on head (03d3104) compared to base (cf61531).
Patch coverage: 2.09% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1617      +/-   ##
===========================================
- Coverage    16.16%   16.07%   -0.09%     
===========================================
  Files          580      581       +1     
  Lines        91432    91980     +548     
  Branches     38555    38807     +252     
===========================================
+ Hits         14777    14789      +12     
- Misses       76176    76711     +535     
- Partials       479      480       +1     
Impacted Files Coverage Δ
MatrixSDK/Background/MXBackgroundStore.swift 0.00% <0.00%> (ø)
MatrixSDK/Contrib/Swift/MXRestClient.swift 1.55% <0.00%> (ø)
...xSDK/Data/EventTimeline/Room/MXRoomEventTimeline.m 5.24% <0.00%> (-0.03%) ⬇️
MatrixSDK/Data/MXRoom.m 1.68% <0.00%> (-0.03%) ⬇️
MatrixSDK/Data/MXRoomSummary.m 7.67% <0.00%> (ø)
MatrixSDK/Data/Store/MXFileStore/MXFileStore.m 17.99% <0.00%> (-0.03%) ⬇️
MatrixSDK/Data/Store/MXMemoryStore/MXReceiptData.m 0.00% <0.00%> (ø)
MatrixSDK/JSONModels/MXEvent.m 34.06% <0.00%> (-1.20%) ⬇️
MatrixSDK/MXRestClient.m 2.45% <0.00%> (-0.01%) ⬇️
MatrixSDK/Threads/MXThreadingService.swift 19.67% <0.00%> (-0.61%) ⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@gileluard gileluard requested a review from ismailgulek November 2, 2022 14:24
@gileluard gileluard merged commit 9d73c93 into develop Nov 4, 2022
@gileluard gileluard deleted the gil/6663-MSC3771_read_receipts_for_threads branch November 4, 2022 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants