feat(web): Add boost chat panel for 'Vinnumálastofnun' #17071
Conversation
WalkthroughThe pull request introduces changes to several components, primarily focusing on the chat functionality. The 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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17071 +/- ##
==========================================
- Coverage 35.69% 35.69% -0.01%
==========================================
Files 6938 6938
Lines 147110 147122 +12
Branches 41820 41824 +4
==========================================
Hits 52516 52516
- Misses 94594 94606 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportBranch report: ✅ 0 Failed, 84 Passed, 0 Skipped, 26.92s Total Time |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
apps/web/components/ChatPanel/BoostChatPanel/config.ts (1)
9-10: Ensure consistent naming conventionThe ID and conversationKey use the same value '313vinnumalastofnun'. While this works, consider using a more descriptive conversation key that reflects the purpose or context of the chat panel.
- id: '313vinnumalastofnun', - conversationKey: '313vinnumalastofnun', + id: '313vinnumalastofnun', + conversationKey: 'vinnumalastofnun-chat',apps/web/hooks/usePlausiblePageview.ts (2)
Line range hint
1-46: Consider refactoring the global state management.The hook uses a global
newestVisitedUrlvariable which could cause issues with concurrent renders or multiple hook instances. Consider using React's state management primitives instead.Here's a suggested refactor:
-let newestVisitedUrl = '' export const usePlausiblePageview = (domain?: string | null) => { const router = useRouter() + const newestVisitedUrlRef = useRef('') useEffect(() => { const onRouteChangeComplete = () => { if ( publicRuntimeConfig.environment !== 'prod' || !domain || - newestVisitedUrl === window.location.href + newestVisitedUrlRef.current === window.location.href ) { return } - newestVisitedUrl = window.location.href + newestVisitedUrlRef.current = window.location.href // Rest of the code... } // Rest of the code... return () => { router.events.off('routeChangeComplete', onRouteChangeComplete) - newestVisitedUrl = '' + newestVisitedUrlRef.current = '' } }, [domain, router.events]) }
Line range hint
24-34: Enhance error handling and configurability.The Plausible API integration could be improved with:
- Error handling for failed requests
- Configuration for the API URL
- Retry logic for failed requests
Here's a suggested improvement:
+const PLAUSIBLE_API_URL = publicRuntimeConfig.plausibleApiUrl || 'https://plausible.io/api/event' +const MAX_RETRIES = 3 + +const trackPageview = async (domain: string, retryCount = 0) => { + try { fetch('https://plausible.io/api/event', { method: 'POST', headers: { 'Content-Type': 'text/plain', }, body: JSON.stringify({ domain: domain, name: 'pageview', url: window.location.href, referrer: window.document.referrer || null, }), }) + } catch (error) { + console.error('Failed to track pageview:', error) + if (retryCount < MAX_RETRIES) { + setTimeout(() => trackPageview(domain, retryCount + 1), 1000 * Math.pow(2, retryCount)) + } + } +}apps/web/components/Organization/Wrapper/OrganizationWrapper.tsx (2)
1132-1141: Consider adding error logging for namespace parsing failuresWhile the error handling is robust, adding error logging would help debug potential JSON parsing issues in production.
const namespace = useMemo(() => { try { return JSON.parse( organizationPage.organization?.namespace?.fields || '{}', ) } catch (error) { + console.error('Failed to parse organization namespace:', error); return {} } }, [organizationPage.organization?.namespace?.fields])
962-974: Enhance type safety for Boost chat panel endpoint lookupThe current implementation is good, but we can improve type safety by using a type guard instead of type assertion.
const organizationIdWithBoost = organizationIds.find((id) => { return id in boostChatPanelEndpoints }) if (organizationIdWithBoost) { + const endpoint = boostChatPanelEndpoints[organizationIdWithBoost] + if (endpoint) { return ( <BoostChatPanel - endpoint={ - organizationIdWithBoost as keyof typeof boostChatPanelEndpoints - } + endpoint={organizationIdWithBoost} /> ) + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
apps/web/components/ChatPanel/BoostChatPanel/config.ts(1 hunks)apps/web/components/Organization/Wrapper/OrganizationWrapper.tsx(5 hunks)apps/web/hooks/usePlausiblePageview.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/web/components/ChatPanel/BoostChatPanel/config.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."
apps/web/components/Organization/Wrapper/OrganizationWrapper.tsx (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."
apps/web/hooks/usePlausiblePageview.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 (4)
apps/web/components/ChatPanel/BoostChatPanel/config.ts (2)
8-11:
Verify the security of the chat panel endpoint
The chat panel URL is using HTTP instead of HTTPS. This could potentially expose user conversations to security risks.
Let's verify if other chat panel endpoints in the codebase use HTTPS:
7-8: Verify Contentful entry reference
The comment references a Contentful entry ID. Let's ensure this matches our configuration.
✅ Verification successful
Contentful entry ID is correctly referenced
The Contentful entry ID co6SSvHZjUpEICpTlJT1B is correctly used and matches between both files:
- It's defined in
InstitutionContentfulIdsenum asVINNUMALASTOFNUN - It's properly referenced in the BoostChatPanel config for Vinnumálastofnun organization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the Contentful entry ID is used consistently
rg "co6SSvHZjUpEICpTlJT1B" --type ts
Length of output: 359
Script:
#!/bin/bash
# Let's check the content of both files to understand the relationship
cat libs/application/types/src/lib/InstitutionContentfulIds.ts
echo "---"
cat apps/web/components/ChatPanel/BoostChatPanel/config.ts
Length of output: 1934
apps/web/hooks/usePlausiblePageview.ts (1)
9-9: LGTM! Verify type usage across the codebase.
The type change from string? to string | null? makes nullability more explicit, which is good TypeScript practice.
Let's verify the usage of this hook across the codebase:
✅ Verification successful
Type change is safe and correctly used across the codebase
All usages of usePlausiblePageview are handling the domain parameter correctly:
article?.organization?.[0]?.trackingDomain ?? undefinedorganization?.trackingDomainorganization?.trackingDomain ?? undefinedDIGITAL_ICELAND_PLAUSIBLE_TRACKING_DOMAIN
The type change from string? to string | null? is backward compatible with existing code as all call sites either:
- Use optional chaining with nullish coalescing to undefined
- Pass a constant string
- Pass undefined explicitly
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of usePlausiblePageview to ensure they handle null correctly
# Search for hook usage
rg -A 2 "usePlausiblePageview" --type ts --type tsx
Length of output: 79
Script:
#!/bin/bash
# Search for hook usage with ripgrep, without file type restrictions
rg "usePlausiblePageview"
# Also search for potential imports of this hook
rg "from ['\"].*usePlausiblePageview['\"]"
Length of output: 1446
apps/web/components/Organization/Wrapper/OrganizationWrapper.tsx (1)
34-35: LGTM: Import statements are correctly placed
The new imports for BoostChatPanel components are properly organized within the existing import block.
Add boost chat panel for 'Vinnumálastofnun'
What
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
Release Notes
New Features
OrganizationWrappercomponent to conditionally render the new chat panel based on organization IDs.Improvements
domainparameter in the pageview tracking hook.These updates enhance user experience by providing new chat functionalities and improving the reliability of existing features.