use domain + dto models for events api + cli#189
Conversation
godoc reference preview ready ✨
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the Events API and CLI to use domain.* models for service/business logic and dto.* models for HTTP/CLI payloads, updating API docs and fixtures accordingly.
Changes:
- Switch Events service and handler layers to operate on
domain.Event/domain.UpdateEventand convert responses todto.Event. - Update CLI event create/update commands (and announcement time parsing) to use
dtopayloads and shared time parsing helpers. - Regenerate Swagger docs and update JSON fixtures for the new DTO shapes.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/time.go | Adds shared time layout and parsing helper used by CLI/API flows. |
| utils/convert.go | Removes old byte-slice conversion helpers by commenting them out. |
| internal/dto/event.go | Adds DTO↔domain conversions for events, including update DTO conversion. |
| internal/domain/event.go | Removes UUID field from domain.UpdateEvent. |
| internal/cli/events/post.go | Uses dto.Event and the new time parsing helper when creating events. |
| internal/cli/events/put.go | Uses dto.Event/dto.UpdateEvent and formats/parses timestamps for edits. |
| internal/cli/announcements/post.go | Switches announcement time parsing to the new helper. |
| internal/cli/announcements/put.go | Switches announcement time parsing to the new helper. |
| internal/api/store/events_mapper.go | Updates event update mapping after removing UUID from domain update model. |
| internal/api/services/event.go | Moves service interface to domain models; adds DB↔domain mapping. |
| internal/api/services/board.go | Adds a TODO note about filter types. |
| internal/api/handlers/event.go | Returns DTOs from handlers and updates swagger annotations. |
| internal/api/docs/swagger.yaml | Replaces dbmodels event schemas with DTO event schemas. |
| internal/api/docs/swagger.json | Same as above for JSON swagger. |
| internal/api/docs/docs.go | Same as above for embedded swagger template. |
| fixtures/event_update_partial.json | Updates fixture payload shape to DTO style. |
| fixtures/event_update_location.json | Updates fixture payload shape to DTO style. |
| fixtures/event_update_full.json | Updates fixture payload shape to DTO style. |
Comments suppressed due to low confidence (1)
internal/dto/event.go:23
- ToDomain converts StartAt/EndAt via utils.UnixToTime, which currently interprets the int64 as Unix seconds. In this repo, event timestamps are stored/used as UTC milliseconds (see sql/migrations/000002_create_events.up.sql and fixtures), so this will mis-convert times by ~1000x. Update the conversion (and the reverse DTO mapping) to use milliseconds consistently.
Uuid: e.Uuid,
Location: e.Location,
StartAt: utils.UnixToTime(e.StartAt),
EndAt: utils.UnixToTime(e.EndAt),
IsAllDay: e.IsAllDay,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| startTime, err := time.ParseInLocation(timeLayout, timeStr, loc) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("error parsing time format: %s", err) | ||
| } | ||
| return startTime.Unix(), nil |
There was a problem hiding this comment.
ParseTime returns startTime.Unix() (seconds). Since the API/DB uses UTC milliseconds for timestamps, callers will end up sending seconds into fields that expect milliseconds. Return milliseconds here (e.g., via UnixMilli) and keep the unit consistent with FormatUnix/UnixToTime.
| *d.StartAt = utils.UnixToTime(*e.StartAt) | ||
| } | ||
| if e.EndAt != nil { | ||
| *d.EndAt = utils.UnixToTime(*e.EndAt) |
There was a problem hiding this comment.
UpdateEvent.ToDomain assigns through *d.StartAt / *d.EndAt without ever initializing d.StartAt/d.EndAt. If StartAt/EndAt are present, this will panic at runtime. Instead, create new time.Time values and set the pointers (or use a helper that returns *time.Time).
| *d.StartAt = utils.UnixToTime(*e.StartAt) | |
| } | |
| if e.EndAt != nil { | |
| *d.EndAt = utils.UnixToTime(*e.EndAt) | |
| startAt := utils.UnixToTime(*e.StartAt) | |
| d.StartAt = &startAt | |
| } | |
| if e.EndAt != nil { | |
| endAt := utils.UnixToTime(*e.EndAt) | |
| d.EndAt = &endAt |
| // For unix times of int64 to readable format of 03:04:05PM 01/02/06 | ||
| func FormatUnix(unixTime int64) string { | ||
| t := time.Unix(unixTime, 0) | ||
| return t.Format("01/02/06 03:04PM") | ||
| return t.Format(timeLayout) | ||
| } |
There was a problem hiding this comment.
FormatUnix treats the input as Unix seconds (time.Unix(unixTime, 0)), but event/announcement timestamps in this repo are stored as UTC milliseconds (see SQL migrations/fixtures). This will format incorrect dates. Update this helper to interpret the value as milliseconds (e.g., time.UnixMilli) and keep units consistent across the codebase.
| func UpdateEventDomainToDB(event domain.UpdateEvent) dbmodels.UpdateEventParams { | ||
| return dbmodels.UpdateEventParams{ | ||
| Uuid: event.Uuid, | ||
| Location: stringToNullString(event.Location), | ||
| StartAt: timeToNullInt64(event.StartAt), | ||
| EndAt: timeToNullInt64(event.EndAt), |
There was a problem hiding this comment.
UpdateEventDomainToDB uses timeToNullInt64, which converts *time.Time via value.Unix() (seconds). The events table stores start_at/end_at as UTC milliseconds (see sql/migrations/000002_create_events.up.sql), so this will write incorrect values. Convert to milliseconds (UnixMilli) in the mapper/util and ensure DB↔domain and domain↔DTO mappings use the same unit.
| // @Produce json | ||
| // @Param body body dbmodels.CreateEventParams true "Event data" | ||
| // @Param body body dto.Event true "Event data" | ||
| // @Success 200 {object} map[string]interface{} "Success message with UUID" |
There was a problem hiding this comment.
The CreateEvent swagger/godoc annotation says the 200 response includes a UUID, but the handler only returns {"message": ...}. Either include the UUID in the JSON response (it’s available on the request body) or update the annotation to match the actual response.
| // @Success 200 {object} map[string]interface{} "Success message with UUID" | |
| // @Success 200 {object} map[string]interface{} "Success message" |
| package utils | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strconv" | ||
| "time" | ||
| // "fmt" | ||
| // "strconv" |
There was a problem hiding this comment.
This file now contains only commented-out imports/functions. If these helpers are no longer used, delete the file; otherwise restore the implementation (and tests) so the utils package doesn’t accumulate dead code.
No description provided.