Skip to content

Support sending no result from taskModule.submit#636

Closed
th1nkful wants to merge 1 commit intomicrosoft:mainfrom
th1nkful:support-taskmodule-submit-no-response
Closed

Support sending no result from taskModule.submit#636
th1nkful wants to merge 1 commit intomicrosoft:mainfrom
th1nkful:support-taskmodule-submit-no-response

Conversation

@th1nkful
Copy link
Copy Markdown

@th1nkful th1nkful commented Sep 27, 2023

Linked issues

closes: #635

Details

Provide a list of your changes here. If you are fixing a bug, please provide steps to reproduce the bug.

  • Return null or undefined from taskModule.submit without having done anything strictly with context and it will show an error "Unable to reach app" even though the intention was to close the app

Change details

If developer returns null or undefined from taskModule.submit then no response is given, allowing a task module to close without showing an error window

Additional information

A PR to go with my issue

Here is a screenshot of the error with the current code:

CleanShot 2023-09-27 at 22 12 17@2x

My code:

bot.taskModules.submit(
  'submit',
  async (context, state, data) => {
    await handleTeamsTaskModuleSubmit(context, data);
    // context.turnState.set(INVOKE_RESPONSE_KEY, { status: 200 });
    return null;
  },
);

If I uncomment the context.turnState.set line then I am able to close the task module without the error, preserving previous behaviour

@corinagum
Copy link
Copy Markdown
Collaborator

Hey @th1nkful thanks for the submission! Would you mind sharing a bit more of your bot code? Are you extending handleTeamsTaskModuleSubmit from botbuilder?

I've tagged aacebo to also take a look to get his experienced feedback :)

@th1nkful
Copy link
Copy Markdown
Author

th1nkful commented Sep 28, 2023

Hey @th1nkful thanks for the submission! Would you mind sharing a bit more of your bot code? Are you extending handleTeamsTaskModuleSubmit from botbuilder?

Hi @corinagum , here is the definition for handleTeamsTaskModuleSubmit

const handleTeamsTaskModuleSubmit = async (
  context: TurnContext,
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  data: Record<string, any>,
): Promise<void | TaskModuleResponse> => {
  const tenant = await findOrCreateTenant(
    context.activity.conversation.tenantId,
    context.activity.serviceUrl,
  );

  await findOrCreateInstall(tenant, context);

  if (data.cancelAction) {
    return undefined;
  }

  if (data.contributions) {
    telemetry.track(telemetry.Event.SubmitViewContributionsTaskModule, { context });
    return undefined;
  }

  if (data.onboarding && data.mission) {
    telemetry.track(telemetry.Event.SubmitOnboardTaskModule, { context });
    return handleOnboard(context, { data });
  }

  if (data.contribute && data.mission) {
    telemetry.track(telemetry.Event.SubmitAddContributionTaskModule, { context });
    return handleContribution(context, { data });
  }

  if (data.cancelled && data.mission) {
    telemetry.track(telemetry.Event.SubmitCancelMissionTaskModule, { context });
    return handleCancelled(context, { data });
  }

  if (data.completed && data.mission) {
    telemetry.track(telemetry.Event.SubmitCompleteMissionTaskModule, { context });
    return handleCompleted(context, { data });
  }

  if (data.created && data.mission) {
    telemetry.track(telemetry.Event.SubmitCreateMissionTaskModule, { context });
    return handleCreated(context, { data });
  }

  if (data.edited && data.mission) {
    telemetry.track(telemetry.Event.SubmitEditMissionTaskModule, { context });
    return handleEdited(context, { data });
  }

  if (data.joined && data.mission) {
    telemetry.track(telemetry.Event.SubmitJoinMissionTaskModule, { context });
    return handleJoined(context, { data });
  }

  log.info('Unhandled task module submission');
  log.info(JSON.stringify({ activity: context.activity, data }, null, 2));

  return undefined;
};

All of the handleX function are doing this but different variations depending on the action

const handleOnboard = async (
  context: TurnContext,
  taskModuleRequest: TaskModuleRequest,
) => {
  try {
    const { mission: slug, themes } = taskModuleRequest.data;

    const install = await findInstall(context);
    await updateThemes(install, themes as number[]);
    await updateWelcomeCard(context, install);

    const mission = await getMissionBySlug(slug, install.id);
    await initialPostMissionCard(mission, context);
    return undefined;
  } catch (error) {
    console.log(error);
    log.warn(error);
    log.warn('Error handling onboard post-submit');
    return undefined;
  }
};

Previously this was used with the BotFrameworkAdapter from botbuilder which was deprecated. Updated to use CloudAdapter instead as it seemed to be required to make use of the AI features. This is the old BotFrameworkAdapter implementation

import {
  AdaptiveCardInvokeValue, TaskModuleRequest, TeamsActivityHandler, TurnContext,
} from 'botbuilder';

import onMessage from './handlers/onMessage';
import onMembersAdded from './handlers/onMembersAdded';
import onMembersRemoved from './handlers/onMembersRemoved';
import onAdaptiveCardInvoke from './handlers/onAdaptiveCardInvoke';
import handleTeamsTaskModuleFetch from './handlers/handleTaskModuleFetch';
import handleTeamsTaskModuleSubmit from './handlers/handleTaskModuleSubmit';
import onConversationUpdate from './handlers/onConversationUpdate';

class TeamsBot extends TeamsActivityHandler {
  constructor() {
    super();

    this.onConversationUpdate(onConversationUpdate);
    this.onMembersAdded(onMembersAdded);
    this.onMembersRemoved(onMembersRemoved);
    this.onMessage(onMessage);
  }

  // eslint-disable-next-line class-methods-use-this
  async onAdaptiveCardInvoke(context: TurnContext, invokeValue: AdaptiveCardInvokeValue) {
    return onAdaptiveCardInvoke(context, invokeValue);
  }

  // eslint-disable-next-line class-methods-use-this
  async handleTeamsTaskModuleFetch(context: TurnContext, taskModuleRequest: TaskModuleRequest) {
    return handleTeamsTaskModuleFetch(context, taskModuleRequest);
  }

  // the return type is not correct in the botbuilder library
  // you CAN return nothing at all if you want to not show a message
  // or a new task module, returning TaskModuleResponse is purely optional
  // See docs: https://learn.microsoft.com/en-us/microsoftteams/platform/task-modules-and-cards/task-modules/task-modules-bots?tabs=nodejs#responds-to-the-tasksubmit-messages
  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore
  // eslint-disable-next-line class-methods-use-this
  async handleTeamsTaskModuleSubmit(context: TurnContext, taskModuleRequest: TaskModuleRequest) {
    return handleTeamsTaskModuleSubmit(context, taskModuleRequest);
  }
}

export default TeamsBot;

Let me know if you need more detail

@corinagum
Copy link
Copy Markdown
Collaborator

@th1nkful Thank you! I spoke with the team, and we want to create a sample to test/showcase the behavior before merge, especially since we don't have any Task Module samples yet :O

Would you be willing to give it a try, following our general format under /samples? For now, I'm thinking we should make a new folder called 06.taskModules.a.___. If not, no problem! I'll work on getting a sample ready - just note it might take me a little longer due to time constraints. :)

Another thing that would be super appreciated is setting up a unit test, if you have the bandwidth.

Please let us know! Thank you~

@corinagum
Copy link
Copy Markdown
Collaborator

Are you extending handleTeamsTaskModuleSubmit from botbuilder?

Hi @th1nkful, I meant to clarify -- using this code, you are not creating an activity handler, correct? The code you are pulling from is from TeamsActivityHandler, but there is not a TeamsActivityHandler class in this repo.

@th1nkful
Copy link
Copy Markdown
Author

th1nkful commented Oct 3, 2023

Are you extending handleTeamsTaskModuleSubmit from botbuilder?

Hi @th1nkful, I meant to clarify -- using this code, you are not creating an activity handler, correct? The code you are pulling from is from TeamsActivityHandler, but there is not a TeamsActivityHandler class in this repo.

@corinagum the code above with TeamsActivityHandler is the old implementation when using BotBuilderFranework from the botbuilder package before I updated to the Teams AI SDK (with the CloudAdapter) when you asked for details about my usage of handleTeamsTaskModuleSubmit I thought more context about where I ran into this and what I was doing before it became an issue would be helpful to understand what was the reasoning behind the suggested change

const result = await handler(context, state, context.activity.value?.data ?? {});

// if result is null or undefined, this indicates no response should be sent
if (!result) return;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is incorrect handling. Because this is an invoke, it is expected that a response is sent back, even if there is no body to the response. Like your example code showed, instead of return, it should send the following:

 await context.sendActivity({
          value: { status: 200 } as InvokeResponse,
          type: ActivityTypes.InvokeResponse
         });

@corinagum
Copy link
Copy Markdown
Collaborator

Hi @th1nkful, thank you very much for originally reporting this bug and putting up a PR! Since there was a required modification and I was working on other Task Module changes, I went ahead and implemented the requested change in this PR. Therefore I will close this pull request and I have mentioned that original changes started from you in that PR. Thanks again!

@corinagum corinagum closed this Nov 3, 2023
corinagum added a commit that referenced this pull request Nov 6, 2023
…h and tests (#772)

## Linked issues

closes: #635
closes: #247 

## Details

- A behavior that was missing from Task Modules was handling submit
"continue" scenario when the handler does not return a
`TaskModuleResponse` or string. This ensures that the expected response
(empty 200) is sent even if there is nothing new to render/send.
- Modify so that Teams events check for channelId using `Channels` enum,
not 'msteams' string
- This included minor refactor for Message Extension and Application as
well
- Add Task Module tests to get the file to 80% coverage

## Attestation Checklist

- [x] My code follows the style guidelines of this project

- I have checked for/fixed spelling, linting, and other errors
- I have commented my code for clarity
- I have made corresponding changes to the documentation (we use
[TypeDoc](https://typedoc.org/) to document our code)
- My changes generate no new warnings
- I have added tests that validates my changes, and provides sufficient
test coverage. I have tested with:
  - Local testing
  - E2E testing in Teams
- New and existing unit tests pass locally with my changes

### Additional information

Thank you @th1nkful for the original
[PR](#636), which I modified
to match Bot Framework protocol :)

---------

Co-authored-by: Corina Gum <>
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.

[Feature Request]: Add support for ignore TaskModule submit invocations

2 participants