Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
72 changes: 72 additions & 0 deletions packages/api/src/clients/conversation/activity.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,76 @@ describe('ConversationActivityClient', () => {
await client.getMembers('1', '2');
expect(spy).toHaveBeenCalledWith('/v3/conversations/1/activities/2/members');
});

describe('targeted messages', () => {
it('should create with query parameter when isTargeted is true', async () => {
const client = new ConversationActivityClient('');
const spy = jest.spyOn(client.http, 'post').mockResolvedValueOnce({});

await client.create('1', {
type: 'message',
text: 'hi',
}, true);

expect(spy).toHaveBeenCalledWith('/v3/conversations/1/activities?isTargetedActivity=true', {
type: 'message',
text: 'hi',
});
});

it('should update with query parameter when isTargeted is true', async () => {
const client = new ConversationActivityClient('');
const spy = jest.spyOn(client.http, 'put').mockResolvedValueOnce({});

await client.update('1', '2', {
type: 'message',
text: 'hi',
}, true);

expect(spy).toHaveBeenCalledWith('/v3/conversations/1/activities/2?isTargetedActivity=true', {
type: 'message',
text: 'hi',
});
});

it('should reply with query parameter when isTargeted is true', async () => {
const client = new ConversationActivityClient('');
const spy = jest.spyOn(client.http, 'post').mockResolvedValueOnce({});

await client.reply('1', '2', {
type: 'message',
text: 'hi',
}, true);

expect(spy).toHaveBeenCalledWith('/v3/conversations/1/activities/2?isTargetedActivity=true', {
type: 'message',
text: 'hi',
replyToId: '2',
});
});

it('should delete with query parameter when isTargeted is true', async () => {
const client = new ConversationActivityClient('');
const spy = jest.spyOn(client.http, 'delete').mockResolvedValueOnce({});

await client.delete('1', '2', true);

expect(spy).toHaveBeenCalledWith('/v3/conversations/1/activities/2?isTargetedActivity=true');
});

it('should not add query parameter when isTargeted is false', async () => {
const client = new ConversationActivityClient('');
const spy = jest.spyOn(client.http, 'post').mockResolvedValueOnce({});

await client.create('1', {
type: 'message',
text: 'hi',
}, false);

expect(spy).toHaveBeenCalledWith('/v3/conversations/1/activities', {
type: 'message',
text: 'hi',
});
});
});
});
47 changes: 28 additions & 19 deletions packages/api/src/clients/conversation/activity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,35 +32,44 @@ export class ConversationActivityClient {
this._apiClientSettings = mergeApiClientSettings(apiClientSettings);
}

async create(conversationId: string, params: ActivityParams) {
const res = await this.http.post<Resource>(
`${this.serviceUrl}/v3/conversations/${conversationId}/activities`,
params
);
async create(conversationId: string, params: ActivityParams, isTargeted: boolean = false) {
let url = `${this.serviceUrl}/v3/conversations/${conversationId}/activities`;
if (isTargeted) {
url += '?isTargetedActivity=true';
}

const res = await this.http.post<Resource>(url, params);
Comment on lines +36 to +41
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

isTargetedActivity should be a URL param. http.post should have support for that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do the same for all the methods below as well.

return res.data;
}

async update(conversationId: string, id: string, params: ActivityParams) {
const res = await this.http.put<Resource>(
`${this.serviceUrl}/v3/conversations/${conversationId}/activities/${id}`,
params
);
async update(conversationId: string, id: string, params: ActivityParams, isTargeted: boolean = false) {
let url = `${this.serviceUrl}/v3/conversations/${conversationId}/activities/${id}`;
if (isTargeted) {
url += '?isTargetedActivity=true';
}

const res = await this.http.put<Resource>(url, params);
return res.data;
}

async reply(conversationId: string, id: string, params: ActivityParams) {
async reply(conversationId: string, id: string, params: ActivityParams, isTargeted: boolean = false) {
params.replyToId = id;
const res = await this.http.post<Resource>(
`${this.serviceUrl}/v3/conversations/${conversationId}/activities/${id}`,
params
);
let url = `${this.serviceUrl}/v3/conversations/${conversationId}/activities/${id}`;
if (isTargeted) {
url += '?isTargetedActivity=true';
}

const res = await this.http.post<Resource>(url, params);
return res.data;
}

async delete(conversationId: string, id: string) {
const res = await this.http.delete<void>(
`${this.serviceUrl}/v3/conversations/${conversationId}/activities/${id}`
);
async delete(conversationId: string, id: string, isTargeted: boolean = false) {
let url = `${this.serviceUrl}/v3/conversations/${conversationId}/activities/${id}`;
if (isTargeted) {
url += '?isTargetedActivity=true';
}

const res = await this.http.delete<void>(url);
return res.data;
}

Expand Down
14 changes: 8 additions & 6 deletions packages/api/src/clients/conversation/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ export class ConversationClient {

activities(conversationId: string) {
return {
create: (params: ActivityParams) => this._activities.create(conversationId, params),
update: (id: string, params: ActivityParams) =>
this._activities.update(conversationId, id, params),
reply: (id: string, params: ActivityParams) =>
this._activities.reply(conversationId, id, params),
delete: (id: string) => this._activities.delete(conversationId, id),
create: (params: ActivityParams, isTargeted?: boolean) =>
this._activities.create(conversationId, params, isTargeted),
update: (id: string, params: ActivityParams, isTargeted?: boolean) =>
this._activities.update(conversationId, id, params, isTargeted),
reply: (id: string, params: ActivityParams, isTargeted?: boolean) =>
this._activities.reply(conversationId, id, params, isTargeted),
delete: (id: string, isTargeted?: boolean) =>
this._activities.delete(conversationId, id, isTargeted),
members: (activityId: string) => this._activities.getMembers(conversationId, activityId),
};
}
Expand Down
74 changes: 74 additions & 0 deletions packages/apps/src/app.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,78 @@ describe('App', () => {
).rejects.toThrow('app not started');
});
});

describe('targeted messages', () => {
let app: TestApp;

beforeEach(() => {
app = new TestApp({
clientId: 'test-client-id',
clientSecret: 'test-client-secret',
tenantId: 'test-tenant-id',
plugins: [new TestHttpPlugin()],
});
});

it('should set ref.user when isTargeted is true', async () => {
await app.start();

const mockSend = jest.fn().mockResolvedValue({ id: 'activity-id' });
jest.spyOn(app.http, 'send').mockImplementation(mockSend);

await app.send('conversation-id', {
type: 'message',
text: 'Hello',
recipient: { id: 'user-123', name: 'Test User', role: 'user' },
}, true);

expect(mockSend).toHaveBeenCalled();
const [, ref, isTargeted] = mockSend.mock.calls[0];
expect(ref.user).toEqual({ id: 'user-123', name: 'Test User', role: 'user' });
expect(isTargeted).toBe(true);
});

it('should not set ref.user when isTargeted is false', async () => {
await app.start();

const mockSend = jest.fn().mockResolvedValue({ id: 'activity-id' });
jest.spyOn(app.http, 'send').mockImplementation(mockSend);

await app.send('conversation-id', {
type: 'message',
text: 'Hello',
recipient: { id: 'user-123', name: 'Test User', role: 'user' },
}, false);

expect(mockSend).toHaveBeenCalled();
const [, ref, isTargeted] = mockSend.mock.calls[0];
expect(ref.user).toBeUndefined();
expect(isTargeted).toBe(false);
});

it('should throw error when isTargeted is true but no recipient', async () => {
await app.start();

await expect(
app.send('conversation-id', {
type: 'message',
text: 'Hello',
}, true)
).rejects.toThrow('activity.recipient is required for targeted messages');
});

it('should allow isTargeted false without recipient', async () => {
await app.start();

const mockSend = jest.fn().mockResolvedValue({ id: 'activity-id' });
jest.spyOn(app.http, 'send').mockImplementation(mockSend);

await app.send('conversation-id', {
type: 'message',
text: 'Hello',
}, false);

expect(mockSend).toHaveBeenCalled();
});
});
});
13 changes: 11 additions & 2 deletions packages/apps/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,12 +399,20 @@ export class App<TPlugin extends IPlugin = IPlugin> {
* send an activity proactively
* @param conversationId the conversation to send to
* @param activity the activity to send
* @param isTargeted when true, sends the message privately to the recipient specified in activity.recipient
*/
async send(conversationId: string, activity: ActivityLike) {
async send(conversationId: string, activity: ActivityLike, isTargeted: boolean = false) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can we do targetedRecipientUserId here?
So users can do app.send(conversationId, "string message", targetedRecipientUserId); Right now it's not clear that isTargeted will lead to the method throwing under some circumstances.

With targetedRecipientUserId, toActivityParams does the work of setting the recipient id in the activity.

if (!this.id) {
throw new Error('app not started');
}

const activityParams = toActivityParams(activity);

// Validate that recipient is provided for targeted messages
if (isTargeted && !activityParams.recipient) {
throw new Error('activity.recipient is required for targeted messages');
}

const ref: ConversationReference = {
channelId: 'msteams',
serviceUrl: this.api.serviceUrl,
Expand All @@ -417,9 +425,10 @@ export class App<TPlugin extends IPlugin = IPlugin> {
id: conversationId,
conversationType: 'personal',
},
user: isTargeted ? activityParams.recipient : undefined,
};

const res = await this.http.send(toActivityParams(activity), ref);
const res = await this.http.send(activityParams, ref, isTargeted);
return res;
}

Expand Down
21 changes: 14 additions & 7 deletions packages/apps/src/contexts/activity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ describe('ActivityContext', () => {
</blockquote>\r\nWhat is up?`,
type: 'message',
}),
mockRef
mockRef,
undefined
);
});

Expand All @@ -152,7 +153,8 @@ describe('ActivityContext', () => {
</blockquote>\r\nWhat is up?`,
type: 'message',
}),
mockRef
mockRef,
undefined
);
});

Expand All @@ -169,7 +171,8 @@ describe('ActivityContext', () => {
text: 'What is up?',
type: 'message',
}),
mockRef
mockRef,
undefined
);
});

Expand All @@ -186,7 +189,8 @@ describe('ActivityContext', () => {
text: '',
type: 'message',
}),
mockRef
mockRef,
undefined
);
});
});
Expand All @@ -203,7 +207,8 @@ describe('ActivityContext', () => {
text: 'What is up?',
type: 'message',
}),
mockRef
mockRef,
undefined
);
});
});
Expand Down Expand Up @@ -277,7 +282,8 @@ describe('ActivityContext', () => {
inputHint: 'acceptingInput',
recipient: { id: 'test-user', name: 'Test User', role: 'user' },
}),
mockRef
mockRef,
undefined
);
});

Expand Down Expand Up @@ -314,7 +320,8 @@ describe('ActivityContext', () => {
expect(mockApiClient.conversations.create).toHaveBeenCalled();
expect(mockSender.send).toHaveBeenCalledWith(
expect.objectContaining({ type: 'message', text: 'Please Sign In...' }),
expect.any(Object)
expect.any(Object),
undefined
);
});

Expand Down
8 changes: 4 additions & 4 deletions packages/apps/src/contexts/activity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,11 @@ export class ActivityContext<T extends Activity = Activity, TExtraCtx extends {}
}
}

async send(activity: ActivityLike, conversationRef?: ConversationReference) {
return await this._plugin.send(toActivityParams(activity), conversationRef ?? this.ref);
async send(activity: ActivityLike, conversationRef?: ConversationReference, isTargeted?: boolean) {
return await this._plugin.send(toActivityParams(activity), conversationRef ?? this.ref, isTargeted);
}

async reply(activity: ActivityLike) {
async reply(activity: ActivityLike, isTargeted?: boolean) {
activity = toActivityParams(activity);
activity.replyToId = this.activity.id;

Expand All @@ -228,7 +228,7 @@ export class ActivityContext<T extends Activity = Activity, TExtraCtx extends {}
}
}

return this.send(activity);
return this.send(activity, this.ref, isTargeted);
}

async signin(options?: Partial<SignInOptions>) {
Expand Down
8 changes: 5 additions & 3 deletions packages/apps/src/plugins/http/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class HttpPlugin implements ISender {
this._server.close();
}

async send(activity: ActivityParams, ref: ConversationReference) {
async send(activity: ActivityParams, ref: ConversationReference, isTargeted: boolean = false) {
const api = new Client(
ref.serviceUrl,
this.client.clone({
Expand All @@ -160,16 +160,18 @@ export class HttpPlugin implements ISender {
...activity,
from: ref.bot,
conversation: ref.conversation,
// Set recipient from ref.user if provided (for targeted messages)
...(ref.user && { recipient: ref.user }),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Putting this logic in toActivityParams might be better/cleaner

};

if (activity.id) {
const res = await api.conversations
.activities(ref.conversation.id)
.update(activity.id, activity);
.update(activity.id, activity, isTargeted);
return { ...activity, ...res };
}

const res = await api.conversations.activities(ref.conversation.id).create(activity);
const res = await api.conversations.activities(ref.conversation.id).create(activity, isTargeted);
return { ...activity, ...res };
}

Expand Down
Loading