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

CallTileTimelineItem.CallStatus.MISSED renders both missed and reject… #5089

Merged
merged 2 commits into from
Feb 2, 2022

Conversation

langleyd
Copy link
Member

Fixes #5088

The if statement switches between CallStatus.MISSED and CallStatus.ENDED. CallStatus.MISSED renders both missed calls and not answered calls. So i think the logic makes more sense to check that the call was not answered vs the old code which checks all events are from one side.

@langleyd langleyd self-assigned this Jan 28, 2022
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, once again, could be nice to have @ganfra's blessing

@@ -101,7 +101,7 @@ class CallItemFactory @Inject constructor(
createCallTileTimelineItem(
roomSummary = roomSummary,
callId = callEventGrouper.callId,
callStatus = if (callEventGrouper.callWasMissed()) CallTileTimelineItem.CallStatus.MISSED else CallTileTimelineItem.CallStatus.ENDED,
callStatus = if (!callEventGrouper.callWasAnswered()) CallTileTimelineItem.CallStatus.MISSED else CallTileTimelineItem.CallStatus.ENDED,
Copy link
Member

Choose a reason for hiding this comment

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

Can I ask you to remove ! and swap if and else blocks to simplify reading?

Copy link
Contributor

Choose a reason for hiding this comment

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

was about to say the same thing 😄

Copy link
Member

Choose a reason for hiding this comment

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

:). also the CI complains that the line is too long

Copy link
Member

Choose a reason for hiding this comment

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

Done

@bmarty bmarty requested a review from ganfra January 28, 2022 09:18
@github-actions
Copy link

github-actions bot commented Jan 28, 2022

Unit Test Results

  76 files  +4    76 suites  +4   1m 1s ⏱️ +5s
143 tests +2  143 ✔️ +2  0 💤 ±0  0 ±0 
448 runs  +8  448 ✔️ +8  0 💤 ±0  0 ±0 

Results for commit a78d88f. ± Comparison against base commit ff6810f.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

Matrix SDK

Integration Tests Results:

  • [org.matrix.android.sdk.session]
    passed="21" failures="0" errors="0" skipped="2"
  • [org.matrix.android.sdk.account]
    passed="5" failures="0" errors="0" skipped="2"
  • [org.matrix.android.sdk.internal]
    passed="158" failures="13" errors="0" skipped="38"
  • [org.matrix.android.sdk.ordering]
    passed="16" failures="0" errors="0" skipped="0"
  • [org.matrix.android.sdk.PermalinkParserTest]
    passed="2" failures="0" errors="0" skipped="0"

@bmarty bmarty enabled auto-merge February 2, 2022 11:04
@bmarty bmarty disabled auto-merge February 2, 2022 11:05
@bmarty bmarty merged commit 54fb28c into develop Feb 2, 2022
@bmarty bmarty deleted the feature/dla/fix_incorrect_call_status branch February 2, 2022 11:06
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.

Fix call status timeline events mixed up(ended, missed and declined)
3 participants