Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 32 additions & 12 deletions client/components/tickets/ticket-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface TicketCreateProps {
export default function TicketCreate({ onCancel, onCreate }: TicketCreateProps) {
const { addToast } = useToast();
const { mutate: createTicket } = useCreateTicket();
const [formData, setFormData] = useState<Partial<Ticket>>({
const [formData, setFormData] = useState<Omit<Partial<Ticket>, "deadline"> & { deadline?: string }>({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I see what you're doing here, the type of this state variable is very cumbersome. This state variable should store the form data that will be sent to the server. The server should expect deadline to be a Go time.Time object (equivalent to JavaScript's Date object), which is not reflected here. I recommend reverting this change.

status: "Open",
title: "",
description: "",
Expand All @@ -26,6 +26,7 @@ export default function TicketCreate({ onCancel, onCreate }: TicketCreateProps)
createdBy: "techsquad@digitalnest.org",
site: "Watsonville",
category: "Hardware",
deadline: "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deadline should be the zero date value instead of a date string. The zero date value in JavaScript is:

new Date(0)

However, this will introduce a bug since JavaScript and Go have different zero values for time/date objects. More on this later (in storage.go).

});

const [isSaving, setIsSaving] = useState(false);
Expand Down Expand Up @@ -54,17 +55,25 @@ export default function TicketCreate({ onCancel, onCreate }: TicketCreateProps)
setIsSaving(true);

try {
createTicket(formData, {
onSuccess: (ticket) => {
onCreate(ticket);
addToast("New ticket created successfully!", "Success", 3500);
},
onError: (err) => {
console.error("Error creating ticket:", err);
addToast("An unexpected error occurred. Please try again.", "Error", 5000);
},
onSettled: () => setIsSaving(false),
});
// Prepare the ticket data, converting deadline string to Date and excluding empty deadline
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit pick: fix the formatting (excess indentation).

const { deadline, ...rest } = formData;
const ticketData: Partial<Ticket> = { ...rest } as Partial<Ticket>;

if (deadline && deadline !== "") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to convert from string to Date anymore.

ticketData.deadline = new Date(deadline);
}

createTicket(ticketData, {
onSuccess: (ticket) => {
onCreate(ticket);
addToast("New ticket created successfully!", "Success", 3500);
},
onError: (err) => {
console.error("Error creating ticket:", err);
addToast("An unexpected error occurred. Please try again.", "Error", 5000);
},
onSettled: () => setIsSaving(false),
});
} catch (error) {
console.error("Unexpected error:", error);
setIsSaving(false);
Expand Down Expand Up @@ -168,6 +177,17 @@ export default function TicketCreate({ onCancel, onCreate }: TicketCreateProps)
</select>
</div>
</div>
{/* Deadline input with date and time */}
<div className="mb-2">
<label className="block mb-1 font-medium text-gray-700 dark:text-gray-300">Deadline (Date & Time)</label>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best practice: instead of explicitly typing &, use the HTML Escape Character &amp;

<input
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check out this React library specifically for picking out dates. It exposes a DatePicker component and it's much friendlier to use.

className="w-full rounded border bg-white dark:bg-gray-800 px-3 py-2 text-sm text-gray-700 dark:text-gray-300"
type="datetime-local"
name="deadline"
value={formData.deadline || ""}
onChange={(e) => setFormData((prev) => ({ ...prev, deadline: e.target.value }))}
/>
</div>
<div className="mt-6 flex justify-end gap-3">
<Button
type="button"
Expand Down
75 changes: 73 additions & 2 deletions client/components/tickets/ticket-detail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,50 @@ export default function TicketDetail({ ticketId, onDismiss, onUpdate }: TicketDe
);
}

const ticketCreatedAt = new Date(ticket.createdOn).toDateString();
const ticketUpdatedAt = new Date(ticket.updatedAt).toDateString();
const ticketCreatedAt = new Date(ticket.createdOn).toLocaleString(undefined, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best practice: I understand these changes were to support dates and times, but let's just focus on the deadline field in this PR.

year: 'numeric',
month: 'numeric',
day: 'numeric',
hour: 'numeric',
minute: '2-digit',
hour12: true
});
const ticketUpdatedAt = new Date(ticket.updatedAt).toLocaleString(undefined, {
year: 'numeric',
month: 'numeric',
day: 'numeric',
hour: 'numeric',
minute: '2-digit',
hour12: true
});

// helper: format datetime strings with both date and time (no seconds)
const formatDateTime = (raw?: string | Date | null) => {
if (!raw) return null;
try {
const date = new Date(raw);
if (isNaN(date.getTime())) return null;
return date.toLocaleString(undefined, {
year: 'numeric',
month: 'numeric',
day: 'numeric',
hour: 'numeric',
minute: '2-digit',
hour12: true
});
} catch {
return null;
}
};

// screenshot: ensure absolute URL when server returns relative path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best practice: This PR is intended to add the deadline field to the Ticket. I recommend moving this logic to a different PR, for uploading images.

const makeScreenshotUrl = (path?: string | null) => {
if (!path) return null;
if (path.startsWith("http://") || path.startsWith("https://")) return path;
const base = process.env.NEXT_PUBLIC_API_URL ?? "";
// build with or without trailing slash cleanly
return `${base.replace(/\/$/, "")}${path.startsWith("/") ? "" : "/"}${path}`;
};

const TicketCreatedBy = ({ createdBy }: { createdBy: string }) => {
return (
Expand Down Expand Up @@ -94,6 +136,22 @@ export default function TicketDetail({ ticketId, onDismiss, onUpdate }: TicketDe
setTimeout(onUpdate, 300);
};

// compute deadline display and overdue
// ticket may not have the `deadline` property on the inferred Ticket type,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of using TypeScript is to avoid these types of situations. There is no need to cast ticket as any, since the Ticket type should define the deadline property to be of type Date:

type Ticket {
    ....
    updatedAt: Date;
    deadline: Date;
}

// so read it into a local variable with a narrow/known type to avoid TS errors.
const deadline = (ticket as any).deadline as string | Date | null | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since deadline should be initialized as new Date(0) in formData, if the date is the zero value (January 1, 1970, 00:00:00 UTC), there was no deadline selection made on the form.

You can use the getTime() method on a Date object to check the number of milliseconds since the zero value.

if (deadline.getTime() == 0) {
    // Handle no deadline selected
} else {
    // Handle deadline selected
}

const deadlineDisplay = formatDateTime(deadline);
const isOverdue = (() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best practice: Avoid self-invoking functions. isOverdue could be:

// Assuming deadline is not the zero value
const now = new Date();
const isOverdue = deadline.getTime() < now.getTime();

if (!deadline) return false;
try {
const deadlineDate = new Date(deadline);
const now = new Date();
return deadlineDate.getTime() < now.getTime();
} catch {
return false;
}
})();

return (
<div className="p-4 bg-gray-50 dark:bg-gray-800">
<div className="flex justify-between">
Expand Down Expand Up @@ -159,12 +217,25 @@ export default function TicketDetail({ ticketId, onDismiss, onUpdate }: TicketDe
</td>
<td className="text-gray-800 dark:text-gray-300">{ticketCreatedAt}</td>
</tr>

{/* Deadline row */}
<tr>
<td className={labelStyles}>
<LabeledIcon icon={<Calendar className="w-4" />} label="Deadline" />
</td>
<td className="text-gray-800 dark:text-gray-300">
{deadlineDisplay ?? <span className="text-gray-500">None</span>}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry about displaying time in this PR.

{isOverdue && <span className="ml-2 text-red-600 font-bold">Overdue</span>}
</td>
</tr>

<tr>
<td className={labelStyles}>
<LabeledIcon icon={<Calendar className="w-4" />} label="Last Modified" />
</td>
<td className="text-gray-800 dark:text-gray-300">{ticketUpdatedAt}</td>
</tr>

</tbody>
</table>
</div>
Expand Down
59 changes: 55 additions & 4 deletions client/components/tickets/ticket-edit.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use client";

import { Building, Tag, User } from "lucide-react";
import { FormEvent, useState } from "react";
import { FormEvent, useEffect, useState } from "react";

import { useTicket, useUpdateTicket } from "@/lib/hooks/queries/use-tickets";
import { useToast } from "@/lib/hooks/use-toast";
Expand All @@ -20,9 +20,36 @@ export default function TicketEdit({ ticketId, onCancel, onSave }: TicketEditPro
const { data: ticket } = useTicket(ticketId);
const { mutate: updateTicket } = useUpdateTicket();

const [formData, setFormData] = useState<Partial<Ticket>>(ticket || {});
const [formData, setFormData] = useState<Partial<Ticket> & { deadline?: string | Date }>(ticket || {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue here as with ticket-detail.tsx, there's no need to add & { deadline?: string | Date }.

const [isSaving, setIsSaving] = useState(false);

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry about displaying time in this PR.

if (ticket) {
setFormData((prev) => {
// Format deadline for datetime-local input (YYYY-MM-DDTHH:MM)
let deadlineString = "";
if (ticket.deadline) {
const deadlineDate = new Date(ticket.deadline);
if (!isNaN(deadlineDate.getTime())) {
// Convert to local datetime string format for datetime-local input
const year = deadlineDate.getFullYear();
const month = String(deadlineDate.getMonth() + 1).padStart(2, '0');
const day = String(deadlineDate.getDate()).padStart(2, '0');
const hours = String(deadlineDate.getHours()).padStart(2, '0');
const minutes = String(deadlineDate.getMinutes()).padStart(2, '0');
deadlineString = `${year}-${month}-${day}T${hours}:${minutes}`;
}
}

return {
...prev,
...ticket,
deadline: deadlineString,
} as Partial<Ticket> & { deadline?: string };
});
}
}, [ticket]);

if (!ticket) {
let layout = <></>;

Expand Down Expand Up @@ -66,8 +93,21 @@ export default function TicketEdit({ ticketId, onCancel, onSave }: TicketEditPro
setIsSaving(true);

try {
updateTicket({ id: ticketId, updates: formData });
onSave(formData);
// Prepare the updates, converting deadline string to Date if needed
const { deadline, ...rest } = formData;
const updates: Partial<Ticket> = { ...rest } as Partial<Ticket>;

// If deadline is provided as a string, convert it to Date
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for any of these conversions since deadline is a Date.

if (deadline && deadline !== "") {
if (typeof deadline === "string") {
updates.deadline = new Date(deadline);
} else {
updates.deadline = deadline;
}
}

updateTicket({ id: ticketId, updates });
onSave(updates);
addToast("Ticket updated successfully.", "Info", 2500);
} catch (error) {
console.error("Error updating ticket:", error);
Expand Down Expand Up @@ -184,6 +224,17 @@ export default function TicketEdit({ ticketId, onCancel, onSave }: TicketEditPro
</select>
</div>
</div>
{/* Deadline input with date and time */}
<div className="mb-4">
<label className="block text-sm font-medium text-gray-800 dark:text-gray-300 mb-1">Deadline (Date & Time)</label>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best practice: Use &amp; instead of & here, too.

<input
type="datetime-local"
name="deadline"
value={(formData as any).deadline ?? ""}
onChange={(e) => setFormData((prev) => ({ ...(prev as any), deadline: e.target.value }))}
className="w-full p-2 border border-gray-300 dark:border-gray-600 text-gray-800 dark:text-gray-300 rounded-md"
/>
</div>
<div className="mt-6 flex justify-end gap-3">
<Button
type="button"
Expand Down
9 changes: 8 additions & 1 deletion client/components/tickets/tickets-table-row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@ export interface TicketsTableRowProps {
}

export default function TicketTableRow({ ticket, onClick }: TicketsTableRowProps) {
const ticketUpdatedAt = new Date(ticket.updatedAt).toDateString();
const ticketUpdatedAt = new Date(ticket.updatedAt).toLocaleString(undefined, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for these changes.

year: 'numeric',
month: 'numeric',
day: 'numeric',
hour: 'numeric',
minute: '2-digit',
hour12: true
});
const columnStyles = "px-4 py-4 text-left text-gray-700 dark:text-gray-300";

let statusStyle: string;
Expand Down
70 changes: 68 additions & 2 deletions client/components/tickets/tickets-table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,77 @@ export default function TicketsTable({ tickets, onClick }: TicketsTableProps) {
<th className={`${commonStyles}`}>Title</th>
<th className={`${commonStyles} hidden md:block`}>Assigned To</th>
<th className={`${commonStyles}`}>Status</th>
<th className={`${commonStyles} hidden sm:block`}>Last Modified</th>

{/* Deadline (separate column) */}
<th className={`${commonStyles} hidden sm:table-cell`}>Deadline</th>

{/* Last Modified */}
<th className={`${commonStyles} hidden sm:table-cell`}>Last Modified</th>
</tr>
</thead>

<tbody className="divide-y divide-gray-200 dark:divide-gray-900 text-sm">
<TicketRows />
{tickets.map((ticket) => {
// Format deadline with both date and time (no seconds)
const deadline = (ticket as any).deadline;
const deadlineDisplay = deadline ? (() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issues here. Don't include the time in the deadline in this PR. Refer to my comments on ticket-detail.ts.

try {
const date = new Date(deadline);
return isNaN(date.getTime()) ? null : date.toLocaleString(undefined, {
year: 'numeric',
month: 'numeric',
day: 'numeric',
hour: 'numeric',
minute: '2-digit',
hour12: true
});
} catch {
return null;
}
})() : null;

// Check if deadline is overdue
const isOverdue = deadline ? (() => {
try {
const deadlineDate = new Date(deadline);
const now = new Date();
return deadlineDate.getTime() < now.getTime();
} catch {
return false;
}
})() : false;

return (
<tr key={ticket.id} className="hover:bg-gray-50 dark:hover:bg-gray-700" onClick={() => onClick(ticket)}>
<td className={commonStyles}>P{ticket.priority ?? "—"}</td>
<td className={`${commonStyles} hidden md:table-cell`}>{ticket.category ?? "—"}</td>
<td className={commonStyles}>{ticket.title}</td>
<td className={`${commonStyles} hidden md:table-cell`}>{ticket.assignedTo ?? "—"}</td>
<td className={commonStyles}>{ticket.status ?? "—"}</td>

{/* Deadline cell with date and time */}
<td className={`${commonStyles} hidden sm:table-cell`}>
{deadlineDisplay ? (
<div className="flex items-center gap-2">
<span>{deadlineDisplay}</span>
{isOverdue && <span className="text-xs text-red-600 font-bold">Overdue</span>}
</div>
) : <span className="text-gray-500">—</span>}
</td>

<td className={`${commonStyles} hidden sm:table-cell`}>
{ticket.updatedAt ? new Date(ticket.updatedAt).toLocaleString(undefined, {
year: 'numeric',
month: 'numeric',
day: 'numeric',
hour: 'numeric',
minute: '2-digit',
hour12: true
}) : "—"}
</td>
</tr>
);
})}
</tbody>
</table>
</div>
Expand Down
1 change: 1 addition & 0 deletions client/lib/types/ticket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export default interface Ticket {
status: Status;
createdOn: Date;
updatedAt: Date;
deadline?: Date;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Property deadline should not be optional.

}

export const Sites = ["Salinas", "Watsonville", "HQ", "Gilroy", "Modesto", "Stockton"] as const;
Expand Down
Loading