Skip to content

Call transfer with consult fails to make outgoing consultation call. #5203

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

Merged
merged 6 commits into from
Feb 14, 2022

Conversation

langleyd
Copy link
Member

@langleyd langleyd commented Feb 10, 2022

Fixes #5201

First part of fix

Intent information(EXTRA_MODE) that is pass to attachViewRenderers here is not present on intent when onNewIntent is called, so the outgoing call is never placed. Fix is to set the intent here.

Second part of fix

A follow on issue was related to a recent PR that puts other caller on hold when going to the transfer selection and off hold when they cancel.

With consult transfers, As the consult call is started in a new task, the call transfer selection activity immediately gets returned with RESULT_CANCELLED (rather than RESULT_OK from the Complete view action in turn calling setResult). This means we do an additional unintended unhold and this places the new call in the incorrect hold state.

2 options for a solution

  • Start changing how the call activity stack works(changing launch options and flags etc)
  • Use the transfer screen just for transfer selection, Return the selection as an activity result and do the actual transfer back on the actual call activity/view model.

The second option seemed pretty reasonable to me and we actually already do the second part of the consult transfer from the call view model already. So went with that.

@langleyd langleyd self-assigned this Feb 10, 2022
@github-actions
Copy link

github-actions bot commented Feb 10, 2022

Unit Test Results

  76 files  ±0    76 suites  ±0   59s ⏱️ -1s
144 tests ±0  144 ✔️ ±0  0 💤 ±0  0 ±0 
452 runs  ±0  452 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit ffd2a76. ± Comparison against base commit 00ada67.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Feb 10, 2022

Matrix SDK

Integration Tests Results:

  • [org.matrix.android.sdk.session]
    passed="25" failures="17" 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="1" 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"

@langleyd langleyd marked this pull request as ready for review February 11, 2022 10:04
Copy link
Contributor

@ouchadam ouchadam left a comment

Choose a reason for hiding this comment

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

small non blocking question around whether we can remove a file, otherwise looking good! 💯

@ouchadam ouchadam requested a review from ganfra February 11, 2022 11:21
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, i have push a small cleanup commit.

@bmarty bmarty enabled auto-merge February 14, 2022 16:32
@bmarty bmarty merged commit 6c4f389 into develop Feb 14, 2022
@bmarty bmarty deleted the feature/dla/fix_consult_transfer branch February 14, 2022 18:54
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.

Call transfer with consult fails to make outgoing consultation call.
3 participants