Skip to content
This repository has been archived by the owner on Jun 30, 2022. It is now read-only.

QnAMakerDialog is not created for different languages #3496

Closed
tommyJimmy87 opened this issue Jun 29, 2020 · 22 comments · Fixed by #3752
Closed

QnAMakerDialog is not created for different languages #3496

tommyJimmy87 opened this issue Jun 29, 2020 · 22 comments · Fixed by #3752
Assignees
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. in-progress The issue is assigned and it is being worked on. Team: Kobuk This issue is assigned to the Kobuk team.

Comments

@tommyJimmy87
Copy link

What project is affected?

Virtual Assistant and Skills

What language is this in?

Typescript

What happens?

When a QnAMakerDialog is created with a specific language (ex. DE) will be resolving always that language even though you change the language or another user uses a different language (ex EN).

What are the steps to reproduce this issue?

User 1: Sends utterance to the bot ( DE );
Bot: The Bot resolves the utterance as QnA intent, and create the new QnADialog, with the knowledgebaseId as Id of the Dialog:

private registerQnADialog(knowledgebaseId: string, cognitiveModels: ICognitiveModelSet, locale: string): void {
     const qnaEndpoint: QnAMakerEndpoint | undefined = cognitiveModels.qnaConfiguration.get(knowledgebaseId);
     if (qnaEndpoint == undefined){
         throw new Error(`Could not find QnA Maker knowledge base configuration with id: ${ knowledgebaseId }.`);
     }

     if (this.dialogs.find(knowledgebaseId) == undefined) {
         const qnaDialog: QnAMakerDialog = new QnAMakerDialog(
             qnaEndpoint.knowledgeBaseId,
             qnaEndpoint.endpointKey,
             // The following line is a workaround until the method getQnAClient of QnAMakerDialog is fixed
             // as per issue https://github.com/microsoft/botbuilder-js/issues/1885
             new URL(qnaEndpoint.host).hostname.split('.')[0],
             this.templateEngine.generateActivityForLocale('UnsupportedMessage', locale) as Activity,
             // Before, instead of 'undefined' a '0.3' value was used in the following line
             undefined,
             this.templateEngine.generateActivityForLocale('QnaMakerAdaptiveLearningCardTitle', locale).text,
             this.templateEngine.generateActivityForLocale('QnaMakerNoMatchText', locale).text
         );

         qnaDialog.id = knowledgebaseId;

         this.addDialog(qnaDialog);
     }
 }

Bot: Sends QnA Answer to User 1 in User 1's locale ( DE )
User 2: Sends utterance to bot ( EN );
Bot: The Bot resolves the utterance as QnA intent ( same the user 1 sent ), no new QnaMakerDialog is created because one Dialog with the same knowledgebaseId already exists.
Bot: The Bot start again the QnA Dialog (added from user 1 iteration) and get the answer from the Knowledge Base (DE) of the first user;
Bot: Sends Message to User 2 in User 1's locale.

What were you expecting to happen?

Another QnAMakerDialog will be created with the right language and right knowledge base ID.

@tommyJimmy87 tommyJimmy87 added Needs Triage Needs to be triaged for assignment Type: Bug Something isn't working labels Jun 29, 2020
@Batta32 Batta32 self-assigned this Jun 29, 2020
@Batta32
Copy link
Collaborator

Batta32 commented Jun 29, 2020

Thanks @tommyJimmy87. As soon as we have any update related to the Dependency Injection's implementation, we will back to you!

@Batta32
Copy link
Collaborator

Batta32 commented Jul 15, 2020

Hi @tommyJimmy87, we created the PR #3559 which solves this problem incorporating Dependency Injection in the bots.

If you are so kind, can you validate the changes following these repro steps and using this branch:

  1. Go to bot-solutions library
  2. Execute npm install to install the dependencies
  3. Execute npm run build to compile the solution
  4. Execute npm pack to create the tgz
  5. Go to the Virtual Assistant Sample
  6. Use the created tgz in the Virtual Assistant package.json as follows:

"bot-solutions": "PATH TO BOT-SOLUTIONS TGZ"

  1. Deploy the Virtual Assistant in multi languages
  2. Follow the steps to reproduce the QnAMakerDialog scenario you mentioned

Last but not least, as it was mentioned, the PR contains the following changes:

  1. Implementation of Dependency Injection (inversifyjs library) in Virtual Assistant and Skill of TypeScript
  2. Update templates

We will be attentive to your answer 😊.

@lauren-mills lauren-mills added Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. and removed Needs Triage Needs to be triaged for assignment labels Jul 16, 2020
@tommyJimmy87
Copy link
Author

Hi @Batta32,

thanks for your support, looks like the issue has been solved with the new bot-solutions library that I integrated from your branch :) Do you have a rough estimation to when you're going to release it?

@tommyJimmy87
Copy link
Author

HI @Batta32, I was noticing that when I try to run the unit tests I get an error : TypeError: Reflect.hasOwnMetadata is not a function.
Is this because you didn't migrate the tests to use inversify?

@Batta32
Copy link
Collaborator

Batta32 commented Aug 5, 2020

Thanks @tommyJimmy87! We will be reviewing this and we will back to you as soon as we have any update of this 😊.

@tommyJimmy87
Copy link
Author

Hi @Batta32, we solved this problem adding this import to the test base js file : require('reflect-metadata');

@DiegoCardozo94
Copy link
Contributor

Hi @tommyJimmy87 - we couldn't reproduce the issue that you mentioned related to the unit tests.

We came up with questions:

  1. Are you using our branch to validate those changes?
  2. Which component do you have the issue?
  3. Can you share the repro steps that you followed?

This is our environment:

  • We are using this branch: feature/southworks/dependency-injection-implementation
  • TypeScript Virtual Assistant Sample
  • TypeScript Skill Sample

We took the following steps to reproduce the issue for the Virtual Assistant and the Skill:

  1. Go to bot-solutions library
  2. Execute npm install to install the dependencies
  3. Execute npm run build to compile the solution
  4. Execute npm pack to create the .tgz
  5. Go to the Virtual Assistant Sample
  6. Use the created .tgz in the Virtual Assistant package.json as follows:
  7. "bot-solutions": "<PATH_TO_BOT_SOLUTIONS.TGZ>"
  8. Execute npm install to install the dependencies
  9. Execute npm run build to compile the solution
  10. Execute npm run test to compile the solution
  11. Verify the test execution

image

image

@DiegoCardozo94
Copy link
Contributor

Hi @tommyJimmy87 - we added the suggested changes in the PR #3559.

We updated the Virtual Assistant & Skill bots adding the following import tests files: require('reflect-metadata'); specifically in the BotTestBase and SkillTestBase, in order to avoid the issue that you mentioned related to the unit tests.

You can follow up these steps to test the changes in the Virtual Assistant and Skill.

Thanks,
Diego.

@peterinnesmsft peterinnesmsft added bug Indicates an unexpected problem or an unintended behavior. in-progress The issue is assigned and it is being worked on. Team: Kobuk This issue is assigned to the Kobuk team. and removed Type: Bug Something isn't working labels Aug 20, 2020
@tommyJimmy87
Copy link
Author

tommyJimmy87 commented Sep 11, 2020

Hi @Batta32 and @DiegoCardozo94,

I have one a generic question but since I'm using this Branch in our project already even though has not been released I would ask it here.

In our scenario, we have MS Teams as the only channel, I see that the TeamsActivityHandler is used only in the VA and not in the skill's sample, is that right? Because maybe wrongly I was extending also in the skill the TeamsActivityHandler and that drove me to one issue specifically: basically the endOfConversation method is never called after a cancel in the skill's DefaultActivityHandler. But maybe I should just switch to the "normal" ActivityHandler extension within the skills. Could you help me or rather give me feedback on this?

Thanks

@Batta32
Copy link
Collaborator

Batta32 commented Sep 11, 2020

Hi @tommyJimmy87, the TeamsActivityHandler is derived from ActivityHandler which adds support for the Microsoft Teams specific events and interactions.

The Virtual Assistant is the one which interacts to the Microsoft Teams Channel in the schema of Virtual Assistant connected to Skills, so the DefaultActivityHandler of the Virtual Assistant needs to extend from the TeamsActivityHandler, not the Skill in that mentioned schema, as you can see in the samples.

The DefaultActivityHandler of the Virtual Assistant in the templates/samples implements the TeamsActivityHandler which enables Teams scenarios out of the box.

We researched the following documentation that you can also review:

Let us know if this helps to you 😊.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The issue hasn't been updated in a long time and will be automatically closed. label Oct 12, 2020
@tommyJimmy87
Copy link
Author

@Batta32 this is not yet released right?

@Batta32
Copy link
Collaborator

Batta32 commented Oct 13, 2020

Hi @tommyJimmy87 - we are finalizing a plan to release new packages. As soon as we have any update we will back to you here 😊.

@peterinnesmsft peterinnesmsft removed the stale The issue hasn't been updated in a long time and will be automatically closed. label Oct 22, 2020
@cwhitten
Copy link
Member

cwhitten commented Nov 9, 2020

@Batta32 any updates?

@Batta32
Copy link
Collaborator

Batta32 commented Nov 9, 2020

Hi @cwhitten, this issue is already fixed with the PR #3559 which is updated with the requested changes of @peterinnesmsft .

As soon as the PR is merged, the changes will be present in the next branch.

@tonyanziano
Copy link

What is the status of #3559? @Batta32

The last update on it was on September 11

Thanks

@Batta32
Copy link
Collaborator

Batta32 commented Dec 11, 2020

Hi @tonyanziano, the PR #3559 is ready to be reviewed and merged containing the requested changes of @peterinnesmsft.

As soon as the PR is merged, the changes will be present in the next branch.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The issue hasn't been updated in a long time and will be automatically closed. label Jan 11, 2021
@Batta32 Batta32 removed the stale The issue hasn't been updated in a long time and will be automatically closed. label Jan 11, 2021
@a-b-r-o-w-n
Copy link

@joshgummersall what is the status of #3752? When do you expect to merge that in?

@joshgummersall
Copy link
Contributor

Just as soon as someone else can review it and approve!

Batta32 pushed a commit to southworks/botframework-solutions that referenced this issue Jan 26, 2021
@Virtual-Josh
Copy link

@joshgummersall,

Can you work to get the PR merged so this ticket can be closed?

@joshgummersall
Copy link
Contributor

@Virtual-Josh, it's just waiting on one more PR review; it's been approved by one reviewer already. Not sure who to ping for a second review.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. in-progress The issue is assigned and it is being worked on. Team: Kobuk This issue is assigned to the Kobuk team.
Projects
None yet
10 participants