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

Update getMeetingDetailsVerbose Interface #2489

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

MSJohnnyLee
Copy link
Contributor

@MSJohnnyLee MSJohnnyLee commented Aug 30, 2024

For more information about how to contribute to this repo, visit this page.

Description

This PR modifies the return interface of the getMeetingDetailsVerbose API. Specifically, the response.details.originalCaller and response.details.dialedEntity properties.

originalCaller represents the caller of a call, and dialedEntity represents the callee. Those two properties originally were of type string, and the return value could've been either an email or phone number in string format.

With this change, two additional properties were added to replace them: originalCallerInfo and dialedEntityInfo. Those two properties are of a new type ICallParticipantIdentifiers. This new type ICallParticipantIdentifiers is an object that has 2 explicit fields for phoneNumber and email.

This new type serves two purposes: 1. The returned value is more explicitly defined 2. The response can return both a phone number and email if that information can be retrieved.

In addition, this change also a new optional property, callId.

This PR is a follow up to the original PR that first introduced the getMeetingDetailsVerbose API:
#2206

Main changes in the PR:

  1. Create new interface ICallParticipantIdentifiers
  2. Deprecate response.details.originalCaller and response.details.dialedEntity
  3. Create two new, optional properties response.details.originalCallerInfo and response.details.dialedEntityInfo of the new interface ICallParticipantIdentifiers
  4. Create new optional property callId

Validation

Validation performed:

  1. Validated in Orange that the new meeting.getMeetingDetailsVerbose API returns the new response.details.originalCaller and response.details.dialedEntity properties in a supported host
  2. Ran automated test cases

Unit Tests added:

Updated existing test

End-to-end tests added:

Updated existing test

Additional Requirements

Change file added:

Yes, added change file

Related PRs:

Original getMeetingDetailsVerbose PR: #2206

@MSJohnnyLee MSJohnnyLee marked this pull request as ready for review August 30, 2024 19:02
@MSJohnnyLee MSJohnnyLee requested a review from a team as a code owner August 30, 2024 19:02
jadahiya-MSFT
jadahiya-MSFT previously approved these changes Sep 25, 2024
* @hidden
* Hide from docs.
*/
export class EmailAddress {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you could put this in its own file (emailAddress.ts) in this folder I would really appreciate it. Same for the unit tests being in their own test file. I'd really like us to start using classes like this in other capabilities as well, so moving this to its own file will make it easier to find and share.

If you do this, remember to add the class to the index.ts file (look for where the appId.ts file is referenced for an example)

@@ -2260,4 +2266,26 @@ describe('meeting', () => {
});
});
});
describe('utility', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please split these into separate tests (one test for each invalid case, and then a single test for all of the valid ones together is probably okay) This will make it more obvious what is wrong if something breaks.

Copy link
Contributor

@TrevorJoelHarris TrevorJoelHarris 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, just one last request to split EmailAddress and its test into separate files

@@ -0,0 +1,6 @@
export function validateEmailAddress(emailString: string): void {
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
if (!emailRegex.test(emailString)) {

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
that depends on
library input
may run slow on strings starting with '!@!.' and with many repetitions of '!.'.
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.

3 participants