feat: implement Epic 5 — Manager Dashboard & Market-Day Oversight - #27
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Implements Epic 5 manager dashboard capabilities in the Go GraphQL backend, adding live attendance oversight, operational updates, confirmation requests, and auto-checkout workflows backed by new persistence and domain events.
Changes:
- Added
market_updatespersistence (migration + market repository interface/model) and GraphQL query/mutation for publishing/listing updates. - Added manager-only attendance dashboard query (
marketAttendance) plus supporting vendor repository methods for market/day check-ins. - Extended
checkInmutation to support manager-initiated check-in on behalf of a vendor; added new domain events and resolver tests.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| markets-api/migrations/000015_create_market_updates.up.sql | Creates market_updates table, index, and audit trigger |
| markets-api/migrations/000015_create_market_updates.down.sql | Drops market_updates table |
| markets-api/internal/vendor/repository.go | Adds vendor repo methods for market/day check-in reads and batch checkout |
| markets-api/internal/market/repository.go | Adds market repo methods for market updates create/list |
| markets-api/internal/market/market.go | Adds MarketUpdateRecord model |
| markets-api/internal/graph/vendor_helpers.go | Moves/centralizes helper mappers used by resolvers |
| markets-api/internal/graph/vendor.resolvers.go | Adds manager “check-in on behalf” logic |
| markets-api/internal/graph/vendor.resolvers_test.go | Updates mocks to satisfy new vendor repo interface |
| markets-api/internal/graph/schema/vendor.graphqls | Extends CheckInInput with optional vendorID |
| markets-api/internal/graph/schema/market.graphqls | Adds MarketAttendance, MarketUpdate, and new query/mutation fields |
| markets-api/internal/graph/model/models_gen.go | Regenerates gqlgen models for new schema fields/types |
| markets-api/internal/graph/market.resolvers.go | Implements publishMarketUpdate, marketUpdates, marketAttendance, autoCheckoutMarket, requestVendorConfirmation |
| markets-api/internal/graph/market.resolvers_test.go | Adds Epic 5 resolver tests and new vendor repo mock for those tests |
| markets-api/internal/graph/generated/generated.go | Regenerates gqlgen executable schema and marshalling code |
| markets-api/internal/events/types.go | Adds 3 new domain event types for Epic 5 workflows |
| _bmad-output/implementation-artifacts/sprint-status.yaml | Updates sprint status tracking to mark Epic 5 done |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() | ||
| ); | ||
|
|
||
| CREATE INDEX idx_market_updates_market_id ON market_updates(market_id); |
There was a problem hiding this comment.
The only index added is on market_id, but FindMarketUpdates is described as returning updates "newest first" and will likely query WHERE market_id = ... ORDER BY created_at DESC LIMIT/OFFSET. Consider adding a composite index on (market_id, created_at DESC) (or (market_id, created_at) depending on Postgres version) to avoid a sort on large markets.
| CREATE INDEX idx_market_updates_market_id ON market_updates(market_id); | |
| CREATE INDEX idx_market_updates_market_id_created_at_desc | |
| ON market_updates (market_id, created_at DESC); |
| for _, re := range roster { | ||
| entry := &model.VendorAttendanceEntry{ | ||
| VendorID: re.VendorID.String(), | ||
| Vendor: &model.Vendor{ID: re.VendorID.String()}, |
There was a problem hiding this comment.
MarketAttendance sets VendorAttendanceEntry.Vendor to &model.Vendor{ID: ...} only. As a result, clients selecting vendor fields like businessName, products, createdAt, etc. will get empty/default values instead of the real vendor profile. If the dashboard needs vendor details, consider populating the Vendor struct from the DB (likely via VendorRepo.FindVendorByUserID since roster entries use user IDs) or change VendorAttendanceEntry to expose only the vendor fields you can reliably provide without extra lookups.
| Vendor: &model.Vendor{ID: re.VendorID.String()}, |
| if input.VendorID != nil && role == "manager" { | ||
| // Manager check-in on behalf (Story 5.3) | ||
| if err := r.checkManagerScope(ctx, domain.MarketID(input.MarketID)); err != nil { | ||
| return nil, err | ||
| } | ||
| v, err = r.VendorRepo.FindVendorByID(ctx, domain.VendorID(*input.VendorID)) | ||
| if err != nil { | ||
| return nil, gqlerr.NewError(gqlerr.CodeValidationError, "vendor not found") | ||
| } | ||
| } else { | ||
| // Self check-in | ||
| v, err = r.VendorRepo.FindVendorByUserID(ctx, domain.UserID(uid)) | ||
| if err != nil { | ||
| slog.Error("failed to find vendor", "error", err, "userID", uid) | ||
| return nil, gqlerr.Internal("vendor profile not found") | ||
| } |
There was a problem hiding this comment.
Manager check-in behavior is ambiguous: if role == "manager" but input.VendorID is nil, the resolver falls back to "self check-in" and looks up a vendor profile by the manager’s userID, which will typically fail and surface as an internal error ("vendor profile not found"). Also, if a non-manager provides vendorID, it is silently ignored. Consider validating inputs explicitly: require vendorID when role is manager, and return a validation/forbidden error if vendorID is provided by a non-manager.
| if ci.MarketID == marketID { | ||
| result = append(result, ci) | ||
| } |
There was a problem hiding this comment.
The new mock FindCheckInsByMarketAndDate ignores the date parameter and returns all check-ins for the market. This can cause resolver tests (e.g., attendance counts) to pass even if the production implementation incorrectly fails to filter by date. Updating the mock to respect date will make the tests better reflect the repository contract.
| if ci.MarketID == marketID { | |
| result = append(result, ci) | |
| } | |
| if ci.MarketID != marketID { | |
| continue | |
| } | |
| // If a date is provided, filter check-ins to that specific day (YYYY-MM-DD). | |
| if date != "" { | |
| if ci.CheckedInAt == nil || ci.CheckedInAt.Format("2006-01-02") != date { | |
| continue | |
| } | |
| } | |
| result = append(result, ci) |
| func (m *mockVendorRepoForMarket) FindCheckInsByMarketAndDate(_ context.Context, marketID domain.MarketID, _ string) ([]*vendor.CheckInRecord, error) { | ||
| var result []*vendor.CheckInRecord | ||
| for _, ci := range m.checkIns { | ||
| if ci.MarketID == marketID { | ||
| result = append(result, ci) | ||
| } |
There was a problem hiding this comment.
The new mock FindCheckInsByMarketAndDate ignores the date parameter and returns all check-ins for the market. This weakens the Epic 5 resolver tests because they won’t catch date-filtering bugs in attendance logic. Consider filtering by the check-in timestamp’s date (or using a test field) to honor the method contract.
| func (m *mockVendorRepoForMarket) FindCheckInsByMarketAndDate(_ context.Context, marketID domain.MarketID, _ string) ([]*vendor.CheckInRecord, error) { | |
| var result []*vendor.CheckInRecord | |
| for _, ci := range m.checkIns { | |
| if ci.MarketID == marketID { | |
| result = append(result, ci) | |
| } | |
| func (m *mockVendorRepoForMarket) FindCheckInsByMarketAndDate(_ context.Context, marketID domain.MarketID, date string) ([]*vendor.CheckInRecord, error) { | |
| var result []*vendor.CheckInRecord | |
| // If a date is provided, parse it and filter check-ins by that calendar date. | |
| var filterDate time.Time | |
| var err error | |
| if date != "" { | |
| filterDate, err = time.Parse("2006-01-02", date) | |
| if err != nil { | |
| return nil, err | |
| } | |
| } | |
| for _, ci := range m.checkIns { | |
| if ci.MarketID != marketID { | |
| continue | |
| } | |
| // If no date was specified, return all check-ins for the market (previous behavior). | |
| if date == "" { | |
| result = append(result, ci) | |
| continue | |
| } | |
| // Match by calendar date of the check-in timestamp. | |
| ciDate := ci.CheckedInAt | |
| if ciDate.Year() == filterDate.Year() && | |
| ciDate.Month() == filterDate.Month() && | |
| ciDate.Day() == filterDate.Day() { | |
| result = append(result, ci) | |
| } |
67d9996 to
f3cc4ba
Compare
a2243d3 to
c5032c7
Compare
f3cc4ba to
04342d2
Compare
c5032c7 to
2b7f750
Compare
04342d2 to
958863d
Compare
2b7f750 to
b0e5ab7
Compare
958863d to
9ed73c9
Compare
b0e5ab7 to
460e758
Compare
9ed73c9 to
afb3307
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> # Conflicts: # markets-api/internal/graph/vendor_helpers.go
80dc7f7 to
c18eaee
Compare
f70d598 to
80dc7f7
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merging Epic 5: Manager Dashboard & Market-Day Oversight All CI checks pass.
Summary
marketAttendancequery returns roster + check-in status with summary countsrequestVendorConfirmationmutation sends confirmation requests via event buscheckInmutation now accepts optionalvendorIDfor manager-initiated check-ins with scope validationpublishMarketUpdatemutation +marketUpdatesquery withmarket_updatesmigrationautoCheckoutMarketmutation batch-checks-out all active vendorsMarketUpdatePublished,VendorConfirmationRequested,MarketAutoCheckoutCompletedTest plan
go test ./...)🤖 Generated with Claude Code