Combine ticket update hooks into one that updates all fields #780
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates multiple ticket update hooks into a single unified hook that updates all ticket fields. It simplifies the architecture by replacing separate hooks for assignee changes, topic changes, and activity updates with one comprehensive UpdateTickets hook.
Key changes:
- Modified ticket modifiers to directly update ModifiedOn and LastActivityOn fields
- Created a new UpdateTickets hook that handles all ticket field updates
- Removed three specialized hooks (UpdateTicketAssignee, UpdateTicketTopic, UpdateTicketActivity)
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| core/tickets/modifiers.go | Added timestamp updates directly in modifier functions |
| core/runner/hooks/update_tickets.go | New unified hook for updating all ticket fields |
| core/runner/hooks/update_ticket_*.go | Removed specialized hooks for assignee, topic, and activity updates |
| core/runner/handlers/ticket_*.go | Updated to use the new unified UpdateTickets hook |
| core/models/ticket_test.go | Replaced separate test functions with comprehensive UpdateTickets test |
| core/models/ticket.go | Replaced multiple update functions with single UpdateTickets function using bulk query |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| now := dates.Now() | ||
| prevAssignee := oa.UserByID(ticket.AssigneeID) | ||
|
|
||
| ticket.AssigneeID = assigneeID | ||
| ticket.ModifiedOn = now |
There was a problem hiding this comment.
The newAssignee variable referenced in line 25 is not defined. It should be oa.UserByID(assigneeID) to get the new assignee reference.
Copilot uses AI. Check for mistakes.
core/runner/hooks/update_tickets.go
Outdated
|
|
||
| for _, args := range scenes { | ||
| for _, item := range args { | ||
| tickets = append(tickets, item.([]*models.Ticket)...) |
There was a problem hiding this comment.
Type assertion without checking could cause a panic if the item is not of type []*models.Ticket. Consider using a type switch or checking the assertion result.
| tickets = append(tickets, item.([]*models.Ticket)...) | |
| if ts, ok := item.([]*models.Ticket); ok { | |
| tickets = append(tickets, ts...) | |
| } else { | |
| return fmt.Errorf("unexpected type in scenes: got %T, want []*models.Ticket", item) | |
| } |
Copilot uses AI. Check for mistakes.
7c51cd0 to
ea0f3d5
Compare
ea0f3d5 to
50e88a0
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #780 +/- ##
==========================================
+ Coverage 50.17% 50.21% +0.03%
==========================================
Files 260 258 -2
Lines 15250 15231 -19
==========================================
- Hits 7652 7648 -4
+ Misses 6799 6785 -14
+ Partials 799 798 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.