-
Notifications
You must be signed in to change notification settings - Fork 61
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
fix(web): Update endpoint for IBM Watson to now send feedback to Cloudant #16817
Conversation
WalkthroughThe changes in this pull request introduce a new property, Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.config.ts (1)
10-10
: LGTM! Consider documenting the new configuration.The schema addition follows the established pattern and properly validates the new Cloudant database name configuration.
Consider adding a comment or updating the documentation to explain the purpose and requirements of this new configuration property, especially since it's part of the migration to Cloudant.
libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.service.ts (1)
61-64
: Explicitly specify fields included in the feedback documentTo avoid unintentionally storing sensitive or unnecessary data, consider explicitly specifying the fields from
input
to include in the document. This enhances security by ensuring only intended data is persisted.Proposed change:
document: { // Specify only the necessary fields sessionId: input.sessionId, feedbackText: input.feedbackText, thumbStatus: thumbStatusToNumberMap[input.thumbStatus], timestamp: new Date().toISOString(), },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (3)
libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.config.ts
(2 hunks)libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.service.ts
(3 hunks)package.json
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.config.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.service.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (4)
package.json (1)
100-100
: Verify security advisories for @ibm-cloud/cloudant.
Let's check for any known security vulnerabilities in the chosen version.
✅ Verification successful
No known security vulnerabilities found for @ibm-cloud/cloudant version 0.11.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for security advisories for the @ibm-cloud/cloudant package.
# Check for security advisories using GitHub's GraphQL API
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@ibm-cloud/cloudant") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
# Check if the package is listed in package-lock.json or yarn.lock
echo "Checking lock files for transitive dependencies..."
rg -A 5 "@ibm-cloud/cloudant" "*lock*"
Length of output: 548
Script:
#!/bin/bash
# Description: Verify the presence of lock files and check for transitive dependencies of @ibm-cloud/cloudant.
# List common lock files
LOCK_FILES=("package-lock.json" "yarn.lock" "pnpm-lock.yaml")
echo "Checking for lock files..."
found_lock_files=0
for lock in "${LOCK_FILES[@]}"; do
if [[ -f "$lock" ]]; then
echo "Found lock file: $lock"
found_lock_files=1
echo "Searching for @ibm-cloud/cloudant in $lock..."
rg "@ibm-cloud/cloudant" "$lock"
fi
done
if [[ $found_lock_files -eq 0 ]]; then
echo "No lock files found in the repository."
fi
Length of output: 681
libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.service.ts (3)
4-4
: Import Cloudant SDK correctly
The Cloudant SDK is correctly imported, allowing for integration with the Cloudant database service.
16-16
: Declaration of the Cloudant client instance
Declaring private cloudant: CloudantV1
prepares the service for Cloudant operations and aligns with TypeScript best practices.
68-68
: Access response.result.ok
correctly
Using response.result.ok
correctly accesses the success status from the Cloudant response, conforming to the SDK's response structure.
libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.config.ts
Show resolved
Hide resolved
libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.service.ts
Show resolved
Hide resolved
libs/api/domains/watson-assistant-chat/src/lib/watson-assistant-chat.service.ts
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 101 Total Test Services: 0 Failed, 98 Passed Test ServicesThis report shows up to 10 services
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
apps/api/infra/api.ts
(1 hunks)charts/islandis/values.dev.yaml
(1 hunks)charts/islandis/values.prod.yaml
(1 hunks)charts/islandis/values.staging.yaml
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/api/infra/api.ts (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (5)
charts/islandis/values.staging.yaml (2)
330-330
: Verify Cloudant database access configuration
Ensure that the corresponding Cloudant database credentials and connection details are properly configured in the secrets section.
#!/bin/bash
# Check for related Watson/Cloudant secrets configuration
rg "WATSON.*FEEDBACK" secrets
330-330
: Environment variable addition aligns with PR objectives
The addition of WATSON_ASSISTANT_CHAT_FEEDBACK_DB_NAME
with value 'island-is-assistant-feedback' correctly implements the requirement to send feedback to Cloudant database.
✅ Verification successful
All Watson Assistant configurations are consistent
Verified that WATSON_ASSISTANT_CHAT_FEEDBACK_DB_NAME
is properly integrated across staging, development, and production environments. No additional changes required.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if there are any other Watson Assistant related configurations that need to be updated
rg -A 5 "WATSON_ASSISTANT"
Length of output: 15447
charts/islandis/values.prod.yaml (1)
318-318
: LGTM! Verify related Watson Assistant configuration
The addition of WATSON_ASSISTANT_CHAT_FEEDBACK_DB_NAME
environment variable aligns with the PR objectives to send feedback to Cloudant. The value appears appropriate for production use.
Let's verify the presence of all required Watson Assistant configuration:
✅ Verification successful
Verified: All related Watson Assistant configurations are present and correctly set up.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for all required Watson Assistant configuration
# Expected: All Watson Assistant related environment variables and secrets should be present
echo "Checking Watson Assistant environment variables..."
rg "WATSON_ASSISTANT_CHAT" -A 1
echo "Checking Watson Assistant secrets..."
rg "watson.*assistant.*" -i
Length of output: 21427
charts/islandis/values.dev.yaml (2)
330-330
: Verify related Watson Assistant configuration
The new Cloudant database configuration should have corresponding secrets for authentication. I notice WATSON_ASSISTANT_CHAT_FEEDBACK_API_KEY
and WATSON_ASSISTANT_CHAT_FEEDBACK_URL
are present in the secrets section, which is good. However, ensure these are properly configured in the secrets management system.
#!/bin/bash
# Check if all required Watson/Cloudant configuration is present
# Look for any Watson/Cloudant related configuration patterns
# Search for Watson/Cloudant related configuration
rg -i "watson|cloudant" -g "!*.yaml"
# Check for any TODO or FIXME comments related to Watson/Cloudant
rg -i "todo|fixme|note" -A 3 -B 3 | rg -i "watson|cloudant"
Also applies to: 1055-1056
330-330
: Verify the database name value for security best practices
The hardcoded database name 'island-is-assistant-feedback' could potentially expose information about the system structure. Consider using a more generic name or moving this value to a secret.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Core files LGTM
…dant (#16817) * Update endpoint * Add env variable * chore: charts update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Update endpoint for IBM Watson to now send feedback to Cloudant
What
I'm basing this off a code snippet that Origo sent us, there they are using the cloudant sdk which is why I added it to package.json
Why
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Chores