Conversation
There was a problem hiding this comment.
Pull request overview
This pull request aims to fix an early signout issue by extending cookie expiration times and refactoring how tenant ID is accessed across the application. The changes consolidate cookie expiry durations and shift from using a reactive tenant ID from user session data to a static tenant ID extracted from the window hostname.
Changes:
- Extended authentication cookie expiration from 1 day to 31 days to prevent premature session termination
- Refactored tenant ID access from user session data to a static import from axiosInterceptor
- Created a new constant file for cookie expiry configuration
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/community/auth/constants/authContants.ts | New constant file defining COOKIE_EXPIRY_DAYS (31 days) for consistent cookie expiration |
| frontend/src/community/auth/utils/authUtils.ts | Updated cookie expiry logic to use new COOKIE_EXPIRY_DAYS constant, extending both accessToken and isPasswordChangedForTheFirstTime cookies to 31 days |
| frontend/src/community/common/hooks/useSessionData.ts | Removed local tenantID computation and imported it from axiosInterceptor instead |
| frontend/src/community/common/components/organisms/Drawer/Drawer.tsx | Updated to import and use tenantID directly from axiosInterceptor |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1 @@ | |||
| export const COOKIE_EXPIRY_DAYS = 31; No newline at end of file | |||
There was a problem hiding this comment.
The filename "authContants.ts" has a spelling error. It should be "authConstants.ts" (missing 's' in "Constants"). This will help maintain consistency and avoid confusion.
There was a problem hiding this comment.
shall we address this comment if not a big deal
Also why 31, usually 30 neh (1 month) ?
There was a problem hiding this comment.
Yes, we can make it 30 days as well. Just added a buffer to ensure we get the most out of the current refresh token.
There was a problem hiding this comment.
shall we then since the standard for the month is 30 @hasalarootcode
| import { TenantStatusEnums, TierEnum } from "~enterprise/common/enums/Common"; | ||
|
|
||
| import { config } from "../../../../middleware"; | ||
| import { COOKIE_EXPIRY_DAYS } from "../constants/authContants"; |
There was a problem hiding this comment.
The import statement references the misspelled filename "authContants.ts". After renaming the file to "authConstants.ts", this import will need to be updated accordingly.
| import { COOKIE_EXPIRY_DAYS } from "../constants/authContants"; | |
| import { COOKIE_EXPIRY_DAYS } from "../constants/authConstants"; |
| return { | ||
| isFreeTier, | ||
| isProTier, |
There was a problem hiding this comment.
The tenantID has been removed from the return value of useSessionData (previously returned at line 117), but it's still being imported and used in frontend/src/community/people/components/organisms/EmploymentFormSection/EmploymentDetailsForm.tsx at line 74. This will cause a runtime error where tenantID will be undefined. Either tenantID needs to be added back to the return value of useSessionData (in the return statement around line 94), or EmploymentDetailsForm.tsx needs to be updated to import tenantID directly from axiosInterceptor.
| @@ -0,0 +1 @@ | |||
| export const COOKIE_EXPIRY_DAYS = 31; No newline at end of file | |||
There was a problem hiding this comment.
shall we address this comment if not a big deal
Also why 31, usually 30 neh (1 month) ?
|
2 participants
PR checklist
TaskId: (https://github.com/SkappHQ/skapp/issues/[id])
Summary
How to test
Project Checklist
npm run formatnpm run check-lintOther
PR Checklist
ready-for-code-review)Additional Information