Adds SF + Nice CXOne voice integration. Adds SF PlatformService refac…#79
Adds SF + Nice CXOne voice integration. Adds SF PlatformService refac…#79jblakey-google wants to merge 1 commit intoHEADfrom
Conversation
…tor & SF unit tests. (#69) * SF + Nice CXOne voice integration. SF PlatformService refactor. Adds SF unit tests. * Updates from OSS Licensing review
Summary of ChangesHello @jblakey-google, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a major architectural overhaul to the Salesforce Agent Assist Lightning Web Component (LWC). The core change involves refactoring the component to utilize a modular, service-oriented design, allowing for more flexible and maintainable integrations across different communication platforms. A key outcome of this refactor is the successful integration with Salesforce Service Cloud Voice, specifically supporting Nice CXone voice interactions. The changes also include a substantial increase in unit test coverage, ensuring the robustness of the new architecture, and several minor UI enhancements for a better user experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-designed refactoring of the Agent Assist LWC. The introduction of platform-specific services greatly improves the architecture, making it more modular and easier to extend. The addition of comprehensive unit tests is also a major improvement. My review includes a few suggestions to improve robustness and configuration, particularly around a hardcoded value that should be exposed as a property. Overall, this is a high-quality contribution that significantly enhances the component's maintainability and functionality.
| const CONFIG = { | ||
| // For this.platform = "servicecloudvoice-nice", the Nice Business Unit Number | ||
| // https://help.nicecxone.com/content/acd/businessunits/managebusinessunit.htm | ||
| niceBusNo: 1234567 // TODO: make sure this matches your Nice CXone Business Unit Number. |
There was a problem hiding this comment.
The niceBusNo is hardcoded with a TODO comment. This is a critical configuration value that will require every user of this component to modify the source code. This should be exposed as a configurable @api property on the main agentAssistContainerModule component, similar to consumerKey or endpoint.
| niceBusNo: 1234567 // TODO: make sure this matches your Nice CXone Business Unit Number. | |
| niceBusNo: 1234567 // TODO: This should be made a configurable @api property in the LWC. |
| @api get projectLocationName() { | ||
| return this.conversationProfile.split("/conversationProfiles")[0]; | ||
| } |
There was a problem hiding this comment.
The projectLocationName getter directly calls split() on this.conversationProfile. If this.conversationProfile is null or undefined when this getter is accessed, it will throw a TypeError and crash the component. It's safer to add a guard to handle cases where conversationProfile might not be available.
| @api get projectLocationName() { | |
| return this.conversationProfile.split("/conversationProfiles")[0]; | |
| } | |
| @api get projectLocationName() { | |
| if (!this.conversationProfile) { | |
| return ""; | |
| } | |
| return this.conversationProfile.split("/conversationProfiles")[0]; | |
| } |
| } else if (this.platform.includes("servicecloudvoice")) { | ||
| this.platformService = new ServiceCloudVoicePlatformService(this, refs); |
There was a problem hiding this comment.
Using this.platform.includes("servicecloudvoice") to select the platform service is a bit broad and could lead to unexpected behavior if other platform strings containing "servicecloudvoice" are introduced in the future. For better maintainability and to avoid ambiguity, it would be safer to use a more specific check, for example, checking if the platform string starts with "servicecloudvoice".
| } else if (this.platform.includes("servicecloudvoice")) { | |
| this.platformService = new ServiceCloudVoicePlatformService(this, refs); | |
| } else if (this.platform.startsWith("servicecloudvoice")) { |
| ingestDemoContextReferences() { | ||
| // Injects context into the Dialogflow conversation for demos and testing. | ||
| // https://cloud.google.com/dialogflow/es/docs/reference/rest/v2/projects.locations.conversations/ingestContextReferences | ||
| const injectContext = () => { | ||
| let url = `${this.endpoint}/v2/${this.conversationName}:ingestContextReferences`; | ||
| let body = JSON.stringify({ | ||
| contextReferences: { | ||
| context: { | ||
| contextContents: [ | ||
| { content: sampleContext, contentFormat: "JSON" } | ||
| ], | ||
| languageCode: "en-us", | ||
| updateMode: "OVERWRITE" | ||
| } | ||
| } | ||
| }); | ||
| fetch(url, this.platformService.createRequestOptions("POST", body)) | ||
| .then((res) => res.text()) | ||
| .then((data) => { | ||
| this.debugLog("ingestDemoContextReferences ran successfully"); | ||
| console.log(JSON.parse(data)); | ||
| }) | ||
| .catch((err) => { | ||
| this.debugLog(`ingestDemoContextReferences failed: ${err.message}`); | ||
| }); | ||
| }; | ||
| setTimeout(injectContext, 1000); | ||
| } |
There was a problem hiding this comment.
The ingestDemoContextReferences method constructs a URL using this.conversationName. If this method is called before conversationName is initialized, the URL will be invalid (e.g., contain .../v2/null:ingestContextReferences) and the fetch call will fail. It would be more robust to add a guard to ensure conversationName exists before proceeding.
@api
ingestDemoContextReferences() {
// Injects context into the Dialogflow conversation for demos and testing.
// https://cloud.google.com/dialogflow/es/docs/reference/rest/v2/projects.locations.conversations/ingestContextReferences
const injectContext = () => {
if (!this.conversationName) {
this.debugLog("ingestDemoContextReferences skipped: conversationName is not yet available.");
return;
}
let url = `${this.endpoint}/v2/${this.conversationName}:ingestContextReferences`;
let body = JSON.stringify({
contextReferences: {
context: {
contextContents: [
{ content: sampleContext, contentFormat: "JSON" }
],
languageCode: "en-us",
updateMode: "OVERWRITE"
}
}
});
fetch(url, this.platformService.createRequestOptions("POST", body))
.then((res) => res.text())
.then((data) => {
this.debugLog("ingestDemoContextReferences ran successfully");
console.log(JSON.parse(data));
})
.catch((err) => {
this.debugLog(`ingestDemoContextReferences failed: ${err.message}`);
});
};
setTimeout(injectContext, 1000);
}| setTimeout(injectContext, 1000) | ||
|
|
||
| } | ||
| },`; |
There was a problem hiding this comment.
The sampleContext string contains a trailing comma at the end of the accounts object, which makes it invalid JSON. While JavaScript objects allow trailing commas, this string is intended to be parsed as JSON (contentFormat: "JSON" in ingestDemoContextReferences) and will cause a parsing error on the receiving end.
}};
|
@nategd-google @matt-degraffenreid The HEAD branch is quite stale at this point but is still set as the default branch for the repo: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-branches-in-your-repository/changing-the-default-branch This means that when they clone the repo, they are checked out on |
1 participant
This PR synchronizes the
HEAD(default) branch with themainbranch.mainshould become the default branch andHEADshould be deleted sinceHEADis a reserved keyword in Git.Please see issue #71
The Problem: The repository’s default branch is currently named HEAD. Because HEAD is a reserved system keyword in Git (referring to the current checkout pointer), having a branch with this name causes "Ambiguous Reference" errors. It breaks standard Git commands, CI/CD tools, and automation scripts for anyone cloning the repo.
Commits: