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.

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

Feature/cs 216 multilingual agent #57

Merged
merged 15 commits into from Apr 3, 2019

Conversation

Projects
None yet
2 participants
@ThWoywod
Copy link
Contributor

ThWoywod commented Feb 27, 2019

For the ability to generate Dialogflow compliant multilingual agents we have to change the multilingual generation behavior.
Currently AssistantJS calls all platform generators with each language. So AssistantJS handles the behavior how a multilingual agent should look like.
With this feature we allow the platform generator to handle the multilingual behavior.

@ThWoywod ThWoywod requested a review from antoniusostermann Feb 27, 2019

@antoniusostermann
Copy link
Collaborator

antoniusostermann left a comment

Well done! It would be nice if we could refactor the generator a little bit while doing this. Just gave you some hints for this. :-)

// Get all configured language keys from utterance templates
const configuredLanguages = Object.keys(utteranceTemplates);

// Throws an missing utterances exception because no utterances will be configured.

This comment has been minimized.

Copy link
@antoniusostermann

antoniusostermann Feb 28, 2019

Collaborator

typo: "are" configured instead of "will be"

const buildIntentConfigs: PlatformGenerator.Multilingual<PlatformGenerator.IntentConfiguration[]> = {};

// Prepare language specific configuration
configuredLanguages.forEach(language => {

This comment has been minimized.

Copy link
@antoniusostermann

antoniusostermann Feb 28, 2019

Collaborator

Could we create a new sub method for this block? Would be nice to improve the generator code a little bit by the way

// If no utterance for the current language will be given we have to set a default value.
if (!utterances[language]) utterances[language] = {};

// Associate utterances to intent

This comment has been minimized.

Copy link
@antoniusostermann

antoniusostermann Feb 28, 2019

Collaborator

Let's build a new private helper method for this :-)

}

// When utterances are "undefined", assign empty array
if (typeof intentUtterances === "undefined") intentUtterances = [];

This comment has been minimized.

Copy link
@antoniusostermann

antoniusostermann Feb 28, 2019

Collaborator

you shouldn't need this anymore, since you set intentUtterances to [] above

// When utterances are "undefined", assign empty array
if (typeof intentUtterances === "undefined") intentUtterances = [];
// Extract entities from utterances
const entities: string[] = [

This comment has been minimized.

Copy link
@antoniusostermann

antoniusostermann Feb 28, 2019

Collaborator

Let's build a new private helper method for this :-)

@@ -160,23 +160,29 @@ export namespace PlatformGenerator {
getUtterancesFor(language: string): { [intent: string]: string[] };
}

/** Generic interface for multi lingual instances */
export interface Multilingual<T> {

This comment has been minimized.

Copy link
@antoniusostermann

antoniusostermann Feb 28, 2019

Collaborator

Cool! 👍

@ThWoywod ThWoywod requested a review from antoniusostermann Mar 27, 2019

afterEach(function() {
/** Cleanup the mocked date */
jasmine.clock().uninstall();
});

This comment has been minimized.

Copy link
@antoniusostermann

antoniusostermann Mar 29, 2019

Collaborator

Don't we also have to reset the fs.-mocks over here? Isn't mkdirSync etc now still just a spy?

await this.getGenerator().execute(this.params.buildDirectory);
});

it("will not permutes any custom entities", async function(this: CurrentThisContext) {

This comment has been minimized.

Copy link
@antoniusostermann

antoniusostermann Mar 29, 2019

Collaborator

typo: "permute"

@ThWoywod ThWoywod requested a review from antoniusostermann Apr 3, 2019

@antoniusostermann antoniusostermann merged commit e4144af into develop Apr 3, 2019

3 checks passed

codeclimate 3 fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@antoniusostermann antoniusostermann deleted the feature/CS-216-multilingual-agent branch Apr 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.