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.

Sign up for GitHub

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

Jump to bottom

Added appId parameter to getContext #2337

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

Added appId parameter to getContext #2337

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
941ab45
Adding internal App ID param in getContext
vikramtha May 22, 2024
f093425
changefile
vikramtha May 22, 2024
9a39dfc
Merge branch 'main' into vikramtha/appIDgetContext
vikramtha May 22, 2024
d1f4c45
Updated intAppId to internalAppId and added link
vikramtha Jun 10, 2024
78af571
Merge branch 'main' into vikramtha/appIDgetContext
vikramtha Jun 10, 2024
4d07b51
Update packages/teams-js/src/public/app.ts
TrevorJoelHarris Jul 1, 2024
75b56c7
Merge branch 'main' into vikramtha/appIDgetContext
TrevorJoelHarris Jul 1, 2024
e4847fd
Updated to remove changes
vikramtha Jul 2, 2024
e9d8e5d
pt2
vikramtha Jul 2, 2024
008d988
This reverts commit e4847fd0b4bce3865f4e9fc13cf48ef7198c7dca.
vikramtha Jul 2, 2024
2fd2a53
update back to the inital
vikramtha Jul 2, 2024
27ac101
Updated entire PR to match the one in the hubSDK where now the host i…
vikramtha Jul 2, 2024
a07acae
Merge branch 'main' into vikramtha/appIDgetContext
vikramtha Jul 2, 2024
09d60af
Update app.spec.ts
vikramtha Jul 2, 2024
8079cc6
Make appID now a required param in the interface as there will always…
vikramtha Jul 10, 2024
5780071
Updated test cases
vikramtha Jul 10, 2024
2c74249
Updated test cases to properly work with requried param
vikramtha Jul 10, 2024
c96f356
another one
vikramtha Jul 10, 2024
86232cf
Merge branch 'main' into vikramtha/appIDgetContext
vikramtha Jul 17, 2024
67df88a
Merge branch 'main' into vikramtha/appIDgetContext
vikramtha Jul 24, 2024
ef4be07
Merge branch 'main' into vikramtha/appIDgetContext
TrevorJoelHarris Aug 13, 2024
17bcf30
Update change/@microsoft-teams-js-07a56fbc-6d3e-474d-b3d9-6c6fe3a3be5…
TrevorJoelHarris Aug 13, 2024
a4b6d3c
Merge branch 'main' into vikramtha/appIDgetContext
TrevorJoelHarris Aug 13, 2024
6a26ec4
Update change/@microsoft-teams-js-07a56fbc-6d3e-474d-b3d9-6c6fe3a3be5…
TrevorJoelHarris Aug 15, 2024
c142a94
Merge branch 'main' into vikramtha/appIDgetContext
jadahiya-MSFT Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@microsoft-teams-js-07a56fbc-6d3e-474d-b3d9-6c6fe3a3be50.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Added a new parameter to `app.getContext` called `appId` which contains the current application's AppId. This value is used in deeplinks and by hosts to identify apps",
TrevorJoelHarris marked this conversation as resolved.
Show resolved Hide resolved
"packageName": "@microsoft/teams-js",
"email": "[email protected]",
"dependentChangeType": "patch"
}
6 changes: 6 additions & 0 deletions packages/teams-js/src/public/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,11 @@ export namespace app {
* ID for the current visible app which is different for across cached sessions. Used for correlating telemetry data.
*/
appLaunchId?: string;

/**
* App id that is used by Hosts to distinguish between different apps sideloaded or in store
*/
appId: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

@maglims brought up a good question that I forgot: since not all hosts will be setting this value, shouldn't it be optional (like appLaunchId)?

Copy link
Contributor Author

@vikramtha vikramtha Aug 16, 2024 •
edited by TrevorJoelHarris
Loading

Choose a reason for hiding this comment

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

This will always be populated as long as there is an app def sent back; I rewrote the code in host SDK so that hosts no longer need to pass the value and instead it is taken from app definition

Copy link
Contributor

Choose a reason for hiding this comment

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

What about hosts that aren't yet using the updated host-sdk? I believe in that case this would not be set, right? That's generally why we mark things like this optional, to handle backwards compatibility

Copy link
Contributor

Choose a reason for hiding this comment

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

string

Can you use the AppId type for this? Strong types are better than strings. It should be easy to create this object as an AppId in transformLegacyContextToAppContext after the data comes in over the wire as a string.

}

/**
Expand Down Expand Up @@ -1007,6 +1012,7 @@ function transformLegacyContextToAppContext(legacyContext: LegacyContext): app.C
ringId: legacyContext.ringId,
},
appLaunchId: legacyContext.appLaunchId,
appId: legacyContext.appId,
},
page: {
id: legacyContext.entityId,
Expand Down
8 changes: 8 additions & 0 deletions packages/teams-js/src/public/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,14 @@ export interface Context {
* They help pre-fill the dialog with necessary information (`dialogParameters`) along with other details.
*/
dialogParameters?: Record<string, string>;

/**
* @deprecated
* As of 2.0.0, please use {@link app.AppInfo.appId | app.Context.app.appId} instead
*
* App id that is used by Hosts to distinguish between different apps sideloaded or in store
*/
appId: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a new feature, can I know why we need to add this parameter to a deprecated interface and as a deprecated element?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the Context deprecated interface is still used in the legacyContext transformation so I kept it

Copy link
Contributor

Choose a reason for hiding this comment

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

This one should probably also be optional

}

/** Represents the parameters used to share a deep link. */
Expand Down
7 changes: 7 additions & 0 deletions packages/teams-js/test/private/privateAPIs.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ describe('AppSDK-privateAPIs', () => {
channelId: 'someChannelId1',
entityId: 'someEntityId1',
userObjectId: 'someUserObjectId1',
appId: 'someAppId',
};
const expectedContext1: app.Context = {
app: {
Expand All @@ -248,6 +249,7 @@ describe('AppSDK-privateAPIs', () => {
clientType: HostClientType.web,
sessionId: '',
},
appId: 'someAppId',
},
page: {
id: 'someEntityId1',
Expand All @@ -267,6 +269,7 @@ describe('AppSDK-privateAPIs', () => {
channelId: 'someChannelId2',
entityId: 'someEntityId2',
userObjectId: 'someUserObjectId2',
appId: 'someAppId',
};
const expectedContext2: app.Context = {
app: {
Expand All @@ -278,6 +281,7 @@ describe('AppSDK-privateAPIs', () => {
clientType: HostClientType.web,
sessionId: '',
},
appId: 'someAppId',
},
page: {
id: 'someEntityId2',
Expand All @@ -297,6 +301,7 @@ describe('AppSDK-privateAPIs', () => {
channelId: 'someChannelId3',
entityId: 'someEntityId3',
userObjectId: 'someUserObjectId3',
appId: 'someAppId',
};
const expectedContext3: app.Context = {
app: {
Expand All @@ -308,6 +313,7 @@ describe('AppSDK-privateAPIs', () => {
clientType: HostClientType.web,
sessionId: '',
},
appId: 'someAppId',
},
page: {
id: 'someEntityId3',
Expand Down Expand Up @@ -360,6 +366,7 @@ describe('AppSDK-privateAPIs', () => {
userClickTime: 1000,
teamTemplateId: 'com.microsoft.teams.ManageAProject',
userFileOpenPreference: FileOpenPreference.Web,
appId: 'someappId',
};

// Get many responses to the same message
Expand Down
4 changes: 4 additions & 0 deletions packages/teams-js/test/public/app.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ describe('Testing app capability', () => {
appLaunchId: 'appLaunchId',
userDisplayName: 'someTestUser',
teamSiteId: 'someSiteId',
appId: 'someAppId',
};

const expectedContext: app.Context = {
Expand All @@ -576,6 +577,7 @@ describe('Testing app capability', () => {
userClickTime: 2222,
userFileOpenPreference: FileOpenPreference.Inline,
appLaunchId: 'appLaunchId',
appId: 'someAppId',
host: {
name: HostName.orange,
clientType: HostClientType.web,
Expand Down Expand Up @@ -1430,6 +1432,7 @@ describe('Testing app capability', () => {
appLaunchId: 'appLaunchId',
userDisplayName: 'someTestUser',
teamSiteId: 'someSiteId',
appId: 'someAppId',
};

const expectedContext: app.Context = {
Expand All @@ -1449,6 +1452,7 @@ describe('Testing app capability', () => {
ringId: 'someRingId',
},
appLaunchId: 'appLaunchId',
appId: 'someAppId',
},
page: {
id: 'someEntityId',
Expand Down
1 change: 1 addition & 0 deletions packages/teams-js/test/public/publicAPIs.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ describe('MicrosoftTeams-publicAPIs', () => {
appSessionId: 'appSessionId',
appLaunchId: 'appLaunchId',
meetingId: 'dummyMeetingId',
appId: 'dummyAppId',
};
getContext((context) => {
Object.keys(expectedContext).forEach((e) => {
Expand Down