Skip to content

Role based device permissions request in CallComposite#2218

Merged
mgamis-msft merged 17 commits intomainfrom
mgamis/role-based-device-permissions-request
Aug 19, 2022
Merged

Role based device permissions request in CallComposite#2218
mgamis-msft merged 17 commits intomainfrom
mgamis/role-based-device-permissions-request

Conversation

@mgamis-msft
Copy link
Copy Markdown
Contributor

What

Porting over code from feature branch to ask device permissions based on role for a Rooms call.
Temporarily using a simplified copy of MockCallAdapter for enzyme test. Will move MockCallAdapter from hermetic tests to fake-backends package in this bug https://skype.visualstudio.com/SPOOL/_workitems/edit/2951482 so that it can be used in the enzyme test.

Why

To have public Calling sample ready to be deployed for testing

How Tested

Tested in local Calling sample

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 12, 2022

CallWithChat bundle size is increased❗.

  • Current size: 5446678
  • Base size: 5446579
  • Diff size: 99

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 12, 2022

Chat bundle size is not changed.

  • Current size: 5191587
  • Base size: 5191587
  • Diff size: 0

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 12, 2022

Calling bundle size is increased❗.

  • Current size: 5062165
  • Base size: 5062066
  • Diff size: 99

adapter.queryCameras();
adapter.queryMicrophones();
adapter.querySpeakers();
if (role === 'Consumer') {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

when we stop explicitly passing in our role. How are we going to know our role is consumer so we only ask for audio permissions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I hope I understand the question correctly. The CallComposite can only know the role is Consumer when it is passed as a prop because that role will be used for the PermissionProvider.

@github-actions
Copy link
Copy Markdown
Contributor

adapter.queryCameras();
adapter.queryMicrophones();
adapter.querySpeakers();
if (role === 'Consumer') {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess we need to conditionally compile this part?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Yes we do!

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@mgamis-msft mgamis-msft enabled auto-merge (squash) August 16, 2022 18:00
@github-actions
Copy link
Copy Markdown
Contributor

@mgamis-msft mgamis-msft merged commit a6590bd into main Aug 19, 2022
@mgamis-msft mgamis-msft deleted the mgamis/role-based-device-permissions-request branch August 19, 2022 08:39
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.

4 participants