fix: improve onboarding and unify environment variables#166
Closed
Benbenzhouz wants to merge 13 commits intomainfrom
Closed
fix: improve onboarding and unify environment variables#166Benbenzhouz wants to merge 13 commits intomainfrom
Benbenzhouz wants to merge 13 commits intomainfrom
Conversation
…ogle OAuth error handling
- Fix unsafe return type in twilio.module.ts - Remove unnecessary condition checks in auth.controller.ts - Fix type definitions in google.strategy.ts - Add eslint-disable comments for console statements - Fix type assertions in auth.controller.ts
- Use 'as unknown as Twilio.Twilio' for mock client type assertion - Add eslint-disable comment for unsafe return type - Fixes TS2740 error for missing properties in mock object
- Add return types to mock Twilio client functions - Replace || with ?? operators in auth.controller.ts - Fix unnecessary optional chain in google.strategy.ts - Add eslint-disable for console.warn in google.strategy.ts - Use non-null assertion for validated profile fields
- Fix unnecessary optional chain error - Add non-null assertion since emails[0] is validated in the check above
- Replace profile?.emails?.[0] with explicit checks - profile parameter is not optional, so optional chain is unnecessary - Fixes @typescript-eslint/no-unnecessary-condition error
…eter - profile is a required parameter, so !profile check is unnecessary - Use profile.emails.length === 0 instead of !profile.emails[0] - Destructure name and emails directly from profile after validation - Fixes @typescript-eslint/no-unnecessary-condition error
- Add apiVersion to Stripe initialization for consistency - Fix street address parsing for 3-part and 4-part address formats - Add explicit demo step ID check (step 6) for skip action - Improve address parsing error message to show missing fields
- Create unified app-config.ts utility for frontend URL and CORS origin - Support multiple environment variable names for flexibility: - APP_URL, FRONTEND_URL, NEXT_PUBLIC_APP_URL (for frontend URL) - CORS_ORIGIN, FRONTEND_URL, APP_URL (for CORS origin) - Update all files to use centralized config functions - Improve compatibility with different environment variable naming conventions Files updated: - src/utils/app-config.ts (new) - src/main.ts - src/modules/auth/auth.controller.ts - src/modules/stripe/stripe.service.ts - src/modules/google-calendar/calendar-oauth.controller.ts
- Use only APP_URL for frontend URL (remove FRONTEND_URL, NEXT_PUBLIC_APP_URL) - Use only CORS_ORIGIN for CORS (remove FRONTEND_URL, APP_URL fallbacks) - Simplify code by using single environment variable names
- Add import statement for getAppUrl from @/utils/app-config - Fixes TypeScript error: Cannot find name 'getAppUrl'
- Add forgot-password endpoint - Add reset-password endpoint - Add ResetPasswordDto - Update auth service with password reset logic
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the onboarding flow with improved address parsing and error handling, while unifying environment variable management through a centralized configuration utility. The changes also include better handling of missing third-party service credentials in development environments.
- Centralized environment variable access through new
app-config.tsutility functions - Enhanced Australian address parsing with multiple regex patterns and manual fallback logic
- Improved error handling for Google OAuth, Stripe, and Twilio when credentials are missing in development
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/app-config.ts | New utility file providing centralized access to APP_URL and CORS_ORIGIN environment variables with development fallbacks |
| src/modules/stripe/stripe.service.ts | Migrated to use getAppUrl(), added development mode handling for missing Stripe credentials with dummy key fallback |
| src/modules/onboarding/onboarding.service.ts | Enhanced address parsing with three regex patterns and manual parsing fallback, improved demo step skip handling, better error messages |
| src/modules/health/health.controller.ts | Formatting improvements to multi-line API decorators for better readability |
| src/modules/google-calendar/calendar-oauth.controller.ts | Migrated from direct process.env.APP_URL access to getAppUrl() utility |
| src/modules/company/schema/company.schema.ts | Whitespace cleanup (trailing space removal) |
| src/modules/auth/strategies/google.strategy.ts | Added development mode handling with dummy credentials, improved type safety with optional profile fields |
| src/modules/auth/dto/reset-password.dto.ts | Whitespace cleanup (trailing space removal) |
| src/modules/auth/auth.service.ts | Import order correction (moved process import to proper position) |
| src/modules/auth/auth.controller.ts | Import order fix, migrated to getAppUrl(), added OAuth error handling with proper redirects, formatting improvements |
| src/main.ts | Migrated CORS origin configuration to use getCorsOrigin() utility |
| src/lib/twilio/twilio.module.ts | Added development mode handling with mock Twilio client when credentials are missing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Special case: if this is the demo step (step 6, field is empty), and user clicks Skip or Demo, | ||
| // mark onboarding as completed (nextStep will be beyond the last step) | ||
| // Only treat empty field as skip action for the specific demo step (step 6) | ||
| const DEMO_STEP_ID = 6; |
There was a problem hiding this comment.
The DEMO_STEP_ID constant is defined inside a method, which means it's recreated on every method call. Consider moving this constant to the class level alongside the other constants (AU_ADDR_REGEX_STRICT, etc.) for better performance and maintainability.
Contributor
Author
|
Closing to restore original environment variable names |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Onboarding Improvements
Environment Variables Unification
Other Fixes
Environment Variables