feature : Show OOO users data on Calendar #1378
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbitRelease Notes
WalkthroughThis PR introduces Out-of-Office (OOO) logs integration to the calendar system by adding a new logs API slice, updating components to fetch and display OOO data, and enhancing the calendar UI to show OOO status alongside existing user status information. Test files are updated to use Jest fake timers for consistent time-relative assertions. Changes
Sequence DiagramsequenceDiagram
participant User
participant Calendar as Calendar Page
participant SearchField as UserSearchField
participant LogsAPI as Logs API
participant Utils as userStatusCalendar Utils
User->>Calendar: Selects user (with dev=true query param)
Calendar->>SearchField: Passes dev=true prop
SearchField->>LogsAPI: useGetLogsByUsernameQuery(username, dev, format, type)
LogsAPI-->>SearchField: LogsResponse with entries
SearchField->>SearchField: Triggers onSearchTextSubmitted
SearchField->>Calendar: Passes user, data, oooLogsData
Calendar->>Utils: processData(userId, data, oooLogsData)
Utils->>Utils: processOOOLogsData() builds OOO entries map
Utils-->>Calendar: [statusDict, taskDict, oooEntriesDict]
Calendar->>Calendar: Updates tile styling with OOO status
Calendar-->>User: Displays OOO details on day click
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Missing Type Enum for Log Types ▹ view | |
 |
Undefined Format Type Constant ▹ view | |
|
Unsafe URL Parameter Construction ▹ view | |
|
Undefined Log Type Constant ▹ view | |
|
Move constant array outside function ▹ view | ✅ Fix detected |
 |
Dev mode enabled by default ▹ view | ✅ Fix detected |
 |
Inefficient array recreation in formatTimestampToDate ▹ view | ✅ Fix detected |
|
Unvalidated query string parameters ▹ view | |
|
Missing dependency in useEffect causes stale closure ▹ view | ✅ Fix detected |
|
Unnecessary API calls on component re-renders ▹ view | ✅ Fix detected |
Files scanned
| File Path | Reviewed |
|---|---|
| src/app/services/api.ts | ✅ |
| src/utils/time.ts | ✅ |
| src/app/services/logsApi.ts | ✅ |
| src/constants/url.ts | ✅ |
| src/utils/userStatusCalendar.ts | ✅ |
| src/components/Calendar/UserSearchField.tsx | ✅ |
| src/pages/calendar/index.tsx | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
__tests__/Unit/Components/Tasks/Card.test.tsx (1)
541-557: Avoid mutatingmoment.prototype; use a spy and restore safelyDirectly overriding the prototype can leak if the test fails. Spy instead and restore in finally.
- const originalFromNow = moment.prototype.fromNow; - moment.prototype.fromNow = jest.fn(() => '4 years ago'); + const spy = jest.spyOn(moment.fn as any, 'fromNow').mockReturnValue('4 years ago'); const { getByTestId } = renderWithRouter( ... ); const spanElement = screen.getByTestId('started-on'); - expect(spanElement).toHaveTextContent('Started 4 years ago'); - moment.prototype.fromNow = originalFromNow; + expect(spanElement).toHaveTextContent('Started 4 years ago'); + spy.mockRestore();If you prefer time control over mocking, we can switch this suite to
jest.setSystemTime(...)like in the other test for consistency.
♻️ Duplicate comments (5)
src/utils/time.ts (1)
29-42: Move months array to module level.The months array is recreated on every function call, adding unnecessary overhead. This concern has already been raised in previous reviews.
src/app/services/logsApi.ts (2)
30-36: Use URLSearchParams for safe URL construction.Template literal concatenation doesn't encode special characters. If
usernamecontains characters like&,=, or spaces, the URL will be malformed or could be exploited.Apply this fix:
- query: ({ - username, - dev = false, - format = 'feed', - type = 'REQUEST_CREATED', - }) => - `/logs?dev=${dev}&format=${format}&type=${type}&username=${username}`, + query: ({ + username, + dev = false, + format = 'feed', + type = 'REQUEST_CREATED', + }) => { + const params = new URLSearchParams({ + dev: String(dev), + format, + type, + username, + }); + return `/logs?${params.toString()}`; + },
33-34: Define constants or enums for format and type values.Magic strings like
'feed'and'REQUEST_CREATED'reduce maintainability and type safety. This has been previously flagged.src/components/Calendar/UserSearchField.tsx (2)
78-88: Add missing dependencies to useEffect.The effect uses
dataandonSearchTextSubmittedbut they're not in the dependency array, risking stale closures. This was flagged previously.Apply this fix:
- }, [dev, logsData, selectedUser]); + }, [dev, logsData, selectedUser, data, onSearchTextSubmitted]);
55-58: Memoize query parameters to prevent unnecessary re-execution.The query parameters object is recreated on every render. While RTK Query has caching, this can still cause unnecessary work. This has been previously noted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
__tests__/Unit/Components/ExtensionRequest/ExtensionStatusModal.test.tsx(1 hunks)__tests__/Unit/Components/Tasks/Card.test.tsx(3 hunks)src/app/services/api.ts(1 hunks)src/app/services/logsApi.ts(1 hunks)src/components/Calendar/UserSearchField.tsx(4 hunks)src/constants/url.ts(1 hunks)src/pages/calendar/index.tsx(4 hunks)src/styles/calendar.scss(1 hunks)src/utils/time.ts(1 hunks)src/utils/userStatusCalendar.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/app/services/logsApi.ts (1)
src/app/services/api.ts (1)
api(9-38)
src/utils/userStatusCalendar.ts (1)
src/app/services/logsApi.ts (1)
LogEntry(3-11)
__tests__/Unit/Components/ExtensionRequest/ExtensionStatusModal.test.tsx (1)
src/components/ExtensionRequest/ExtensionStatusModal.tsx (1)
formatToRelativeTime(24-27)
src/components/Calendar/UserSearchField.tsx (3)
src/interfaces/user.type.ts (1)
userDataType(1-29)src/app/services/logsApi.ts (1)
LogEntry(3-11)src/hooks/useOutsideAlerter.ts (1)
useOutsideAlerter(3-19)
src/pages/calendar/index.tsx (6)
__tests__/Unit/Components/Tasks/TaskDetails.test.tsx (1)
useRouter(706-713)src/interfaces/user.type.ts (1)
userDataType(1-29)src/utils/userStatusCalendar.ts (2)
OOOEntry(15-23)processData(76-121)src/utils/time.ts (1)
formatTimestampToDate(27-47)src/constants/url.ts (1)
OOO_REQUEST_DETAILS_URL(37-37)src/constants/calendar.ts (1)
MONTHS(54-67)
🪛 ast-grep (0.39.6)
src/pages/calendar/index.tsx
[warning] 91-91: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/pages/calendar/index.tsx
[error] 92-92: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (5)
src/app/services/api.ts (1)
31-32: Tag type addition looks good; tag usage verified in logs API sliceLGTM. Verified that the logs slice correctly uses
providesTags: ['Logs']to benefit from caching with the new tag type.src/utils/userStatusCalendar.ts (1)
53-75: Confirm timestamp units in logsApi response (seconds vs milliseconds)The review comment raises two points:
✓ Unused second return value is confirmed:
dictWithTaskis created but never populated, and at line 111 only[oooEntries]is destructured from the tuple, leaving the second element wasted. Removing it simplifies the API.? Timestamp unit normalization requires verification: The
LogEntry.fromandLogEntry.untilfields are typed asnumberbut their units (seconds vs milliseconds) are not documented in the codebase. The API integration code doesn't reveal the actual units returned by the backend. The suggested defensive check (logEntry.from < 1e12 ? logEntry.from * 1000 : logEntry.from) assumes a potential mismatch, but this needs confirmation.The refactoring to remove the unused return value should proceed. However, before applying the timestamp normalization logic, please verify with the backend API documentation or team whether
from/untilare indeed in seconds or milliseconds.src/constants/url.ts (1)
37-37: LGTM! Clean constant addition.The OOO_REQUEST_DETAILS_URL constant follows the established pattern and integrates well with the existing URL constants.
src/pages/calendar/index.tsx (2)
15-17: LGTM! Clean dev mode detection.Using the router query parameter to enable dev mode is appropriate and follows the pattern seen elsewhere in the codebase.
163-167: LGTM! Safe type handling.The type guard ensures
onDateChangeonly receives a valid Date instance, preventing potential runtime errors.
__tests__/Unit/Components/ExtensionRequest/ExtensionStatusModal.test.tsx
Show resolved
Hide resolved
| @@ -0,0 +1,42 @@ | |||
| import { api } from './api'; | |||
|
|
|||
| export interface LogEntry { | |||
There was a problem hiding this comment.
- any specific reason we are using interface over type
| const [displayList, setDisplayList] = useState<userDataType[]>([]); | ||
| const [data, setData] = useState([]); | ||
| const [data, setData] = useState< | ||
| Array<{ userId: string; data: LOG_DATA[] }> |
There was a problem hiding this comment.
define it type please
- type UserLogData = Array<{ userId: string; data: LOG_DATA[] }>;
| useEffect(() => { | ||
| if (userData?.users) { | ||
| const users: userDataType[] = userData.users; | ||
| const filteredUsers: userDataType[] = users.filter( | ||
| (user: userDataType) => !user.incompleteUserDetails | ||
| ); | ||
| const logData: any = filteredUsers.map((user: userDataType) => { | ||
| const log = logs[Math.floor(Math.random() * 4)]; | ||
| return { | ||
| data: log, | ||
| userId: user.id, | ||
| }; | ||
| }); | ||
| const logData: Array<{ userId: string; data: LOG_DATA[] }> = | ||
| filteredUsers.map((user: userDataType) => { | ||
| const log = logs[Math.floor(Math.random() * 4)]; | ||
| return { | ||
| data: log, | ||
| userId: user.id, | ||
| }; | ||
| }); | ||
| setData(logData); | ||
| setUsersList(filteredUsers); | ||
| } | ||
| }, [isLoading, userData]); |
There was a problem hiding this comment.
Looking at this file, it seems that we’re currently fetching all users from the database and then filtering them locally. Instead of doing that, I’d suggest using the user search API to fetch users directly based on their type.
For example, if we have 200 active users in the database but the getAllUsers only returns 100 users due to pagination, we might never find the intended user. Using the search API will make this process more efficient and reliable.
| const log = logs[Math.floor(Math.random() * 4)]; | ||
| return { |
There was a problem hiding this comment.
also what is this math.random doing here
There was a problem hiding this comment.
- this search field doing so much ideally we should just search the user as the name suggested and pass to parent component
|
|
||
| const MONTHS = [ | ||
| 'Jan', | ||
| 'Feb', | ||
| 'Mar', | ||
| 'Apr', | ||
| 'May', | ||
| 'Jun', | ||
| 'Jul', | ||
| 'Aug', | ||
| 'Sep', | ||
| 'Oct', | ||
| 'Nov', | ||
| 'Dec', | ||
| ]; | ||
|
|
||
| export const formatTimestampToDate = (timestamp: number): string => { | ||
| const date = new Date(timestamp); | ||
| return `${ | ||
| MONTHS[date.getMonth()] | ||
| } ${date.getDate()}, ${date.getFullYear()}`; | ||
| }; |
There was a problem hiding this comment.
- since we already have moment then we can just do this
export const formatTimestampToDate = (timestamp: number): string => {
return moment(timestamp).format('MMM D, YYYY');
};
|
|
||
| const [message, setMessage]: any = useState(null); | ||
| const [loading, setLoading]: any = useState(false); | ||
| const [selectedDate, onDateChange] = useState<Date>(new Date()); |
There was a problem hiding this comment.
- follow React convention for naming state
| if (event.currentTarget.classList.contains('OOO')) { | ||
| setMessage( | ||
| `${selectedUser.username} is OOO on ${value.getDate()}-${ | ||
| `${selectedUser?.username} is OOO on ${value.getDate()}-${ |
There was a problem hiding this comment.
- this piece of code
${value.getDate()}-${MONTHS[value.getMonth()]}-${value.getFullYear()}is used in around 3 4 places in this file , can we please make a small util function and reuse it
| if (date.getDay() === 0) return 'sunday'; | ||
| return processedData[0] ? processedData[0][date.getTime()] : null; | ||
|
|
||
| if (processedData[2].has(date.getTime())) { |
There was a problem hiding this comment.
where is this processedData[2] , like what is the point of creating map
|
|
||
| const oooEntries = processedData[2].get(value.getTime()); | ||
| if (oooEntries) { | ||
| const message = formatOOODayMessage( |
There was a problem hiding this comment.
- nit use different name for variable, we already have a state variable with name message
| import { LogEntry } from '@/app/services/logsApi'; | ||
|
|
||
| interface LOG_DATA { | ||
| export interface LOG_DATA { |
There was a problem hiding this comment.
- can we use type if possible
| logsData: LogEntry[] | ||
| ): [Map<number, OOOEntry[]>, Map<number, string>] => { | ||
| const dictWithOOOEntries = new Map<number, OOOEntry[]>(); | ||
| const dictWithTask = new Map<number, string>(); |
There was a problem hiding this comment.
- we just define this variable and return it but never assign it something is it intentional
| if (!itemId) { | ||
| return [{}, {}]; | ||
| return [new Map(), new Map(), new Map()]; | ||
| } else { |
There was a problem hiding this comment.
- I think we can remove else condition as we are doing early return in the if case
| oooEntries.forEach((entries, dateTimestamp) => { | ||
| dictWithOOOEntries.set(dateTimestamp, entries); | ||
| }); | ||
|
|
||
| oooEntries.forEach((_, dateTimestamp) => { | ||
| dictWithStatus.set(dateTimestamp, 'OOO'); | ||
| }); |
There was a problem hiding this comment.
can we combine both in one loop ?
Date: 30-10-2025
Developer Name: Rishi Chaubey
Issue Ticket Number
Description
Currently, our dashboard site has an Activity Feed powered by the Logs API that tracks and displays user activities.
On the status site, we already have a calendar component with a user search option (by username), but it is non-functional.
The goal of this task is to integrate the relevant APIs with the calendar so that when a username is entered, the calendar dynamically displays the user’s activities in a structured and visually intuitive way.
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Screen.Recording.2025-10-30.195817.mp4
Test Coverage
Test will be in next pr updating the coverage soon !
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Implement feature to show active tasks on calendar by integrating logs fetching, building per-date activity mappings, and linking GitHub issues when available. Add a development flow that fetches task logs, derives active date ranges from task details, and displays links in calendar messages. Also enhance the calendar page to optionally use this dev flow when the dev query parameter is true.
Why are these changes being made?
To display active task periods directly on the calendar and provide quick access to related GitHub issues during development, while preserving existing behavior for non-dev usage. This enables a richer, interactive calendar experience with minimal disruption to the default flow.