feat: get shift priorities endpoint#135
Conversation
This adds, as I forgot it, an endpoint to retrieve all priorities for a specific shift group.
There was a problem hiding this comment.
Pull request overview
Adds a missing ShiftGroup API endpoint to retrieve all user priorities for a given shift group, and regenerates OpenAPI + TypeScript client artifacts accordingly.
Changes:
- Add
GET /roster/shift-groups/{id}/priorityhandler + service method. - Rename the update DTO from
GroupUpdatePriorityParamtoGroupPriorityUpdateParamacross server + OpenAPI + client. - Regenerate Swagger artifacts and the TS client/docs to include the new endpoint and renamed DTO.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/roster/service.go | Adds service interface + implementation for fetching shift group priorities; updates update method param type. |
| internal/roster/handler.go | Registers new GET route + handler; switches CreateShiftGroup binding to ShouldBindBodyWith; updates update DTO usage. |
| internal/roster/dto.go | Renames the priority update request DTO type. |
| internal/models/roster.go | Adds Swagger naming comment for GroupPriority. |
| docs/swagger.yaml | Adds GET priority endpoint; renames DTO schema reference. |
| docs/swagger.json | Adds GET priority endpoint; renames DTO schema reference. |
| docs/docs.go | Regenerated embedded Swagger template with new endpoint + renamed DTO. |
| client/src/docs/ShiftGroupApi.md | Adds client docs for new endpoint; updates DTO name references. |
| client/src/docs/GroupPriorityUpdateParam.md | Adds generated doc page for renamed DTO. |
| client/src/api.ts | Adds generated client method getShiftGroupPriorities; renames DTO type usage. |
| client/src/.openapi-generator/FILES | Updates generated file manifest to new DTO doc filename. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| g.GET("/shift-groups/:id", h.GetShiftGroup) | ||
|
|
||
| g.GET("/shift-groups/:id/priority", requireShiftGroupOrganRoleParams(db, models.RoleAdmin), h.GetShiftGroupPriorities) | ||
| g.PUT("/shift-groups/:id/priority", requireShiftGroupOrganRoleParams(db, models.RoleAdmin)) |
There was a problem hiding this comment.
Route registration for PUT /shift-groups/:id/priority is missing the actual handler (UpdateShiftGroupPriority). As written, only the access-check middleware runs and the request will fall through with an empty 200 response. Register the PUT route with h.UpdateShiftGroupPriority after the middleware.
| g.PUT("/shift-groups/:id/priority", requireShiftGroupOrganRoleParams(db, models.RoleAdmin)) | |
| g.PUT("/shift-groups/:id/priority", requireShiftGroupOrganRoleParams(db, models.RoleAdmin), h.UpdateShiftGroupPriority) |
| func (s *service) GetShiftGroupPriorities(groupID uint) ([]*models.ShiftGroupPriority, error) { | ||
| var priorities []*models.ShiftGroupPriority | ||
| if err := s.db.Find(&priorities).Error; err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return priorities, nil |
There was a problem hiding this comment.
GetShiftGroupPriorities ignores the provided groupID and currently returns priorities for all shift groups. This can leak data across organs and will also grow unbounded as the table grows. Add a WHERE filter on shift_group_id = groupID (and consider an explicit ORDER BY if you want deterministic results).
|
|
||
| // GetShiftGroupPriorities | ||
| // | ||
| // @Summary Get a shift group priorities for a shift group |
There was a problem hiding this comment.
The Swagger summary text is grammatically incorrect ("Get a shift group priorities for a shift group"). Since this string is propagated into generated swagger/docs and the TS client docs, consider changing it to something like "Get shift group priorities" (or similar) in the handler annotation.
| // @Summary Get a shift group priorities for a shift group | |
| // @Summary Get shift group priorities |
| docs/GEWISRoosterInternalModelsOrganRole.md | ||
| docs/GroupUpdatePriorityParam.md | ||
| docs/GroupPriorityUpdateParam.md |
There was a problem hiding this comment.
The generator file list was updated to reference GroupPriorityUpdateParam.md, but the old generated markdown (docs/GroupUpdatePriorityParam.md) still exists in the repo (it is no longer referenced here). Consider removing the old file to avoid stale/duplicated client docs.
This adds, as I forgot it, an endpoint to retrieve all priorities for a specific shift group.
Types of changes