-
Notifications
You must be signed in to change notification settings - Fork 167
Calendar: Integrate real logs and tasks to render ACTIVE/OOO days #1373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 CodeRabbit
WalkthroughIntroduces a logs RTK Query slice, extends tasks API with an assignee query and response typing, updates the calendar’s user search to fetch/merge logs and tasks, augments calendar page logic to mark ACTIVE days and show link text, adds ACTIVE styling, and expands calendar utils to return a 3-tuple including task URLs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant SF as UserSearchField
participant Logs as logsApi.getLogs
participant Tasks as tasksApi.getTasksByAssignee
participant Cal as Calendar Page
U->>SF: Submit username + date range
SF->>Logs: fetch logs (format=feed, type filters, dev=true or provided)
SF->>Tasks: fetch tasks by assignee (size?, dev=true)
alt Both responses OK
SF->>SF: Normalize times, map logs→events (OOO/ACTIVE)
SF->>SF: Dedup + map tasks→range entries (title/url)
SF-->>Cal: onSearchTextSubmitted({ userId, data: merged })
else Error/No user
SF-->>Cal: onSearchTextSubmitted({ userId, data: [] })
end
sequenceDiagram
autonumber
participant Cal as Calendar Page
participant Utils as processData
note over Cal,Utils: Data processing for rendering
Cal->>Utils: processData(itemId, data)
Utils-->>Cal: [statusByDate, taskTitleByDate, taskUrlByDate]
Cal->>Cal: setTileClassName(date)\n- if taskTitleByDate[time] → "ACTIVE"\n- else → statusByDate[time] or default
Cal->>Cal: handleDayClick(date)\n- build message with title\n- append "(Link: URL)" if taskUrlByDate[time]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
| Ambiguous Date Format Type ▹ view | ||
| Unsafe Log Type String ▹ view | ||
| Inefficient query parameter construction ▹ view | ||
| Untyped LogEntry defeats type safety ▹ view | ||
| Non-specific return type ▹ view | ||
| Missing null check for tasks array ▹ view | ||
| Redundant date.getTime() calls in tile rendering ▹ view | ||
| Untyped Calendar Tile Properties ▹ view | ||
| Hardcoded CSS class name may not exist ▹ view | ||
| Unclear rationale for dual task fetching ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| src/app/services/logsApi.ts | ✅ |
| src/utils/userStatusCalendar.ts | ✅ |
| src/pages/calendar/index.tsx | ✅ |
| src/app/services/tasksApi.ts | ✅ |
| src/components/Calendar/UserSearchField.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.
| startDate?: number; // seconds since epoch | ||
| endDate?: number; // seconds since epoch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ambiguous Date Format Type 
Tell me more
What is the issue?
Using raw numbers for dates with only comments to indicate the format is error-prone and unclear.
Why this matters
Developers may misinterpret the date format or forget to convert to seconds, leading to confusion and potential bugs.
Suggested change ∙ Feature Preview
Create a more descriptive type alias:
type UnixTimestamp = number;
export type GetLogsQuery = {
username?: string;
startDate?: UnixTimestamp;
endDate?: UnixTimestamp;
// ...
};Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| username?: string; | ||
| startDate?: number; // seconds since epoch | ||
| endDate?: number; // seconds since epoch | ||
| type?: string; // comma-separated list of types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsafe Log Type String 
Tell me more
What is the issue?
Using string type for a comma-separated list of predefined types is not type-safe and relies on comments for clarity.
Why this matters
Without type safety, developers might pass invalid log types or format the string incorrectly.
Suggested change ∙ Feature Preview
Define an enum or union type for log types:
export type LogType = 'OOO' | 'ACTIVE' | 'OTHER'; // Add actual log types
export type GetLogsQuery = {
// ...
type?: LogType[];
// ...
};Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| const queryParams = new URLSearchParams(); | ||
| if (dev) queryParams.set('dev', 'true'); | ||
| if (format) queryParams.set('format', format); | ||
| if (type) queryParams.set('type', type); | ||
| if (username) queryParams.set('username', username); | ||
| if (startDate) queryParams.set('startDate', String(startDate)); | ||
| if (endDate) queryParams.set('endDate', String(endDate)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inefficient query parameter construction 
Tell me more
What is the issue?
Multiple individual URLSearchParams.set() calls create unnecessary overhead when building query strings.
Why this matters
Each set() call triggers internal operations and potential string manipulations. For APIs that may be called frequently, this creates cumulative performance overhead that could be avoided with batch construction.
Suggested change ∙ Feature Preview
Use URLSearchParams constructor with an object or build the params object first, then construct URLSearchParams once:
const params: Record<string, string> = {};
if (dev) params.dev = 'true';
if (format) params.format = format;
if (type) params.type = type;
if (username) params.username = username;
if (startDate) params.startDate = String(startDate);
if (endDate) params.endDate = String(endDate);
const queryParams = new URLSearchParams(params);Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| @@ -0,0 +1,50 @@ | |||
| import { api } from './api'; | |||
|
|
|||
| export type LogEntry = any; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Untyped LogEntry defeats type safety 
Tell me more
What is the issue?
The LogEntry type is defined as 'any', which eliminates type safety for log entries throughout the application.
Why this matters
Using 'any' defeats the purpose of TypeScript's type system, making it impossible to catch type-related errors at compile time and reducing code maintainability and developer experience.
Suggested change ∙ Feature Preview
Define a proper interface for LogEntry based on the actual log data structure. For example:
export interface LogEntry {
id: string;
timestamp: number;
type: string;
username: string;
// Add other expected properties based on actual log structure
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| export const processData = ( | ||
| itemId: string | null, | ||
| data: [] | ||
| ): [object, object] => { | ||
| ): [object, object, object] => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-specific return type 
Tell me more
What is the issue?
The return type [object, object, object] is too generic and loses type safety. The function actually returns specific record types.
Why this matters
Using generic object types masks the actual structure of the returned data, making it harder to use correctly and more prone to runtime errors.
Suggested change ∙ Feature Preview
type ProcessDataReturn = [Record<number, string>, Record<number, string>, Record<number, string>];
export const processData = (
itemId: string | null,
data: []
): ProcessDataReturnProvide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
src/app/services/tasksApi.ts
Outdated
| transformResponse: (response: TasksResponseType) => { | ||
| return { | ||
| message: response.message ?? '', | ||
| tasks: response.tasks, | ||
| } as TasksQueryResponse; | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing null check for tasks array 
Tell me more
What is the issue?
The transformResponse function doesn't handle the case where response.tasks is undefined or null, which could cause runtime errors when the tasks array is accessed.
Why this matters
If the API returns a response where tasks is undefined or null, accessing response.tasks directly will pass undefined/null to the consuming code, potentially causing crashes when array methods are called on it.
Suggested change ∙ Feature Preview
Add a null check and provide a default empty array:
transformResponse: (response: TasksResponseType) => {
return {
message: response.message ?? '',
tasks: response.tasks ?? [],
} as TasksQueryResponse;
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| const time = date.getTime(); | ||
| // If there is a task on this date, mark as ACTIVE (green) | ||
| if (processedData[1] && processedData[1][time]) return 'ACTIVE'; | ||
| // Otherwise fall back to status classes like OOO/IDLE | ||
| return processedData[0] ? processedData[0][time] : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant date.getTime() calls in tile rendering 
Tell me more
What is the issue?
The setTileClassName function calls date.getTime() for every tile rendered in the calendar, which is an unnecessary computation that gets repeated for each calendar tile on every render.
Why this matters
This creates O(n) time complexity for each calendar render where n is the number of visible tiles. For a monthly view with ~42 tiles, this means 42 redundant getTime() calls per render, which can impact performance during frequent re-renders or when users navigate between months.
Suggested change ∙ Feature Preview
Memoize the time calculation or pre-compute timestamps when processing data. Consider using useMemo to cache the time value or modify the data structure to use date strings as keys instead of timestamps to avoid repeated getTime() calls.
Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| const [message, setMessage]: any = useState(null); | ||
| const [loading, setLoading]: any = useState(false); | ||
|
|
||
| const setTileClassName = ({ activeStartDate, date, view }: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Untyped Calendar Tile Properties 
Tell me more
What is the issue?
The function parameters are typed as 'any' instead of using the proper Calendar tile properties type.
Why this matters
Without proper typing, it's unclear what properties are available in the tile object, and unused parameters (activeStartDate, view) are not flagged.
Suggested change ∙ Feature Preview
import { CalendarTileProperties } from 'react-calendar';
const setTileClassName = ({ date }: CalendarTileProperties) => {Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| return processedData[0] ? processedData[0][date.getTime()] : null; | ||
| const time = date.getTime(); | ||
| // If there is a task on this date, mark as ACTIVE (green) | ||
| if (processedData[1] && processedData[1][time]) return 'ACTIVE'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded CSS class name may not exist 
Tell me more
What is the issue?
The setTileClassName function returns 'ACTIVE' as a CSS class name when tasks exist, but this hardcoded string may not correspond to actual CSS classes defined in the stylesheet.
Why this matters
If the CSS class 'ACTIVE' doesn't exist or is incorrectly styled, the calendar tiles won't display the intended visual styling for active days, leading to inconsistent or broken UI appearance.
Suggested change ∙ Feature Preview
Verify that the 'ACTIVE' CSS class is properly defined in the stylesheet, or use a constant/enum to ensure consistency between the returned class name and actual CSS definitions:
const CSS_CLASSES = {
ACTIVE: 'active',
OOO: 'OOO',
IDLE: 'IDLE',
SUNDAY: 'sunday'
} as const;
// Then use:
if (processedData[1] && processedData[1][time]) return CSS_CLASSES.ACTIVE;Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| return; | ||
| } | ||
|
|
||
| // Fetch logs (events) and tasks (continuous ranges). Try tasks by both userId and username. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unclear rationale for dual task fetching 
Tell me more
What is the issue?
The inline comment doesn't explain why we need to fetch tasks by both userId and username.
Why this matters
Future developers might remove this seemingly redundant double fetch without understanding its necessity.
Suggested change ∙ Feature Preview
// Fetch logs (events) and tasks (continuous ranges). Tasks are fetched by both userId and username
// because legacy data might be linked to either identifier
Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/utils/userStatusCalendar.ts (1)
1-4:LOG_TYPE.datais incorrectly typed as an empty tuple.This makes
log.dataunusable under strict settings. Type it asLOG_DATA[].interface LOG_TYPE { userId: string; - data: []; + data: LOG_DATA[]; }src/app/services/tasksApi.ts (2)
57-66: Don’t cast away missing fields; includemessageingetAllTasksresponse.Current transform drops
messagebut assertsTasksResponseType.transformResponse: (response: TasksResponseType) => { return { + message: response.message, tasks: response.tasks?.sort( (a: task, b: task) => +a.endsOn - +b.endsOn ), next: response.next, prev: response.prev, - } as TasksResponseType; + } as TasksResponseType; },
96-107: Tag mismatch: queries provide only type, mutations invalidate type+id.
invalidatesTagswith{ type:'Tasks', id }won’t refetch queries that provide only'Tasks'. Add broad invalidation.updateSelfTask: builder.mutation<void, TaskRequestPayload>({ ... - invalidatesTags: (_result, _err, { id }) => [ - { - type: 'Tasks', - id, - }, - ], + invalidatesTags: (_result, _err, { id }) => [ + { type: 'Tasks', id }, + 'Tasks', + ], ... updateTask: builder.mutation<void, TaskRequestPayload>({ ... - invalidatesTags: (_result, _err, { id }) => [ - { - type: 'Tasks', - id, - }, - ], + invalidatesTags: (_result, _err, { id }) => [ + { type: 'Tasks', id }, + 'Tasks', + ],Also applies to: 116-122
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/app/services/logsApi.ts(1 hunks)src/app/services/tasksApi.ts(3 hunks)src/components/Calendar/UserSearchField.tsx(3 hunks)src/pages/calendar/index.tsx(2 hunks)src/styles/calendar.scss(1 hunks)src/utils/userStatusCalendar.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/pages/calendar/index.tsx (1)
src/constants/calendar.ts (1)
MONTHS(54-67)
src/app/services/logsApi.ts (1)
src/app/services/api.ts (1)
api(9-37)
src/components/Calendar/UserSearchField.tsx (1)
src/interfaces/user.type.ts (1)
userDataType(1-29)
src/app/services/tasksApi.ts (2)
src/interfaces/task.type.ts (1)
TasksResponseType(38-43)__mocks__/handlers/users.handler.js (1)
size(10-10)
⏰ 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 (4)
src/pages/calendar/index.tsx (1)
21-27: Precedence: Sunday vs ACTIVE.You return 'sunday' before checking ACTIVE, so Sundays never show ACTIVE. Confirm product intent.
src/app/services/logsApi.ts (3)
21-48: RTK Query slice structure looks solid.Endpoint wiring and hook export are correct; tag type exists in api.ts.
29-31: Defaulting dev=true: confirm intended for production.If not, gate via env or pass explicitly from callers.
11-19: Confirm epoch units (seconds vs milliseconds) across callers. GetLogsQuery in src/app/services/logsApi.ts (lines 11–19) documents startDate/endDate as "seconds since epoch"; repo search returned no usages of GetLogsQuery/useLazyGetLogsQuery — confirm the backend expects seconds and ensure any JS Date.getTime() values are divided by 1000 (or convert in the client / clarify the type).
| @@ -0,0 +1,50 @@ | |||
| import { api } from './api'; | |||
|
|
|||
| export type LogEntry = any; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Replace any with a safer type.
Avoid any to keep type-safety.
Apply this diff:
-export type LogEntry = any;
+export type LogEntry = unknown; // TODO: narrow to the server schema📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type LogEntry = any; | |
| export type LogEntry = unknown; // TODO: narrow to the server schema |
🤖 Prompt for AI Agents
In src/app/services/logsApi.ts around line 3 replace the unsafe "any" with a
concrete LogEntry type: define and export an interface/alias that captures the
expected shape (e.g. timestamp as string|number|Date, level as a union of log
levels or string, message as string, and an optional metadata/fields property
typed as Record<string, unknown>), or if the shape is truly dynamic use
Record<string, unknown> instead of any; update all usages to accept that type
and narrow/validate fields where needed.
| username?: string; | ||
| startDate?: number; // seconds since epoch | ||
| endDate?: number; // seconds since epoch | ||
| type?: string; // comma-separated list of types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Support multiple types without manual CSV handling at call sites.
Accept string[] and join here.
Apply this diff:
- type?: string; // comma-separated list of types
+ type?: string | string[]; // accepts array; joined as CSV- if (type) queryParams.set('type', type);
+ if (type) queryParams.set('type', Array.isArray(type) ? type.join(',') : type);Also applies to: 39-39
🤖 Prompt for AI Agents
In src/app/services/logsApi.ts around lines 15 and 39, the type field is
currently declared as a string (expected to be a comma-separated list) which
forces callers to manually CSV their arrays; update the declaration to accept
string | string[] and, where the parameter is sent (line ~39), detect if the
value is an array and join it with commas before sending (otherwise leave the
string as-is), ensuring callers can pass either a string or an array without
extra handling.
| if (nextLink) { | ||
| return nextLink; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nextLink passthrough can fetch cross‑origin URLs. Constrain to same‑origin.
Returning arbitrary nextLink can bypass baseUrl and hit external origins (CORS/observability risk).
Apply this diff:
- if (nextLink) {
- return nextLink;
- }
+ if (nextLink) {
+ // Allow only same-origin relative URLs; strip origin if absolute.
+ if (/^https?:\/\//i.test(nextLink)) {
+ const u = new URL(nextLink);
+ return `${u.pathname}${u.search}`;
+ }
+ return nextLink.startsWith('/') ? nextLink : `/${nextLink}`;
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/services/logsApi.ts around lines 33 to 35, returning nextLink
directly allows cross-origin URLs; instead parse/resolve nextLink against the
configured baseUrl and only return it if its origin matches baseUrl.origin (and
optionally its pathname starts with baseUrl.pathname/base path); otherwise
ignore or return null/undefined. Validate using a URL constructor (with baseUrl
as base) or compare origins, and ensure any returned nextLink is same-origin to
avoid CORS/observability leaks.
| const queryParams = new URLSearchParams(); | ||
| if (dev) queryParams.set('dev', 'true'); | ||
| if (format) queryParams.set('format', format); | ||
| if (type) queryParams.set('type', type); | ||
| if (username) queryParams.set('username', username); | ||
| if (startDate) queryParams.set('startDate', String(startDate)); | ||
| if (endDate) queryParams.set('endDate', String(endDate)); | ||
| return `/logs?${queryParams.toString()}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Falsy checks drop 0 timestamps. Use explicit number checks.
0 (Unix epoch) is valid but currently omitted.
Apply this diff:
- if (dev) queryParams.set('dev', 'true');
+ if (dev) queryParams.set('dev', 'true');
if (format) queryParams.set('format', format);
- if (type) queryParams.set('type', type);
+ if (type) queryParams.set('type', type);
if (username) queryParams.set('username', username);
- if (startDate) queryParams.set('startDate', String(startDate));
- if (endDate) queryParams.set('endDate', String(endDate));
+ if (typeof startDate === 'number') queryParams.set('startDate', String(startDate));
+ if (typeof endDate === 'number') queryParams.set('endDate', String(endDate));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const queryParams = new URLSearchParams(); | |
| if (dev) queryParams.set('dev', 'true'); | |
| if (format) queryParams.set('format', format); | |
| if (type) queryParams.set('type', type); | |
| if (username) queryParams.set('username', username); | |
| if (startDate) queryParams.set('startDate', String(startDate)); | |
| if (endDate) queryParams.set('endDate', String(endDate)); | |
| return `/logs?${queryParams.toString()}`; | |
| const queryParams = new URLSearchParams(); | |
| if (dev) queryParams.set('dev', 'true'); | |
| if (format) queryParams.set('format', format); | |
| if (type) queryParams.set('type', type); | |
| if (username) queryParams.set('username', username); | |
| if (typeof startDate === 'number') queryParams.set('startDate', String(startDate)); | |
| if (typeof endDate === 'number') queryParams.set('endDate', String(endDate)); | |
| return `/logs?${queryParams.toString()}`; |
🤖 Prompt for AI Agents
In src/app/services/logsApi.ts around lines 36 to 43, the current falsy checks
for startDate and endDate drop valid numeric timestamps like 0; change the
guards to explicit presence checks (e.g. startDate !== undefined && startDate
!== null) or a typeof/Number.isFinite check so that numeric 0 is included, then
set queryParams.set('startDate', String(startDate)) and similarly for endDate
only when the explicit check passes.
| return `/logs?${queryParams.toString()}`; | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Avoid trailing ? when no params.
Minor cleanliness improvement for the request URL.
Apply this diff:
- return `/logs?${queryParams.toString()}`;
+ const search = queryParams.toString();
+ return search ? `/logs?${search}` : '/logs';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return `/logs?${queryParams.toString()}`; | |
| }, | |
| const search = queryParams.toString(); | |
| return search ? `/logs?${search}` : '/logs'; | |
| }, |
🤖 Prompt for AI Agents
In src/app/services/logsApi.ts around lines 43-44, the function always appends a
'?' even when queryParams.toString() is empty; change the return to
conditionally append the query string: compute const qs = queryParams.toString()
and return qs ? `/logs?${qs}` : `/logs` so no trailing '?' appears when there
are no params.
| const title = processedData[1][value.getTime()]; | ||
| const url = processedData[2] | ||
| ? processedData[2][value.getTime()] | ||
| : undefined; | ||
| const linkText = url ? ` (Link: ${url})` : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same normalization on click handler.
- const title = processedData[1][value.getTime()];
- const url = processedData[2]
- ? processedData[2][value.getTime()]
+ const key = getStartOfDay(value).getTime();
+ const title = processedData[1][key];
+ const url = processedData[2]
+ ? processedData[2][key]
: undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const title = processedData[1][value.getTime()]; | |
| const url = processedData[2] | |
| ? processedData[2][value.getTime()] | |
| : undefined; | |
| const linkText = url ? ` (Link: ${url})` : ''; | |
| const key = getStartOfDay(value).getTime(); | |
| const title = processedData[1][key]; | |
| const url = processedData[2] | |
| ? processedData[2][key] | |
| : undefined; | |
| const linkText = url ? ` (Link: ${url})` : ''; |
🤖 Prompt for AI Agents
In src/pages/calendar/index.tsx around lines 55 to 59, the code uses
value.getTime() to index processedData but the click handler elsewhere uses a
normalized date (zeroed time) — apply the same normalization before calling
getTime() so both reads use the identical key. Compute the normalized date from
value (using the same normalize/zero-time function or logic used in the click
handler) and use normalizedDate.getTime() when accessing processedData[1],
processedData[2] and when building linkText.
| .react-calendar .ACTIVE { | ||
| background: #22c55e; /* green-500 */ | ||
| color: #ffffff; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
🧩 Analysis chain
A11y + specificity: ACTIVE contrast likely fails AA; also increase selector specificity.
White on green-500 (#22c55e) is marginal for normal text, and .react-calendar .ACTIVE may lose to library tile styles. Use a darker green and target the tile element.
-.react-calendar .ACTIVE {
- background: #22c55e; /* green-500 */
- color: #ffffff;
-}
+.react-calendar .react-calendar__tile.ACTIVE {
+ background: #16a34a; /* green-600 */
+ color: #ffffff;
+}
+/* Keyboard focus */
+.react-calendar .react-calendar__tile.ACTIVE:focus-visible {
+ outline: 2px solid #111827;
+ outline-offset: 2px;
+}Run to verify contrast ratio:
🏁 Script executed:
#!/bin/bash
python - <<'PY'
def rel(c):
c=[int(c[i:i+2],16)/255 for i in (1,3,5)]
def lin(u): return u/12.92 if u<=0.03928 else ((u+0.055)/1.055)**2.4
r,g,b=[lin(u) for u in c]
return 0.2126*r+0.7152*g+0.0722*b
def ratio(fg,bg):
L1=max(rel(fg),rel(bg)); L2=min(rel(fg),rel(bg))
return (L1+0.05)/(L2+0.05)
print("contrast(#ffffff on #22c55e) =", ratio("ffffff","22c55e"))
print("contrast(#ffffff on #16a34a) =", ratio("ffffff","16a34a"))
PYLength of output: 97
Contrast OK (8.11:1); increase selector specificity and add keyboard focus
White (#ffffff) on #22c55e = 8.11:1 (passes WCAG AA and AAA). Target the tile element to avoid being overridden (e.g. .react-calendar .react-calendar__tile.ACTIVE) and add a :focus-visible outline for keyboard users in src/styles/calendar.scss (≈ lines 16–19).
🤖 Prompt for AI Agents
In src/styles/calendar.scss around lines 16 to 19, the .react-calendar .ACTIVE
rule is too generic and lacks a keyboard focus style; update the selector to
target the tile element (e.g. .react-calendar .react-calendar__tile.ACTIVE) to
increase specificity so it won't be overridden, and add a matching
:focus-visible rule (e.g. .react-calendar .react-calendar__tile:focus-visible or
.react-calendar .react-calendar__tile.ACTIVE:focus-visible) that defines a
visible outline or ring for keyboard users; ensure the color/background values
remain the same and the focus outline contrasts and is clearly visible.
| ): [object, object, object] => { | ||
| if (!itemId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Type the API surface precisely (no []/object).
Use concrete types for inputs and return to prevent misuse and enable tooling.
-export const processData = (
- itemId: string | null,
- data: []
-): [object, object, object] => {
+type StatusMap = Record<number, string>;
+type TaskTitleMap = Record<number, string>;
+type TaskUrlMap = Record<number, string>;
+
+export const processData = (
+ itemId: string | null,
+ data: LOG_TYPE[]
+): [StatusMap, TaskTitleMap, TaskUrlMap] => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ): [object, object, object] => { | |
| if (!itemId) { | |
| type StatusMap = Record<number, string>; | |
| type TaskTitleMap = Record<number, string>; | |
| type TaskUrlMap = Record<number, string>; | |
| export const processData = ( | |
| itemId: string | null, | |
| data: LOG_TYPE[] | |
| ): [StatusMap, TaskTitleMap, TaskUrlMap] => { | |
| if (!itemId) { |
🤖 Prompt for AI Agents
In src/utils/userStatusCalendar.ts around lines 45-46, the function signature
currently returns a vague [object, object, object]; replace that with a precise
tuple of concrete types (e.g., [EventSummary, UserAvailability,
CalendarMetadata]) or whatever three specific interfaces match each returned
value, and explicitly type the function parameters (e.g., itemId: string |
undefined, userId: string). Add or import the corresponding interfaces/types
near the top of the file, update the return statements to ensure they conform to
the new types, and adjust any callers if needed.
| return log.userId === itemId; | ||
| }); | ||
| if (!log || log.data?.length == 0) return [{}, {}]; | ||
| if (!log || log.data?.length == 0) return [{}, {}, {}]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Use strict equality.
- if (!log || log.data?.length == 0) return [{}, {}, {}];
+ if (!log || log.data?.length === 0) return [{}, {}, {}];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!log || log.data?.length == 0) return [{}, {}, {}]; | |
| if (!log || log.data?.length === 0) return [{}, {}, {}]; |
🤖 Prompt for AI Agents
In src/utils/userStatusCalendar.ts around line 52, the conditional uses loose
equality (==) when checking log.data?.length; replace it with strict equality
(===) so the line reads to check for length === 0, i.e., use if (!log ||
log.data?.length === 0) return [{}, {}, {}]; to ensure type-safe comparison.
| if (logData.taskTitle) { | ||
| dictWithTask[dateTimestamp] = logData.taskTitle; | ||
| } | ||
| if (logData.taskUrl) { | ||
| dictWithTaskUrl[dateTimestamp] = logData.taskUrl; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Optionally also record 'ACTIVE' in status map for completeness.
If other consumers read only dictWithStatus, they’ll miss ACTIVE days.
if (logData.status === 'ACTIVE') {
dates.forEach((dateTimestamp) => {
+ dictWithStatus[dateTimestamp] = 'ACTIVE';
if (logData.taskTitle) {
dictWithTask[dateTimestamp] = logData.taskTitle;
}
if (logData.taskUrl) {
dictWithTaskUrl[dateTimestamp] = logData.taskUrl;
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (logData.taskTitle) { | |
| dictWithTask[dateTimestamp] = logData.taskTitle; | |
| } | |
| if (logData.taskUrl) { | |
| dictWithTaskUrl[dateTimestamp] = logData.taskUrl; | |
| } | |
| }); | |
| if (logData.status === 'ACTIVE') { | |
| dates.forEach((dateTimestamp) => { | |
| dictWithStatus[dateTimestamp] = 'ACTIVE'; | |
| if (logData.taskTitle) { | |
| dictWithTask[dateTimestamp] = logData.taskTitle; | |
| } | |
| if (logData.taskUrl) { | |
| dictWithTaskUrl[dateTimestamp] = logData.taskUrl; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/utils/userStatusCalendar.ts around lines 63 to 69, when populating
dictWithTask and dictWithTaskUrl for a given dateTimestamp, also mark
dictWithStatus[dateTimestamp] = 'ACTIVE' so consumers that only read
dictWithStatus see those days; implement this assignment alongside the existing
task/title/url assignments and optionally guard it to avoid overwriting a more
specific status if one already exists for that date.
Date:
Developer Name:
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Integrate real logs and tasks data to render ACTIVE/OOO days in the calendar by enhancing APIs, wiring data fetching in the user search flow, and updating calendar rendering and data processing to support task titles and URLs.
Why are these changes being made?
To render accurate user activity by combining log events (OOO/other statuses) with task activity, and to surface task titles and links on the calendar UI for better context. This approach replaces placeholder/mock data with real data and extends the data model accordingly.