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

[TypeScript][Virtual Assistant] fix: localize qna maker dialog IDs #3752

Merged
merged 3 commits into from
Feb 9, 2021
Merged

[TypeScript][Virtual Assistant] fix: localize qna maker dialog IDs #3752

merged 3 commits into from
Feb 9, 2021

Conversation

joshgummersall
Copy link
Contributor

Fixes #3496

@Batta32 Batta32 self-requested a review January 11, 2021 11:59
Batta32
Batta32 approved these changes Jan 12, 2021
View reviewed changes
Copy link
Collaborator

@Batta32 Batta32 left a comment

Choose a reason for hiding this comment

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

LGTM! Just a minor comment.

templates/typescript/samples/sample-assistant/src/dialogs/mainDialog.ts Outdated
@@ -40,6 +40,7 @@ import { BotServices } from '../services/botServices';
import { IBotSettings } from '../services/botSettings';
import { StateProperties } from '../models/stateProperties';
import { OnboardingDialog } from './onboardingDialog';
import { QnaMakerService } from 'botframework-config';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this import necessary? It's not used in the MainDialog and in the template is not added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a remnant of an earlier change that I backed out. Fixed!

@Batta32 Batta32 self-requested a review January 12, 2021 17:57
@a-b-r-o-w-n a-b-r-o-w-n mentioned this pull request Jan 25, 2021
Closed
VictorGrycuk added a commit to southworks/botframework-solutions that referenced this pull request Jan 26, 2021
@VictorGrycuk
Apply changes of PR microsoft#3752
eba3355
@Batta32
Copy link
Collaborator

Batta32 commented Jan 26, 2021

Hi @joshgummersall, we were using these changes internally and we noticed that for the faq dialog IDs the issue still happens.

We fixed the problem replicating the changes for that scenario. Can we apply the same changes for this case?

Replicated changes for q_faq
image

@joshgummersall
Copy link
Contributor Author

@Batta32, rather than duplicate the code, I refactored the common elements to a helper method. This should achieve the same thing, though.

@srinaath
Copy link

srinaath commented Feb 5, 2021

@lauren-mills can we get another review on this PR to get it merged?

@joshgummersall
Copy link
Contributor Author

@lauren-mills, is there one more person I can request to review this PR? It looks like I need two approving reviews to merge.

@lauren-mills lauren-mills merged commit 65df3be into master Feb 9, 2021
@delete-merged-branch delete-merged-branch bot deleted the jpg/localized-qna-dialogs branch February 9, 2021 19:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Batta32 Batta32 Batta32 approved these changes

@lauren-mills lauren-mills lauren-mills approved these changes

Assignees
No one assigned
Labels
PR: merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

QnAMakerDialog is not created for different languages
4 participants
@joshgummersall @Batta32 @srinaath @lauren-mills