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

fix(meetings): pass encrypted title based on APPI API for conversation #3166

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

Neeraj-swarnkar
Copy link
Collaborator

@Neeraj-swarnkar Neeraj-swarnkar commented Oct 26, 2023

COMPLETES #SPARK-469170

This pull request addresses

Experiencing a failure when attempting to start an instant space meeting URL: https://co.webex.com/wbxappapi/v2/meetings/spaceInstant.

The error message received is '416 Range Not Satisfiable' with the response '416020' and message 'e2ee decrypt error.'

by making the following changes

should pass encrypted title based on APPI API ("/wbxappapi/v2/meetings/spaceInstant) Team.
Updated the code and tests for this.

Screen.Recording.2023-11-02.at.10.58.40.AM.mov
Screenshot 2023-11-03 at 3 04 17 PM Screenshot 2023-11-03 at 3 03 50 PM Screenshot 2023-11-03 at 3 03 28 PM

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios where tested

< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >

Tested some basic manual flow and also attached the screen recording -

  • Register
  • Create ad-hoc meeting with conv URL
  • Join with media
  • Checked participants
  • checked if meeting is started in space or not
  • leave

I certified that

  • I have read and followed contributing guidelines

  • I discussed changes with code owners prior to submitting this pull request

  • I have not skipped any automated checks

  • All existing and new tests passed

  • I have updated the documentation accordingly


Make sure to have followed the contributing guidelines before submitting.

@Neeraj-swarnkar
Copy link
Collaborator Author

@arun3528, Tested locally, I will test this again once amplify preview link is generated.

@aws-amplify-us-east-2
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-3166.d3m3l2kee0btzx.amplifyapp.com

@@ -151,7 +151,7 @@ export default class MeetingInfoV2 {
};

return this.webex
.request(conversationUrl)
.request(conversationUrl, {disableTransform: true})
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice looks like it take the url and options separately also

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes @arun3528, looks like.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@arun3528, should we have includeParticipants: true, also, I don't think that is required, because it's working fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added screen recording after testing in amplify preview link.
cc - @arun3528

Copy link
Collaborator

Choose a reason for hiding this comment

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

in the query string it should say includeParticipant , it might break in future so better add that and check the query string for the api call

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2023-11-03 at 3 03 28 PM Verified. @arun3528

@Neeraj-swarnkar Neeraj-swarnkar changed the base branch from master to next October 27, 2023 05:46
@Neeraj-swarnkar Neeraj-swarnkar changed the base branch from next to master October 27, 2023 05:46
@sreenara sreenara added the validated If the pull request is validated for automation. label Oct 27, 2023
Copy link
Contributor

@mkesavan13 mkesavan13 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just one minor comment!

@Neeraj-swarnkar Neeraj-swarnkar marked this pull request as ready for review October 27, 2023 10:12
@Neeraj-swarnkar Neeraj-swarnkar requested a review from a team as a code owner October 27, 2023 10:12
@Neeraj-swarnkar Neeraj-swarnkar requested a review from a team as a code owner October 27, 2023 10:28
@Neeraj-swarnkar Neeraj-swarnkar force-pushed the SPARK-469170 branch 3 times, most recently from d6e641a to 62d4f85 Compare November 2, 2023 05:34
pass encrypted title based on APPI API for conversation
@arun3528 arun3528 merged commit 080d786 into webex:master Nov 3, 2023
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validated If the pull request is validated for automation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants