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: Teams Meeting Notification interface structure changes #4416

Merged
merged 8 commits into from
Feb 10, 2023

Conversation

singhk97
Copy link
Collaborator

@singhk97 singhk97 commented Jan 24, 2023

Fixes #4415

Description

  • Refactored types and interfaces associated with sendMeetingNotification api call, so that it can be extended without any breaking changes. And by 'extend`, I mean adding new types, making-optional existing properties and adding properties to existing interfaces.

Specific Changes

  • Made channelData an optional property
  • Updated the type of onBehalfOf property to be a list of MeetingOnBehalfOf objects.
  • Notification payload type expected the notification parameter in sendMeetingNotification API function is MeetingNotification. Currently it is equal to TargetedMeetingNotification. However it can made be a union of any arbitrary type. This makes the payload type extensible without any breaking changes.
  • The request body mappers were simplified so that any value passed to the value property would be serialized without any hard coded constraints.
  • surfaces now is a list of MeetingSurface type. Just like MeetingNotification it can be a union of any arbitrary type. Making it possible to have more than one type of surface object in the list of surfaces.
  • Name change: All the TeamsMeeting prefixed type names are not just Meeting. This conforms to the standards of meetings-related type within the SDK.

Testing

  • Performed end-to-end test
  • Unit tests remain the same

@singhk97 singhk97 requested a review from a team as a code owner January 24, 2023 00:42
@singhk97 singhk97 changed the title type structure changes, tested fix: type structure changes, tested Jan 24, 2023
@coveralls
Copy link

coveralls commented Jan 24, 2023

Pull Request Test Coverage Report for Build 4108444092

  • 10 of 10 (100.0%) changed or added relevant lines in 2 files are covered.
  • 5 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.001%) to 84.652%

Files with Coverage Reduction New Missed Lines %
libraries/botframework-streaming/src/webSocket/nodeWebSocket.ts 5 86.27%
Totals Coverage Status
Change from base Build 3841042555: 0.001%
Covered Lines: 19998
Relevant Lines: 22387

💛 - Coveralls

@singhk97 singhk97 changed the title fix: type structure changes, tested fix: teams meeting notification interface structure changes Jan 24, 2023
Copy link

@YunnyChung YunnyChung left a comment

Choose a reason for hiding this comment

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

First PR review: leaving a few comments :)

libraries/botframework-schema/src/teams/index.ts Outdated Show resolved Hide resolved
libraries/botbuilder/etc/botbuilder.api.md Outdated Show resolved Hide resolved
Copy link

@YunnyChung YunnyChung left a comment

Choose a reason for hiding this comment

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

Follow-up

Copy link

@YunnyChung YunnyChung left a comment

Choose a reason for hiding this comment

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

A follow up comment :)

Copy link

@YunnyChung YunnyChung 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! Thank you :)

@tracyboehrer tracyboehrer merged commit 802f82f into main Feb 10, 2023
@tracyboehrer tracyboehrer deleted the kavin/bug-fix-teams-targetted-meeting-notification branch February 10, 2023 20:11
tracyboehrer pushed a commit that referenced this pull request Feb 13, 2023
* type structure changes, tested

* yarn lint fix

* name change

* name change'

* name change

* update MeetingOnBehalfOf to just OnBehalfOf

* changes

* moved channelData to child interface
@tracyboehrer tracyboehrer changed the title fix: teams meeting notification interface structure changes fix: Teams Meeting Notification interface structure changes Feb 16, 2023
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.

targeted meeting notification interface changes
6 participants