Minor bug fixes #1 #9
Conversation
Summary by CodeRabbit
WalkthroughThis pull request introduces several updates to the project's configuration and dependencies. New environment variables, including Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Proxy as "Proxy Middleware"
participant API as "MeetingBaas API"
Client->>Proxy: Send request to /api/meetingbaas/...
Proxy->>API: Rewrite URL and forward request
API-->>Proxy: Return API response
Proxy-->>Client: Deliver response
sequenceDiagram
participant User
participant Browser
participant WebApp as "Login Route"
participant Auth as "Authentication Service"
User->>Browser: Navigate to Login Page
Browser->>WebApp: Load login component (with VITE_WEB_URL)
WebApp->>Auth: Call signIn.social(callbackURL from VITE_WEB_URL)
Auth-->>WebApp: Return authentication result
WebApp-->>Browser: Render login outcome
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🧰 Additional context used🪛 LanguageToolCONTRIBUTING.md[uncategorized] ~94-~94: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) 🔇 Additional comments (9)
✨ Finishing Touches
🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.env.example(1 hunks).gitignore(1 hunks)apps/api/.env.example(1 hunks)apps/api/README.md(1 hunks)apps/api/src/lib/auth.ts(1 hunks)apps/docs/content/docs/concepts/environment-variables.mdx(2 hunks)apps/docs/next.config.mjs(0 hunks)apps/docs/package.json(2 hunks)apps/proxy/.env.example(1 hunks)apps/proxy/nitro.config.ts(2 hunks)apps/proxy/package.json(1 hunks)apps/proxy/server/middleware/cors.ts(1 hunks)apps/proxy/server/middleware/meetingbaas-proxy.ts(1 hunks)apps/proxy/server/middleware/s3-proxy.ts(1 hunks)apps/web/package.json(3 hunks)apps/web/src/components/settings/settings-form.tsx(2 hunks)apps/web/src/lib/constants.ts(1 hunks)apps/web/src/routes/login.tsx(2 hunks)package.json(2 hunks)packages/shared/package.json(1 hunks)packages/ui/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)turbo.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/docs/next.config.mjs
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/src/routes/login.tsx (1)
apps/web/src/lib/constants.ts (1)
VITE_WEB_URL(3-3)
🔇 Additional comments (32)
packages/shared/package.json (1)
28-28: Approved: React typings dependency updateThe change from
"catalog:react19"to"catalog:"aligns with the PR objective of rolling back React from v19 to v18 for better compatibility with external libraries.packages/ui/package.json (1)
95-95: Approved: React dependencies updated consistentlyThe changes from
"catalog:react19"to"catalog:"for both direct and peer dependencies of React and its type definitions align with the PR objective of rolling back from React v19 to v18 for better compatibility with external libraries.Also applies to: 98-98, 103-103
apps/web/src/lib/constants.ts (1)
3-3: Approved: New web URL constant addedAdding the
VITE_WEB_URLconstant properly exports the environment variable required for fixing the authentication redirect URL, as mentioned in the PR objectives. This ensures the authentication flow redirects to the web URL instead of the API endpoint..env.example (1)
11-12: Approved: Web app URL environment variable addedThe addition of the
VITE_WEB_URLenvironment variable with its default local development value matches the PR requirements for fixing the authentication redirect issue. The comment clearly explains the variable's purpose.apps/api/README.md (1)
11-13: Clear Addition ofNITRO_WEB_URL
The new environment variableNITRO_WEB_URLis clearly documented alongside the other required variables. This enhances clarity for deployment configuration.turbo.json (1)
52-58: Correct Inclusion ofVITE_WEB_URLin Global Environment
Adding"VITE_WEB_URL"to theglobalEnvarray ensures that tasks relying on environment configurations can access the web URL consistently. Please verify that the corresponding.env.examplehas the expected value..gitignore (1)
8-9: New Ignored Directory Addition
The addition of.pnpm-store/*in the.gitignorefile prevents inadvertent tracking of pnpm store artifacts, keeping the repository clean.apps/api/.env.example (1)
1-3: API URL Correction Confirmed
The update ofMEETINGBAAS_API_URLto"https://api.meetingbaas.com"(without the hyphen) aligns with the live API endpoint. This correction should resolve issues stemming from the previous URL mismatch.apps/proxy/package.json (1)
33-37: Dependency Updates Verified
The introduction of theh3-proxydependency (^1.13.0) in the dependencies section is appropriate for supporting new middleware features. Additionally, the version declaration forfirebase-functions-testappears consistent. Ensure that any middleware usingh3-proxyis thoroughly tested after this change.apps/api/src/lib/auth.ts (1)
9-9: Good addition of CORS trusted origins configurationAdding the
trustedOriginsproperty to thebetterAuthconfiguration is a good solution for the CORS issues mentioned in the PR objectives. This implementation properly reads from the environment variable and has a sensible fallback.apps/docs/content/docs/concepts/environment-variables.mdx (2)
187-187: API URL correction is consistent with PR objectivesThe update to
MEETINGBAAS_API_URLremoves the hyphen in the domain, changing fromapi.meeting-baas.comtoapi.meetingbaas.com, which aligns with the PR objective to correct the API URL for Meeting Baas.
255-255: Consistent API URL updateThis change correctly updates the same
MEETINGBAAS_API_URLin the API configuration section, maintaining consistency across the documentation.apps/web/src/routes/login.tsx (2)
8-8: Good import of environment-based web URL constantAdding this import is a good first step to making the callback URL configurable via environment variables, which addresses the auth redirect issue mentioned in the PR objectives.
27-27: Auth redirect URL fix implemented correctlyThis change properly implements the fix for the auth redirect issue by using the
VITE_WEB_URLenvironment variable to dynamically construct the callback URL instead of hardcoding it. This ensures the auth flow redirects to the web URL as specified in the PR objectives.apps/proxy/.env.example (2)
2-2: Consistent API URL correctionThis update correctly changes the MeetingBaas API URL to remove the hyphen, which aligns with the changes made in the documentation and addresses the API URL correction objective mentioned in the PR.
4-4: Good addition of web URL environment variableAdding
NITRO_WEB_URLprovides the necessary configuration for the auth redirect fix mentioned in the PR objectives. This variable will be important for maintaining proper redirection in the authentication flow.Verify that the corresponding variable has been added to your deployment environments as well, not just in the .env.example file.
apps/proxy/server/middleware/meetingbaas-proxy.ts (1)
6-12: Good implementation of API routing with path rewritingThis proxy middleware correctly implements the path rewriting to route requests from
/api/meetingbaas/**to the Meeting Baas API URL. This aligns with the PR objective to update the API URL from staging to live.apps/proxy/server/middleware/s3-proxy.ts (1)
6-12: Well-structured S3 proxy implementationThe S3 proxy middleware is well implemented with appropriate path rewriting and filtering. This supports the CORS resolution mentioned in the PR objectives.
package.json (1)
41-42: React dependency changes aligned with PR objectivesUpdating React type dependencies from "catalog:react19" to "catalog:" aligns with the PR objective to roll back from React v19 to v18 for compatibility with external libraries.
apps/proxy/nitro.config.ts (3)
6-6: Approve API URL correctionThe Meeting Baas API URL has been updated by removing the hyphen, changing from
meeting-baas.comtomeetingbaas.com. This aligns with the PR objective to update the API URL.
9-9: Approve web URL configurationAdding the
NITRO_WEB_URLconstant with a default value provides proper fallback while allowing for environment-specific configuration.
14-17: Approve runtime configuration additionThe new
runtimeConfigsection properly exposes trusted origins and API URLs to the application. This change correctly addresses the CORS issues mentioned in the PR objectives.pnpm-workspace.yaml (1)
14-18: Approve React and Next.js version rollbackThe catalog entries have been correctly updated to use React 18 (
^18.3.0) and Next.js 14 (^14.2.25), which aligns with the PR objective to ensure compatibility with external libraries that don't support React v19.apps/web/package.json (1)
67-68: Approve type definitions updateThe type definitions have been correctly updated to use the catalog entries for React 18.
apps/web/src/components/settings/settings-form.tsx (3)
98-98: Approve session error handling additionAdding the
errorproperty fromuseSessionis a good improvement that enables proper error handling in the component.
254-258: Approve improved session display logicThe conditional rendering has been enhanced to properly handle three states (loading, error, authenticated), which addresses the runtime error mentioned in the PR objectives.
261-267: Approve login/logout button logicThe conditions for showing SignOut and Login buttons have been updated to correctly respond to authentication state, fixing the runtime error that occurred when a user was not authenticated.
apps/docs/package.json (5)
36-36: Next.js Dependency Update Verification.
Please verify that the catalog reference for Next.js indeed resolves to the intended version (as per PR objectives, it should target Next.js v14) to ensure compatibility with the external libraries.
38-38: React Dependency Update Check.
Confirm that the catalog reference for React is resolving to version 18 rather than v19, in line with the compatibility rollback described in the PR objectives.
39-39: React-DOM Version Confirmation.
Ensure that the catalog reference for React-DOM resolves to the correct version (v18) so that it remains consistent with the React version update.
60-60: Type Definitions (@types/react) Validation.
Verify that the catalog reference used for "@types/react" aligns with the intended React version (v18) across the project to avoid any type mismatches.
61-61: Type Definitions (@types/react-dom) Check.
Ensure that the catalog reference for "@types/react-dom" matches the expected version for React-DOM to maintain consistency with the React dependency.
| if (corsHandled) { | ||
| return; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Missing default handler for non-CORS requests
The middleware returns early if CORS is handled, but doesn't provide a default path for when CORS isn't handled. This might lead to incomplete request processing if this is the only middleware in the chain.
if (corsHandled) {
return;
}
+ // Continue to the next handler for non-CORS requests
});Committable suggestion skipped: line range outside the PR's diff.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/proxy/server/middleware/cors.ts (1)
15-18: 🧹 Nitpick (assertive)Missing default handler for non-CORS requests
The middleware returns early if CORS is handled, but doesn't provide a default path for when CORS isn't handled. This might lead to incomplete request processing.
if (corsHandled) { return; } + // Continue to the next handler for non-CORS requests });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/proxy/server/middleware/cors.ts(1 hunks)apps/proxy/server/middleware/meetingbaas-proxy.ts(1 hunks)
🔇 Additional comments (4)
apps/proxy/server/middleware/cors.ts (2)
1-3: Proper imports added ✓The imports for H3 and Nitro functions have been correctly added as suggested in previous reviews.
4-14: CORS middleware implementation looks goodThe CORS middleware properly configures allowed origins from runtime config, handles preflight requests, and sets appropriate CORS headers.
apps/proxy/server/middleware/meetingbaas-proxy.ts (2)
1-3: Proper imports added ✓The imports for
eventHandleranduseRuntimeConfighave been correctly added as suggested in previous reviews.
5-17: Proxy implementation matches PR objectivesThe proxy middleware correctly implements the Meeting Baas API URL correction mentioned in the PR objectives. The implementation looks good with appropriate path rewriting and filtering.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/proxy/server/middleware/s3-proxy.ts(1 hunks)
🔇 Additional comments (2)
apps/proxy/server/middleware/s3-proxy.ts (2)
6-6:❓ Verification inconclusive
Verify the configuration value in your environment.
Since the PR mentions updating API URLs from staging to live, ensure that the
meetingBaasS3Urlconfiguration is correctly set in your environment files to point to the live S3 endpoint.
🏁 Script executed:
#!/bin/bash # Check the configuration for S3 URL in proxy app echo "Checking for meetingBaasS3Url configuration..." rg "meetingBaasS3Url" apps/proxyLength of output: 372
Action Required: Confirm MEETINGBAAS_S3_URL is Set to the Live S3 Endpoint
The configuration for
meetingBaasS3Urlis correctly referenced inapps/proxy/nitro.config.ts(usingMEETINGBAAS_S3_URL) and injected intoapps/proxy/server/middleware/s3-proxy.ts. However, since this PR updates API URLs from staging to live, please double-check that theMEETINGBAAS_S3_URLenvironment variable in your live deployment is pointing to the correct live S3 endpoint.
13-13:❓ Verification inconclusive
Consider adding CORS-related headers if needed for S3 access.
Since this PR addresses CORS issues, verify if this S3 proxy also needs CORS headers for browser access. If your frontend directly communicates with this endpoint, you might need to handle preflight requests and add appropriate CORS headers.
Consider running the following test to check if frontend requests to this endpoint work properly:
🏁 Script executed:
#!/bin/bash # Check if there are any frontend calls to the S3 proxy echo "Checking for frontend calls to S3 proxy endpoint..." rg --type=js --type=ts --type=tsx --type=jsx "/api/s3" apps/webLength of output: 204
Action Required: Manually Verify Frontend Usage for S3 Proxy CORS Handling
The S3 proxy implementation in
apps/proxy/server/middleware/s3-proxy.ts(line 13) currently does not include CORS headers, which might be necessary if a frontend directly accesses this endpoint. An automated search for/api/s3in theapps/webdirectory returned an error (unrecognized file type: tsx), so we couldn’t confidently determine if there are frontend calls to this endpoint.Next Steps:
- Manually verify if any frontend code calls
/api/s3.- If such calls exist, update the proxy to handle preflight requests and include appropriate CORS headers.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
List of bugs fixed:
React version mismatch with dependencies - Rolled back to React v18 and Next.js v14 to sync with external libraries that do not support React v19
Cors resolution - Fixed Cors issues in auth flow and proxy
API URL Correction - Changed meeting baas API URL from staging api to live api
Auth redirect issue - Corrected auth redirect url to redirect to web URL instead of API endpoint
IMPORTANT: Requires creating a new .env key in root env named VITE_WEB_URL with value as the URL of the web app (https://app.transcriptseeker.com/)
Runtime error fix: fixed error on settings page when user isn't authenticated