-
Notifications
You must be signed in to change notification settings - Fork 2
client(feat): add deadline field to ticket management with input and … #10
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
base: dev
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -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 }>({ | ||
| status: "Open", | ||
| title: "", | ||
| description: "", | ||
|
|
@@ -26,6 +26,7 @@ export default function TicketCreate({ onCancel, onCreate }: TicketCreateProps) | |
| createdBy: "techsquad@digitalnest.org", | ||
| site: "Watsonville", | ||
| category: "Hardware", | ||
| deadline: "", | ||
|
Contributor
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.
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); | ||
|
|
@@ -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 | ||
|
Contributor
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. Nit pick: fix the formatting (excess indentation). |
||
| const { deadline, ...rest } = formData; | ||
| const ticketData: Partial<Ticket> = { ...rest } as Partial<Ticket>; | ||
|
|
||
| if (deadline && deadline !== "") { | ||
|
Contributor
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. No need to convert from string to |
||
| 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); | ||
|
|
@@ -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> | ||
|
Contributor
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. Best practice: instead of explicitly typing |
||
| <input | ||
|
Contributor
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. Check out this React library specifically for picking out dates. It exposes a |
||
| 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" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, { | ||
|
Contributor
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. Best practice: I understand these changes were to support dates and times, but let's just focus on the |
||
| 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 | ||
|
Contributor
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. Best practice: This PR is intended to add the |
||
| 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 ( | ||
|
|
@@ -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, | ||
|
Contributor
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. The purpose of using TypeScript is to avoid these types of situations. There is no need to cast 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; | ||
|
Contributor
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. Since You can use the if (deadline.getTime() == 0) {
// Handle no deadline selected
} else {
// Handle deadline selected
} |
||
| const deadlineDisplay = formatDateTime(deadline); | ||
| const isOverdue = (() => { | ||
|
Contributor
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. Best practice: Avoid self-invoking functions. // 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"> | ||
|
|
@@ -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>} | ||
|
Contributor
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. 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> | ||
|
|
||
| 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"; | ||
|
|
@@ -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 || {}); | ||
|
Contributor
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 issue here as with |
||
| const [isSaving, setIsSaving] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
|
Contributor
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. 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 = <></>; | ||
|
|
||
|
|
@@ -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 | ||
|
Contributor
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. No need for any of these conversions since |
||
| 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); | ||
|
|
@@ -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> | ||
|
Contributor
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. Best practice: Use |
||
| <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" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, { | ||
|
Contributor
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. 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; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 ? (() => { | ||
|
Contributor
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 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> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ export default interface Ticket { | |
| status: Status; | ||
| createdOn: Date; | ||
| updatedAt: Date; | ||
| deadline?: Date; | ||
|
Contributor
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. Property |
||
| } | ||
|
|
||
| export const Sites = ["Salinas", "Watsonville", "HQ", "Gilroy", "Modesto", "Stockton"] as const; | ||
|
|
||
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.
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
deadlineto be a Gotime.Timeobject (equivalent to JavaScript'sDateobject), which is not reflected here. I recommend reverting this change.