-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -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 commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick Replace any with a safer type. Avoid Apply this diff: -export type LogEntry = any;
+export type LogEntry = unknown; // TODO: narrow to the server schema📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export type LogsResponse = { | ||||||||||||||||||||||||||||||||||
| data: LogEntry[]; | ||||||||||||||||||||||||||||||||||
| next?: string | null; | ||||||||||||||||||||||||||||||||||
| prev?: string | null; | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export type GetLogsQuery = { | ||||||||||||||||||||||||||||||||||
| username?: string; | ||||||||||||||||||||||||||||||||||
| startDate?: number; // seconds since epoch | ||||||||||||||||||||||||||||||||||
| endDate?: number; // seconds since epoch | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ambiguous Date Format Type
Tell me moreWhat is the issue?Using raw numbers for dates with only comments to indicate the format is error-prone and unclear. Why this mattersDevelopers may misinterpret the date format or forget to convert to seconds, leading to confusion and potential bugs. Suggested change ∙ Feature PreviewCreate 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. |
||||||||||||||||||||||||||||||||||
| type?: string; // comma-separated list of types | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unsafe Log Type String
Tell me moreWhat 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 mattersWithout type safety, developers might pass invalid log types or format the string incorrectly. Suggested change ∙ Feature PreviewDefine 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick Support multiple types without manual CSV handling at call sites. Accept 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 |
||||||||||||||||||||||||||||||||||
| format?: 'feed'; | ||||||||||||||||||||||||||||||||||
| dev?: boolean; | ||||||||||||||||||||||||||||||||||
| nextLink?: string; | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export const logsApi = api.injectEndpoints({ | ||||||||||||||||||||||||||||||||||
| endpoints: (build) => ({ | ||||||||||||||||||||||||||||||||||
| getLogs: build.query<LogsResponse, GetLogsQuery>({ | ||||||||||||||||||||||||||||||||||
| query: ({ | ||||||||||||||||||||||||||||||||||
| username, | ||||||||||||||||||||||||||||||||||
| startDate, | ||||||||||||||||||||||||||||||||||
| endDate, | ||||||||||||||||||||||||||||||||||
| type, | ||||||||||||||||||||||||||||||||||
| format = 'feed', | ||||||||||||||||||||||||||||||||||
| dev = true, | ||||||||||||||||||||||||||||||||||
| nextLink, | ||||||||||||||||||||||||||||||||||
| }) => { | ||||||||||||||||||||||||||||||||||
| if (nextLink) { | ||||||||||||||||||||||||||||||||||
| return nextLink; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+33
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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}`;
+ }
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| 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)); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+36
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inefficient query parameter construction
Tell me moreWhat is the issue?Multiple individual URLSearchParams.set() calls create unnecessary overhead when building query strings. Why this mattersEach 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 PreviewUse 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. |
||||||||||||||||||||||||||||||||||
| return `/logs?${queryParams.toString()}`; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+36
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Falsy checks drop 0 timestamps. Use explicit number checks.
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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| providesTags: ['Status'], | ||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick Use keyed tags for better cache invalidation granularity. Current static tag may over‑invalidate. Apply this diff: - providesTags: ['Status'],
+ providesTags: (result, error, arg) =>
+ arg?.username ? [{ type: 'Status' as const, id: arg.username }, 'Status'] : ['Status'],📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export const { useLazyGetLogsQuery } = logsApi; | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,10 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useState, useEffect, ChangeEvent, useRef } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import classNames from './UserSearchField.module.scss'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useGetAllUsersQuery } from '@/app/services/usersApi'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { logs } from '@/constants/calendar'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { userDataType } from '@/interfaces/user.type'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useOutsideAlerter } from '@/hooks/useOutsideAlerter'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useLazyGetLogsQuery } from '@/app/services/logsApi'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useLazyGetAllTasksQuery } from '@/app/services/tasksApi'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type SearchFieldProps = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onSearchTextSubmitted: (user: userDataType | undefined, data: any) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -22,37 +23,122 @@ const SearchField = ({ onSearchTextSubmitted, loading }: SearchFieldProps) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filterUser(e.target.value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleOnSubmit = (e: React.FormEvent) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { data: userData, isLoading: isUsersLoading } = useGetAllUsersQuery(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [usersList, setUsersList] = useState<userDataType[]>([]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [displayList, setDisplayList] = useState<userDataType[]>([]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [triggerGetLogs, { isFetching: isLogsFetching }] = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useLazyGetLogsQuery(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [triggerGetTasks, { isFetching: isTasksFetching }] = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useLazyGetAllTasksQuery(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const toMs = (value?: number | string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof value === 'string') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const parsed = Date.parse(value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return isNaN(parsed) ? undefined : parsed; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof value !== 'number') return undefined as unknown as number; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return value >= 1e12 ? value : value * 1000; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+35
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix Return - const toMs = (value?: number | string) => {
+ const toMs = (value?: number | string): number | undefined => {
if (typeof value === 'string') {
const parsed = Date.parse(value);
return isNaN(parsed) ? undefined : parsed;
}
- if (typeof value !== 'number') return undefined as unknown as number;
- return value >= 1e12 ? value : value * 1000;
+ if (typeof value !== 'number') return undefined;
+ return value >= 1e12 ? value : value * 1000;
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleOnSubmit = async (e: React.FormEvent) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| e.preventDefault(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setDisplayList([]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const user = usersList.find( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (user: userDataType) => user.username === searchText | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onSearchTextSubmitted(user, data); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { data: userData, isError, isLoading } = useGetAllUsersQuery(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [usersList, setUsersList] = useState<userDataType[]>([]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [displayList, setDisplayList] = useState<userDataType[]>([]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [data, setData] = useState([]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!user) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onSearchTextSubmitted(undefined, []); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 commentThe reason will be displayed to describe this comment to others. Learn more. Unclear rationale for dual task fetching
Tell me moreWhat is the issue?The inline comment doesn't explain why we need to fetch tasks by both userId and username. Why this mattersFuture 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 Provide feedback to improve future suggestions💬 Looking for more details? Reply to this comment to chat with Korbit. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [logsResult, tasksByIdResult, tasksByUsernameResult] = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await Promise.all([ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| triggerGetLogs({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| username: user.username || undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: ['task', 'REQUEST_CREATED'].join(','), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| format: 'feed', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dev: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| triggerGetTasks({ assignee: user.id }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| triggerGetTasks({ assignee: user.username || '' }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick Avoid querying tasks with empty assignee. Skip the username-based query when - triggerGetTasks({ assignee: user.username || '' }),
+ user.username
+ ? triggerGetTasks({ assignee: user.username })
+ : Promise.resolve({ data: { tasks: [] } as any }),📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const logsResponse = logsResult.data; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tasksById = tasksByIdResult.data?.tasks || []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tasksByUsername = tasksByUsernameResult.data?.tasks || []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Merge and de-duplicate tasks by id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const taskMap: Record<string, any> = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [...tasksById, ...tasksByUsername].forEach((t: any) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (t?.id) taskMap[t.id] = t; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const mergedTasks = Object.values(taskMap); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const eventEntries = (logsResponse?.data || []) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter((log: any) => !!log) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map((log: any) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // OOO ranges | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (log.type === 'REQUEST_CREATED' && log.from && log.until) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| startTime: toMs(log.from), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| endTime: toMs(log.until), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status: 'OOO', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Task events => single-day ACTIVE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (log.type === 'task') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const ts = toMs(log.timestamp); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| startTime: ts, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| endTime: ts, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status: 'ACTIVE', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| taskTitle: log.taskTitle, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter(Boolean); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+80
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick Filter out log entries with invalid timestamps. Prevents pushing - const eventEntries = (logsResponse?.data || [])
+ const eventEntries = (logsResponse?.data || [])
.filter((log: any) => !!log)
.map((log: any) => {
...
if (log.type === 'task') {
const ts = toMs(log.timestamp);
return {
startTime: ts,
endTime: ts,
status: 'ACTIVE',
taskTitle: log.taskTitle,
};
}
return null;
})
- .filter(Boolean);
+ .filter((e: any) => e && e.startTime);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const taskRangeEntries = mergedTasks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter((t: any) => t) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map((t: any) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const start = toMs(t.startedOn); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const end = toMs(t.endsOn); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const taskUrl = t.github?.issue?.html_url || undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| startTime: start, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| endTime: end ?? start, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status: 'ACTIVE', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| taskTitle: t.title, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| taskUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter((e: any) => e.startTime); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const calendarDataForUser = [...eventEntries, ...taskRangeEntries]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const mapped = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| userId: user.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| data: calendarDataForUser, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onSearchTextSubmitted(user, mapped); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setData(logData); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setUsersList(filteredUsers); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [isLoading, userData]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [isUsersLoading, userData]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isValidUsername = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const usernames = usersList.map((user: userDataType) => user.username); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -112,7 +198,11 @@ const SearchField = ({ onSearchTextSubmitted, loading }: SearchFieldProps) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <button | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className={classNames.userSearchSubmitButton} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| disabled={ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loading || !(searchText ?? '').trim() || !isValidUsername() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loading || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isLogsFetching || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isTasksFetching || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| !(searchText ?? '').trim() || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| !isValidUsername() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Submit | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,7 +19,11 @@ const UserStatusCalendar: FC = () => { | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const setTileClassName = ({ activeStartDate, date, view }: any) => { | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Untyped Calendar Tile Properties
Tell me moreWhat is the issue?The function parameters are typed as 'any' instead of using the proper Calendar tile properties type. Why this mattersWithout proper typing, it's unclear what properties are available in the tile object, and unused parameters (activeStartDate, view) are not flagged. Suggested change ∙ Feature Previewimport { 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. |
||||||||||||||||||||||||||
| if (date.getDay() === 0) return 'sunday'; | ||||||||||||||||||||||||||
| 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 commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded CSS class name may not exist
Tell me moreWhat 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 mattersIf 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 PreviewVerify 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. |
||||||||||||||||||||||||||
| // Otherwise fall back to status classes like OOO/IDLE | ||||||||||||||||||||||||||
| return processedData[0] ? processedData[0][time] : null; | ||||||||||||||||||||||||||
|
Comment on lines
+22
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant date.getTime() calls in tile rendering
Tell me moreWhat is the issue?The Why this mattersThis 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 Suggested change ∙ Feature PreviewMemoize the time calculation or pre-compute timestamps when processing data. Consider using Provide feedback to improve future suggestions💬 Looking for more details? Reply to this comment to chat with Korbit.
Comment on lines
+22
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Normalize keys to start-of-day to match
-import { processData } from '@/utils/userStatusCalendar';
+import { processData, getStartOfDay } from '@/utils/userStatusCalendar';
...
- const time = date.getTime();
+ const time = getStartOfDay(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;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const handleDayClick = (value: Date, event: any) => { | ||||||||||||||||||||||||||
|
|
@@ -48,12 +52,15 @@ const UserStatusCalendar: FC = () => { | |||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (processedData[1] && processedData[1][value.getTime()]) { | ||||||||||||||||||||||||||
| const title = processedData[1][value.getTime()]; | ||||||||||||||||||||||||||
| const url = processedData[2] | ||||||||||||||||||||||||||
| ? processedData[2][value.getTime()] | ||||||||||||||||||||||||||
| : undefined; | ||||||||||||||||||||||||||
| const linkText = url ? ` (Link: ${url})` : ''; | ||||||||||||||||||||||||||
|
Comment on lines
+55
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| setMessage( | ||||||||||||||||||||||||||
| `${selectedUser.username} is ACTIVE on ${value.getDate()}-${ | ||||||||||||||||||||||||||
| MONTHS[value.getMonth()] | ||||||||||||||||||||||||||
| }-${value.getFullYear()} having task with title - ${ | ||||||||||||||||||||||||||
| processedData[1][value.getTime()] | ||||||||||||||||||||||||||
| }` | ||||||||||||||||||||||||||
| }-${value.getFullYear()} having task with title - ${title}${linkText}` | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,11 @@ | |
| color: #000000; | ||
| } | ||
|
|
||
| .react-calendar .ACTIVE { | ||
| background: #22c55e; /* green-500 */ | ||
| color: #ffffff; | ||
| } | ||
|
Comment on lines
+16
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick 🧩 Analysis chainA11y + specificity: ACTIVE contrast likely fails AA; also increase selector specificity. White on green-500 (#22c55e) is marginal for normal text, and -.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 |
||
|
|
||
| .calendar-container { | ||
| padding: 20px; | ||
| max-width: 750px; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,8 @@ interface LOG_DATA { | |||||||||||||||||||||||||||||||||||
| status: string; | ||||||||||||||||||||||||||||||||||||
| startTime: number; | ||||||||||||||||||||||||||||||||||||
| endTime: number; | ||||||||||||||||||||||||||||||||||||
| taskTitle: string; | ||||||||||||||||||||||||||||||||||||
| taskTitle?: string; | ||||||||||||||||||||||||||||||||||||
| taskUrl?: string; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export const getStartOfDay = (date: Date): Date => { | ||||||||||||||||||||||||||||||||||||
|
|
@@ -22,13 +23,13 @@ export const getStartOfDay = (date: Date): Date => { | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export const getDatesInRange = (startDate: Date, endDate: Date) => { | ||||||||||||||||||||||||||||||||||||
| const date = getStartOfDay(startDate); | ||||||||||||||||||||||||||||||||||||
| const dates = []; | ||||||||||||||||||||||||||||||||||||
| const dates: number[] = []; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||
| !(startDate instanceof Date && !isNaN(startDate.getTime())) || | ||||||||||||||||||||||||||||||||||||
| !(endDate instanceof Date && !isNaN(endDate.getTime())) | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| return []; | ||||||||||||||||||||||||||||||||||||
| return [] as number[]; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| while (date <= getStartOfDay(endDate)) { | ||||||||||||||||||||||||||||||||||||
| dates.push(getStartOfDay(date).getTime()); | ||||||||||||||||||||||||||||||||||||
|
|
@@ -41,31 +42,37 @@ export const getDatesInRange = (startDate: Date, endDate: Date) => { | |||||||||||||||||||||||||||||||||||
| export const processData = ( | ||||||||||||||||||||||||||||||||||||
| itemId: string | null, | ||||||||||||||||||||||||||||||||||||
| data: [] | ||||||||||||||||||||||||||||||||||||
| ): [object, object] => { | ||||||||||||||||||||||||||||||||||||
| ): [object, object, object] => { | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
41
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-specific return type
Tell me moreWhat 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 mattersUsing 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 Previewtype 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. |
||||||||||||||||||||||||||||||||||||
| if (!itemId) { | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Type the API surface precisely (no 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| return [{}, {}]; | ||||||||||||||||||||||||||||||||||||
| return [{}, {}, {}]; | ||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| const log: any = data.find((log: LOG_TYPE) => { | ||||||||||||||||||||||||||||||||||||
| 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 commentThe 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| const dictWithStatus: Record<number, string> = {}; | ||||||||||||||||||||||||||||||||||||
| const dictWithTask: Record<number, string> = {}; | ||||||||||||||||||||||||||||||||||||
| const dictWithTaskUrl: Record<number, string> = {}; | ||||||||||||||||||||||||||||||||||||
| log.data.forEach((logData: LOG_DATA) => { | ||||||||||||||||||||||||||||||||||||
| const dates = getDatesInRange( | ||||||||||||||||||||||||||||||||||||
| new Date(logData.startTime), | ||||||||||||||||||||||||||||||||||||
| new Date(logData.endTime) | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
| if (logData.status === 'ACTIVE') { | ||||||||||||||||||||||||||||||||||||
| dates.forEach((dateTimestamp) => { | ||||||||||||||||||||||||||||||||||||
| dictWithTask[dateTimestamp] = logData.taskTitle; | ||||||||||||||||||||||||||||||||||||
| if (logData.taskTitle) { | ||||||||||||||||||||||||||||||||||||
| dictWithTask[dateTimestamp] = logData.taskTitle; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if (logData.taskUrl) { | ||||||||||||||||||||||||||||||||||||
| dictWithTaskUrl[dateTimestamp] = logData.taskUrl; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+63
to
69
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| dates.forEach((dateTimestamp) => { | ||||||||||||||||||||||||||||||||||||
| dictWithStatus[dateTimestamp] = logData.status; | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| return [dictWithStatus, dictWithTask]; | ||||||||||||||||||||||||||||||||||||
| return [dictWithStatus, dictWithTask, dictWithTaskUrl]; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
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:
Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.