client(feat): add deadline field to ticket management with input and …#10
client(feat): add deadline field to ticket management with input and …#10
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive deadline support to the ticket management system, allowing users to set, edit, and view deadlines for tickets with visual indicators for overdue items.
- Added deadline field to the backend Ticket model with proper BSON handling and RFC3339 parsing
- Implemented deadline input fields in ticket creation and editing forms
- Enhanced ticket views to display deadlines with overdue indicators
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/internal/models/ticket.go | Added Deadline field to Ticket struct and BSON unmarshalling |
| server/internal/storage/storage.go | Added deadline parsing and storage logic for ticket updates |
| client/components/tickets/ticket-create.tsx | Added deadline input field to ticket creation form |
| client/components/tickets/ticket-edit.tsx | Added deadline input field and initialization logic to edit form |
| client/components/tickets/ticket-detail.tsx | Added deadline display with overdue indicator in ticket details |
| client/components/tickets/tickets-table.tsx | Added deadline column with overdue highlighting to tickets table |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
villaleo
left a comment
villaleo
left a comment
There was a problem hiding this comment.
Overall, nice job on adding the deadline. However, there is room for improvement in your code.
Please make sure to keep changes in PRs targeted to a single feature/fix.
| createdBy: "techsquad@digitalnest.org", | ||
| site: "Watsonville", | ||
| category: "Hardware", | ||
| deadline: "", |
There was a problem hiding this comment.
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 { addToast } = useToast(); | ||
| const { mutate: createTicket } = useCreateTicket(); | ||
| const [formData, setFormData] = useState<Partial<Ticket>>({ | ||
| const [formData, setFormData] = useState<Omit<Partial<Ticket>, "deadline"> & { deadline?: string }>({ |
There was a problem hiding this comment.
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.
| }, | ||
| onSettled: () => setIsSaving(false), | ||
| }); | ||
| // Prepare the ticket data, converting deadline string to Date and excluding empty deadline |
There was a problem hiding this comment.
Nit pick: fix the formatting (excess indentation).
| status: Status; | ||
| createdOn: Date; | ||
| updatedAt: Date; | ||
| deadline?: Date; |
There was a problem hiding this comment.
Property deadline should not be optional.
| const { deadline, ...rest } = formData; | ||
| const ticketData: Partial<Ticket> = { ...rest } as Partial<Ticket>; | ||
|
|
||
| if (deadline && deadline !== "") { |
There was a problem hiding this comment.
No need to convert from string to Date anymore.
| Status string `json:"status"` | ||
| CreatedOn time.Time `json:"createdOn"` | ||
| UpdatedAt time.Time `json:"updatedAt"` | ||
| Deadline *time.Time `json:"deadline,omitempty"` |
There was a problem hiding this comment.
Field Deadline should not be a pointer. Remove the omitempty tag.
| Status string `json:"status"` | ||
| CreatedOn time.Time `json:"createdOn"` | ||
| UpdatedAt time.Time `json:"updatedAt"` | ||
| Deadline *time.Time `json:"deadline,omitempty"` |
There was a problem hiding this comment.
Remove pointer. Remove omitempty.
| ) | ||
|
|
||
| // Add deadline if it's provided | ||
| if ticket.Deadline != nil { |
There was a problem hiding this comment.
Since Deadline is no longer a pointer, check if its the JavaScript Date zero value (January 1, 1970, 00:00:00 UTC) to determine how to store the date:
// Get the milliseconds since the Unix epoch (AKA the JavaScript zero value)
milliseconds := ticket.Deadline.UnixMilli()
if milliseconds == 0 {
// Create a time.Time representing the Unix epoch
unixEpoch := time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC)
// Store the Unix epoch instead
doc = append(doc, bson.E{Key: "deadline", Value: unixEpoch})
} else {
// Non-zero date value provided; store the deadline
doc = append(doc, bson.E{Key: "deadline", Value: ticket.Deadline})
}| updatesDoc = append(updatesDoc, bson.E{Key: "status", Value: status}) | ||
| } | ||
|
|
||
| if deadline, ok := updates["deadline"].(string); ok { |
There was a problem hiding this comment.
Almost! No need to try parsing different formatted date strings. RFC3339 should work:
if datestr, ok := updates["deadline"].(string); ok {
deadline, err := time.Parse(time.RFC3339, datestr)
if err != nil {
sugar.Error(err)
return nil, err
}
updatesDoc = append(updatesDoc, bson.E{Key: "deadline", Value: deadline})
}| {tickets.map((ticket) => { | ||
| // Format deadline with both date and time (no seconds) | ||
| const deadline = (ticket as any).deadline; | ||
| const deadlineDisplay = deadline ? (() => { |
There was a problem hiding this comment.
Same issues here. Don't include the time in the deadline in this PR. Refer to my comments on ticket-detail.ts.
2 participants
This pull request adds support for a "deadline" field to tickets across both the frontend and backend. Users can now set, edit, and view deadlines for tickets, and the UI will display deadlines and highlight overdue tickets. The backend has been updated to store and handle the new deadline field, including parsing and saving it correctly.
Frontend changes:
Ticket creation, editing, and viewing:
ticket-create.tsx) and edit (ticket-edit.tsx) forms, storing the value as a date string in the form state. [1] [2] [3] [4]ticket-detail.tsx) to display the deadline, formatted as a local date, and show an "Overdue" label if the deadline has passed. [1] [2] [3]tickets-table.tsx) to include a deadline column, displaying the formatted deadline and an "Overdue" indicator for past deadlines.Backend changes:
Ticket model and persistence:
Deadlinefield of typetime.Timeto theTicketstruct and ensured it is handled in BSON unmarshalling. [1] [2] [3]