[LFXV2-512] feat: Implement PUT endpoints for GroupsIO services, mailing lists, and members #9
Conversation
…ing lists, and members - Add comprehensive update functionality for all three resource types - Implement immutable field validation to protect critical attributes - Add ETag-based optimistic concurrency control - Enhanced payload converters with proper nil checks - Improved validators with consistent error messages - Add comprehensive test coverage for all update scenarios - Update OpenAPI specifications with PUT endpoint documentation Assisted by [Claude Code](https://claude.ai/code) Signed-off-by: Prabodh Chaudhari <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughSwitches update endpoints from PATCH to PUT and replaces per-resource “no-defaults” update attributes with shared/base attribute sets. Payloads now use concrete types (not pointers) for many fields. Converters perform full replacements, validators compare direct values, and a post-conversion service business-rule check is added before persisting updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as API (GOA)
participant Svc as MailingList Service
participant Conv as Converter
participant Val as Validators
participant BR as Business Rules
participant Repo as Repository
Client->>API: PUT /groupsio/services/{uid}
API->>Svc: UpdateGrpsioService(payload)
Svc->>Conv: convertUpdatePayloadToDomain(payload, existing)
Conv-->>Svc: domainService (full replacement)
Svc->>Val: validateUpdateImmutabilityConstraints(payload, existing)
Val-->>Svc: ok / error
alt validation error
Svc-->>API: error
API-->>Client: 4xx
else validation ok
Svc->>BR: validateServiceBusinessRules(domainService)
BR-->>Svc: ok / error
alt business-rule error
Svc-->>API: error
API-->>Client: 4xx
else ok
Svc->>Repo: Update(domainService)
Repo-->>Svc: updated
Svc-->>API: success
API-->>Client: 2xx + body
end
end
sequenceDiagram
autonumber
actor Client
participant API as API (GOA)
participant Svc as MailingList Service
participant Conv as Converter
participant Val as Validators
participant Reader as ServiceReader (optional)
participant Repo as Repository
Client->>API: PUT /groupsio/mailing-lists/{uid}
API->>Svc: UpdateGrpsioMailingList(payload)
Svc->>Conv: convertUpdatePayloadToDomain(payload, existing)
Conv-->>Svc: domainList (full replacement)
Svc->>Val: validateMailingListUpdate(payload, existing, Reader)
alt ServiceUID changed
Val->>Reader: GetService(payload.ServiceUID)
Reader-->>Val: newService
Val-->>Svc: ok / cross-project error
else
Val-->>Svc: ok
end
alt error
Svc-->>API: error
API-->>Client: 4xx
else
Svc->>Repo: Update(domainList)
Repo-->>Svc: updated
Svc-->>API: success
API-->>Client: 2xx + body
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR implements PUT endpoints for GroupsIO services, mailing lists, and members by changing the HTTP method from PATCH to PUT across the API and adapting the payload structures to use PUT semantics (requiring all fields instead of partial updates).
- Changed HTTP methods from PATCH to PUT for all update operations
- Updated payload structures to require core fields and use non-pointer types where appropriate
- Modified validation logic to handle the new required field semantics
Reviewed Changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gen/mailing_list/service.go | Updated payload types to use non-pointer types for required fields |
| gen/http/openapi3.yaml | Changed endpoint methods from PATCH to PUT and updated request body schemas |
| gen/http/openapi.yaml | Changed endpoint methods from PATCH to PUT and updated parameter requirements |
| gen/http/mailing_list/server/types.go | Added validation for required fields and default value handling |
| gen/http/mailing_list/server/server.go | Updated HTTP method registration from PATCH to PUT |
| gen/http/mailing_list/client/types.go | Changed field types from pointers to non-pointers for required fields |
| gen/http/mailing_list/client/encode_decode.go | Updated HTTP request methods from PATCH to PUT |
| gen/http/mailing_list/client/cli.go | Updated validation logic for non-pointer field types |
| cmd/mailing-list-api/service/service_validators_test.go | Updated test payloads to use non-pointer types |
| cmd/mailing-list-api/service/service_validators.go | Updated validation logic to handle non-pointer required fields |
| cmd/mailing-list-api/service/service_payload_converters_test.go | Updated test cases for new field types |
| cmd/mailing-list-api/service/service_payload_converters.go | Simplified payload conversion logic for PUT semantics |
| cmd/mailing-list-api/service/mailing_list_validators_test.go | Updated test payloads for non-pointer types |
| cmd/mailing-list-api/design/type.go | Removed update-specific attribute functions that handled pointer types |
| cmd/mailing-list-api/design/mailing_list.go | Updated endpoint definitions to use PUT and base attributes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cmd/mailing-list-api/service/service_payload_converters.go (1)
176-199: Member update should preserve existing fields when omitted.Current code zeros Username/First/Last/Org/Job when pointers are nil and overwrites DeliveryMode/ModStatus with empty values. Preserve existing unless explicitly provided.
- return &model.GrpsIOMember{ - // Preserve immutable fields - UID: existing.UID, - MailingListUID: existing.MailingListUID, - Email: existing.Email, // Immutable - MemberType: existing.MemberType, // Immutable for now - GroupsIOMemberID: existing.GroupsIOMemberID, - GroupsIOGroupID: existing.GroupsIOGroupID, - CreatedAt: existing.CreatedAt, - Status: existing.Status, - LastReviewedAt: existing.LastReviewedAt, - LastReviewedBy: existing.LastReviewedBy, - - // Update all mutable fields (PUT semantics - all fields provided) - Username: payloadStringValue(payload.Username), - FirstName: payloadStringValue(payload.FirstName), - LastName: payloadStringValue(payload.LastName), - Organization: payloadStringValue(payload.Organization), - JobTitle: payloadStringValue(payload.JobTitle), - DeliveryMode: payload.DeliveryMode, - ModStatus: payload.ModStatus, - UpdatedAt: time.Now().UTC(), - } + // Preserve-if-omitted semantics for optional fields + username := existing.Username + if payload.Username != nil { username = *payload.Username } + firstName := existing.FirstName + if payload.FirstName != nil { firstName = *payload.FirstName } + lastName := existing.LastName + if payload.LastName != nil { lastName = *payload.LastName } + organization := existing.Organization + if payload.Organization != nil { organization = *payload.Organization } + jobTitle := existing.JobTitle + if payload.JobTitle != nil { jobTitle = *payload.JobTitle } + deliveryMode := existing.DeliveryMode + if payload.DeliveryMode != "" { deliveryMode = payload.DeliveryMode } + modStatus := existing.ModStatus + if payload.ModStatus != "" { modStatus = payload.ModStatus } + + return &model.GrpsIOMember{ + // Preserve immutable fields + UID: existing.UID, + MailingListUID: existing.MailingListUID, + Email: existing.Email, + MemberType: existing.MemberType, + GroupsIOMemberID: existing.GroupsIOMemberID, + GroupsIOGroupID: existing.GroupsIOGroupID, + CreatedAt: existing.CreatedAt, + Status: existing.Status, + LastReviewedAt: existing.LastReviewedAt, + LastReviewedBy: existing.LastReviewedBy, + // Mutables + Username: username, + FirstName: firstName, + LastName: lastName, + Organization: organization, + JobTitle: jobTitle, + DeliveryMode: deliveryMode, + ModStatus: modStatus, + UpdatedAt: time.Now().UTC(), + }Note: If Goa sets defaults for delivery_mode/mod_status on update, prefer removing those defaults in the Update attributes (see design comment) to avoid silent resets.
cmd/mailing-list-api/service/service_payload_converters_test.go (1)
513-517: Align partial/empty service update tests with PUT or drop them.
- For “only status”/“only public”, include type and project_uid (and any other required fields) to avoid unintended zeroing.
- “empty update” does not make sense for PUT; either remove or turn into a no‑op where all required fields are provided equal to existing.
Also applies to: 533-536, 553-560, 576-581
🧹 Nitpick comments (7)
cmd/mailing-list-api/service/mailing_list_validators_test.go (2)
39-44: Tests use value-types for PUT; ensure required fields are always present.Design now requires title, type, group_name, public, description, service_uid for PUT. These cases omit title and/or type. Either:
- Add the missing required fields to these payloads, or
- Keep these as pure validator-unit tests but add integration tests hitting the Goa layer to assert required-field enforcement.
Also applies to: 55-60, 70-77, 85-93, 103-109, 119-127
164-188: Main-group rules look correct; clarify PUT semantics in tests.These new-rule tests focus on business rules (immutability, visibility/type constraints). Consider adding at least one positive case that includes all PUT‑required fields to mirror real request payloads.
Also applies to: 205-214, 231-240, 257-266, 283-291, 309-316
cmd/mailing-list-api/service/service_payload_converters.go (1)
21-38: Normalize timestamps to UTC for consistency.Creation paths use time.Now(); other paths use UTC. Prefer UTC everywhere.
- now := time.Now() + now := time.Now().UTC()cmd/mailing-list-api/service/service_validators.go (4)
21-45: Disallow weak ETags for If-Match.If-Match requires strong validators; accepting W/ weak ETags can relax concurrency guarantees.
Apply this diff to reject weak ETags:
- // Handle weak ETags: W/"123" -> "123" - if strings.HasPrefix(raw, "W/") || strings.HasPrefix(raw, "w/") { - raw = strings.TrimSpace(raw[2:]) - } + // Reject weak ETags for write ops (If-Match requires strong validators) + if strings.HasPrefix(raw, "W/") || strings.HasPrefix(raw, "w/") { + return 0, errors.NewValidation("weak ETag (W/) is not allowed for If-Match; use a strong ETag") + }
306-315: Return 503 when serviceReader is unavailable.Reader absence is an infra/state issue, not a client validation error.
Apply this diff:
- return errors.NewValidation("cannot change parent service") + return errors.NewServiceUnavailable("parent service change validation is temporarily unavailable")
318-325: Preserve root cause in error response (wrapped).Wrap the retrieval error to aid diagnostics while keeping a validation surface message.
Apply this diff:
- return errors.NewValidation("new parent service not found") + return errors.NewValidation("new parent service not found", err)
346-350: Use slog.DebugContext for consistency and to carry request context.Apply this diff:
- slog.Debug("committee change detected - member sync required", "mailing_list_uid", existing.UID) + slog.DebugContext(ctx, "committee change detected - member sync required", "mailing_list_uid", existing.UID)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (10)
gen/http/mailing_list/client/cli.gois excluded by!**/gen/**gen/http/mailing_list/client/encode_decode.gois excluded by!**/gen/**gen/http/mailing_list/client/types.gois excluded by!**/gen/**gen/http/mailing_list/server/server.gois excluded by!**/gen/**gen/http/mailing_list/server/types.gois excluded by!**/gen/**gen/http/openapi.jsonis excluded by!**/gen/**gen/http/openapi.yamlis excluded by!**/gen/**gen/http/openapi3.jsonis excluded by!**/gen/**gen/http/openapi3.yamlis excluded by!**/gen/**gen/mailing_list/service.gois excluded by!**/gen/**
📒 Files selected for processing (7)
cmd/mailing-list-api/design/mailing_list.go(6 hunks)cmd/mailing-list-api/design/type.go(0 hunks)cmd/mailing-list-api/service/mailing_list_validators_test.go(12 hunks)cmd/mailing-list-api/service/service_payload_converters.go(3 hunks)cmd/mailing-list-api/service/service_payload_converters_test.go(9 hunks)cmd/mailing-list-api/service/service_validators.go(4 hunks)cmd/mailing-list-api/service/service_validators_test.go(7 hunks)
💤 Files with no reviewable changes (1)
- cmd/mailing-list-api/design/type.go
🧰 Additional context used
📓 Path-based instructions (4)
cmd/mailing-list-api/service/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place GOA service implementations under cmd/mailing-list-api/service/
Files:
cmd/mailing-list-api/service/service_validators_test.gocmd/mailing-list-api/service/mailing_list_validators_test.gocmd/mailing-list-api/service/service_payload_converters.gocmd/mailing-list-api/service/service_validators.gocmd/mailing-list-api/service/service_payload_converters_test.go
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use custom error constructors from pkg/errors (e.g., NewServiceUnavailable, NewUnexpected) for error handling
Use slog for structured logging throughout the codebase
Propagate request-scoped data via context.Context; use context keys from pkg/constants/context.go
Use storage-related constants from pkg/constants/storage.go instead of hardcoded strings
Files:
cmd/mailing-list-api/service/service_validators_test.gocmd/mailing-list-api/design/mailing_list.gocmd/mailing-list-api/service/mailing_list_validators_test.gocmd/mailing-list-api/service/service_payload_converters.gocmd/mailing-list-api/service/service_validators.gocmd/mailing-list-api/service/service_payload_converters_test.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Name unit tests with *_test.go alongside source files
Write integration tests using testify/suite patterns
Files:
cmd/mailing-list-api/service/service_validators_test.gocmd/mailing-list-api/service/mailing_list_validators_test.gocmd/mailing-list-api/service/service_payload_converters_test.go
cmd/mailing-list-api/design/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
After modifying GOA design files, run
make apigen
Files:
cmd/mailing-list-api/design/mailing_list.go
🧬 Code graph analysis (4)
cmd/mailing-list-api/service/service_validators_test.go (1)
gen/mailing_list/service.go (1)
UpdateGrpsioServicePayload(607-643)
cmd/mailing-list-api/design/mailing_list.go (1)
cmd/mailing-list-api/design/type.go (5)
GrpsIOServiceBaseAttributes(11-57)WritersAttribute(142-146)AuditorsAttribute(149-153)GrpsIOMailingListBaseAttributes(249-296)GrpsIOMemberUpdateAttributes(493-530)
cmd/mailing-list-api/service/service_payload_converters.go (4)
internal/domain/model/grpsio_service.go (1)
GrpsIOService(19-39)internal/domain/model/grpsio_mailing_list.go (1)
GrpsIOMailingList(21-45)gen/mailing_list/service.go (1)
UpdateGrpsioMailingListPayload(572-603)internal/domain/model/grpsio_member.go (1)
GrpsIOMember(21-52)
cmd/mailing-list-api/service/service_validators.go (1)
pkg/errors/client.go (1)
NewValidation(24-31)
🪛 GitHub Actions: Mailing List Service Build
cmd/mailing-list-api/service/service_validators_test.go
[error] 479-479: TestValidateUpdateImmutabilityConstraints: valid_update_with_mutable_fields_only failed due to immutability violation: attempted to change immutable field 'domain' from 'example.groups.io' to '' (Step: make test)
cmd/mailing-list-api/service/service_payload_converters_test.go
[error] 310-310: UpdatePayloadToDomain: complete_update_payload_conversion failed. Expected field 'primary' to be populated, but got empty. (Step: make test)
[error] 319-319: UpdatePayloadToDomain: complete_update_payload_conversion failed. Expected field 'project_uid' to be 'project-123', but got empty. (Step: make test)
[error] 320-320: UpdatePayloadToDomain: complete_update_payload_conversion failed. Expected field 'url' to be 'https://example.groups.io/g/test', but got empty. (Step: make test)
[error] 321-321: UpdatePayloadToDomain: complete_update_payload_conversion failed. Expected field 'group' to be 'test-group', but got empty. (Step: make test)
[error] 310-310: UpdatePayloadToDomain: minimal_update_payload_conversion failed. Expected field 'formation' to be populated, but got empty. (Step: make test)
[error] 470-476: MemberUpdatePayloadToDomain: partial_update_-_only_name_fields failed; multiple fields empty during conversion (DeliveryMode, ModStatus, and delivery-related fields). Expected preserved values but got empty. (Step: make test)
[error] 600-606: ServiceUpdatePayloadToDomain: partial_update_-_only_status,_preserve_public_field failed; required fields were not preserved when payload lacked them. (Step: make test)
[error] 725-730: MailingListUpdatePayloadToDomain: partial_update_-_only_title/partial_update-_only_public_field failed; expected existing group details to be preserved, but got empty. (Step: make test)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: MegaLinter
🔇 Additional comments (16)
cmd/mailing-list-api/service/mailing_list_validators_test.go (2)
103-109: SubjectTag remains pointer-backed; good.Usage with stringPtr matches gen types. No change needed.
Also applies to: 119-127
480-531: Subject tag validator coverage is solid.Good mix of allowed/forbidden patterns; aligns with helper behavior.
cmd/mailing-list-api/service/service_validators_test.go (2)
766-771: Mocks and cross‑project checks look good.Test setup exercises parent lookup and project boundary validation correctly.
Also applies to: 780-786, 800-806, 820-831, 835-846, 850-861
124-159: ETag parsing tests are comprehensive.Coverage for weak/quoted/trimmed forms is good.
cmd/mailing-list-api/service/service_payload_converters.go (1)
104-129: Mailing list update converter is fine.Preserves readonly fields and updates mutable ones as expected.
cmd/mailing-list-api/service/service_payload_converters_test.go (1)
366-372: Member update tests: preservation semantics.With the converter fix, DeliveryMode/ModStatus and other optional fields will be preserved unless provided. Tests’ expectations are correct.
Run tests after converter change to confirm preservation works as intended.
Also applies to: 389-393, 413-420, 441-447
cmd/mailing-list-api/design/mailing_list.go (1)
147-158: PUT routes look correct.Method/paths/headers align with objectives.
Also applies to: 282-293, 422-434
cmd/mailing-list-api/service/service_validators.go (9)
160-166: Immutable: type and project_uid checks look correct.
278-281: LGTM: group_name immutability enforced.
285-291: LGTM: main group must be announcement and public.
294-296: LGTM: prevent switching to custom type.
301-303: LGTM: block private→public visibility flips.
328-336: LGTM: block cross‑project service moves.
338-343: LGTM: allow intra‑project service reassignment with audit log.
355-360: LGTM: subject tag validated only when provided.
424-432: Do not remove — contains() is used.
- Referenced by: cmd/mailing-list-api/service/service_validators_test.go:711.
- There's a separate contains implementation in internal/domain/model/grpsio_mailing_list.go (defined ~lines 287–289, used at 284 and 140).
- Action: keep the service-local function or extract a single shared helper and update callers if you want to deduplicate.
Likely an incorrect or invalid review comment.
Assisted by [Claude Code](https://claude.ai/code) Signed-off-by: Prabodh Chaudhari <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (9)
cmd/mailing-list-api/service/service_payload_converters_test.go (9)
233-239: Update tests to reflect PUT semantics: include required fields (Type, ProjectUID) in service updates.Without Type/ProjectUID, the converter sets zero values which may cause failures. Provide them explicitly (carry over from existing for partial updates).
payload: &mailinglistservice.UpdateGrpsioServicePayload{ UID: stringPtr("service-123"), + Type: "primary", + ProjectUID: "project-123", Status: stringPtr("inactive"), GlobalOwners: []string{"[email protected]"}, Public: true,
272-276: Service update test missing required fields.payload: &mailinglistservice.UpdateGrpsioServicePayload{ UID: stringPtr("service-456"), + Type: "formation", + ProjectUID: "project-456", Public: false,
383-389: Service update test missing required fields for PUT semantics.payload: &mailinglistservice.UpdateGrpsioServicePayload{ + Type: "primary", + ProjectUID: "project-123", Status: stringPtr("active"), GlobalOwners: []string{"[email protected]"}, Public: true,
394-397: Service update test missing required ProjectUID field.payload: &mailinglistservice.UpdateGrpsioServicePayload{ Type: "formation", + ProjectUID: "project-123",
402-405: Service update test missing required Type field.payload: &mailinglistservice.UpdateGrpsioServicePayload{ + Type: "primary", ProjectUID: "different-project",
410-414: Service update tests missing required fields.Apply similar fixes for all other test cases by including both Type and ProjectUID fields from the existing service to maintain PUT semantics.
Also applies to: 419-423, 428-433, 437-441, 446-451
640-644: Mailing list PUT: include all required fields in update test payloads.Design requires group_name, public, type, description, title, service_uid. Add missing fields in "only title" case.
payload: &mailinglistservice.UpdateGrpsioMailingListPayload{ + GroupName: "existing-group", + Public: false, + Type: "discussion_moderated", + Description: "Existing description for the group", Title: "Updated Title", + ServiceUID: "service-123", },
659-663: Mailing list update test missing required fields.payload: &mailinglistservice.UpdateGrpsioMailingListPayload{ + GroupName: "existing-group", Public: true, + Type: "discussion_moderated", + Description: "Existing description for the group", + Title: "Existing Title", + ServiceUID: "service-123", },
701-703: Empty mailing list update test should still include required fields.payload: &mailinglistservice.UpdateGrpsioMailingListPayload{ + GroupName: "existing-group", + Public: false, + Type: "discussion_moderated", + Description: "Existing description for the group", + Title: "Existing Title", + ServiceUID: "service-123", },
🧹 Nitpick comments (1)
cmd/mailing-list-api/service/service_validators_test.go (1)
383-389: Update test payloads to match PUT semantics with complete field sets.Since we've switched to PUT semantics, the update payloads should include all required base attributes, not just the changing fields. Add Type and ProjectUID from the existing service.
payload: &mailinglistservice.UpdateGrpsioServicePayload{ Type: "primary", ProjectUID: "project-123", Status: stringPtr("active"), GlobalOwners: []string{"[email protected]"}, Public: true, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
cmd/mailing-list-api/service/mailing_list_service.go(1 hunks)cmd/mailing-list-api/service/service_payload_converters.go(3 hunks)cmd/mailing-list-api/service/service_payload_converters_test.go(10 hunks)cmd/mailing-list-api/service/service_validators.go(5 hunks)cmd/mailing-list-api/service/service_validators_test.go(8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
cmd/mailing-list-api/service/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place GOA service implementations under cmd/mailing-list-api/service/
Files:
cmd/mailing-list-api/service/mailing_list_service.gocmd/mailing-list-api/service/service_validators_test.gocmd/mailing-list-api/service/service_payload_converters_test.gocmd/mailing-list-api/service/service_payload_converters.gocmd/mailing-list-api/service/service_validators.go
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use custom error constructors from pkg/errors (e.g., NewServiceUnavailable, NewUnexpected) for error handling
Use slog for structured logging throughout the codebase
Propagate request-scoped data via context.Context; use context keys from pkg/constants/context.go
Use storage-related constants from pkg/constants/storage.go instead of hardcoded strings
Files:
cmd/mailing-list-api/service/mailing_list_service.gocmd/mailing-list-api/service/service_validators_test.gocmd/mailing-list-api/service/service_payload_converters_test.gocmd/mailing-list-api/service/service_payload_converters.gocmd/mailing-list-api/service/service_validators.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Name unit tests with *_test.go alongside source files
Write integration tests using testify/suite patterns
Files:
cmd/mailing-list-api/service/service_validators_test.gocmd/mailing-list-api/service/service_payload_converters_test.go
🧠 Learnings (1)
📚 Learning: 2025-09-04T08:39:23.930Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-mailing-list-service#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-04T08:39:23.930Z
Learning: Applies to cmd/mailing-list-api/service/** : Place GOA service implementations under cmd/mailing-list-api/service/
Applied to files:
cmd/mailing-list-api/service/mailing_list_service.go
🧬 Code graph analysis (4)
cmd/mailing-list-api/service/service_validators_test.go (2)
gen/mailing_list/service.go (1)
UpdateGrpsioServicePayload(607-643)internal/domain/model/grpsio_service.go (1)
GrpsIOService(19-39)
cmd/mailing-list-api/service/service_payload_converters_test.go (4)
gen/mailing_list/service.go (3)
UpdateGrpsioMailingListMemberPayload(543-568)UpdateGrpsioServicePayload(607-643)UpdateGrpsioMailingListPayload(572-603)internal/domain/model/grpsio_member.go (1)
GrpsIOMember(21-52)internal/domain/model/grpsio_service.go (1)
GrpsIOService(19-39)internal/domain/model/grpsio_mailing_list.go (1)
GrpsIOMailingList(21-45)
cmd/mailing-list-api/service/service_payload_converters.go (3)
internal/domain/model/grpsio_service.go (1)
GrpsIOService(19-39)internal/domain/model/grpsio_mailing_list.go (1)
GrpsIOMailingList(21-45)gen/mailing_list/service.go (1)
UpdateGrpsioMailingListPayload(572-603)
cmd/mailing-list-api/service/service_validators.go (2)
pkg/errors/client.go (1)
NewValidation(24-31)internal/domain/model/grpsio_service.go (1)
GrpsIOService(19-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: MegaLinter
🔇 Additional comments (8)
cmd/mailing-list-api/service/mailing_list_service.go (1)
154-162: Enhanced post-conversion validation properly enforces business rules.Good implementation of the post-conversion business rule validation for PUT semantics. This correctly prevents clearing required fields like
GlobalOwnersfor primary services.cmd/mailing-list-api/service/service_validators_test.go (1)
890-955: Well-structured post-conversion business rule validation tests.The new test suite
TestValidateServiceBusinessRulesprovides comprehensive coverage for the critical business rule that primary services must have at least one global owner. This properly validates the enhanced validation logic that runs after PUT conversion.cmd/mailing-list-api/service/service_payload_converters.go (3)
79-102: Service update correctly preserves all immutable fields and applies PUT semantics.The implementation properly:
- Preserves all immutable fields from existing service (Type, Domain, GroupID, URL, GroupName, etc.)
- Applies PUT semantics for mutable fields where nil values clear to empty/zero values
- Maintains ProjectUID as immutable despite it being in the payload
107-131: Mailing list update correctly handles immutable fields and PUT replacement.The converter properly preserves immutable fields (UID, GroupName, ProjectUID, etc.) while applying PUT semantics to all mutable fields.
177-201: Member update converter implements PUT semantics correctly.The implementation properly preserves all immutable fields while applying complete replacement for mutable profile fields.
cmd/mailing-list-api/service/service_validators.go (3)
168-191: Immutability validation correctly guards optional fields.The validation now properly checks optional fields only when explicitly provided in the payload, avoiding false failures when fields are omitted. This is the correct approach for PUT semantics.
541-554: Essential post-conversion business rule validation.The
validateServiceBusinessRulesfunction correctly enforces that primary services must have at least one global owner, preventing PUT updates from violating this critical constraint.
273-276: ```shell
#!/bin/bashSearch for mailing-list update flows and ServiceUID usage
echo "PWD: $(pwd)"
echo "---- Searching for 'MailingList' occurrences ----"
rg -n --hidden --no-ignore --type go 'MailingList' -C3 || trueecho "---- Searching for mailing-list file names/keywords ----"
rg -n --hidden --no-ignore --type go 'mailing[-_ ]?list|mailinglist' -C3 || trueecho "---- Searching for UpdateMailingList function names ----"
rg -n --hidden --no-ignore --type go '\bUpdate\wMailingList\b' -C3 || trueecho "---- Searching for 'ServiceUID' usages ----"
rg -n --hidden --no-ignore --type go 'ServiceUID' -C3 || trueecho "---- Searching for 'ServiceID' usages ----"
rg -n --hidden --no-ignore --type go '\bServiceID\b' -C3 || trueecho "---- Searching for 'GroupName' usages ----"
rg -n --hidden --no-ignore --type go 'GroupName' -C3 || trueecho "---- Searching for 'orchestrator' references ----"
rg -n --hidden --no-ignore 'orchestrator' -S -C3 || trueecho "---- Done ----"
</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Issue - https://linuxfoundation.atlassian.net/browse/LFXV2-512
🧪 LFX V2 Mailing List Service - PUT Endpoint Testing Evidence
Branch: LFXV2-512 | PUT Endpoints Comprehensive Testing
🏆 Test Results Summary
📋 Test Data Setup
Existing Project Used
{ "uid": "c1e7ce27-1952-424c-a999-4432f54dcfb0", "slug": "ml-test-project-1756812310", "name": "Mailing List Test Project 1756812310", "description": "Test project for mailing list integration testing", "public": false }🔧 PUT Endpoint Testing Results
Test 1: PUT GroupsIO Service Update ✅
Initial State Query
Request:
Response:
Immutable Field Protection Test ✅
Request:
Response:
{ "message": "field 'url' is immutable. Cannot change from 'https://lists.ml-test.org' to 'https://lists.ml-test.org/updated'" }Status: ✅ PASS (Immutable field protection working correctly)
Successful Service Update ✅
Request:
Response:
{ "uid": "684ea133-ce2f-4fbe-9a68-9751ef419f65", "type": "primary", "domain": "lists.ml-test.org", "group_id": 55555, "status": "active", "global_owners": ["[email protected]", "[email protected]"], "prefix": "", "project_slug": "ml-test-project-1756812310", "project_uid": "c1e7ce27-1952-424c-a999-4432f54dcfb0", "url": "https://lists.ml-test.org", "group_name": "ml-test-primary", "public": false, "project_name": "Mailing List Test Project 1756812310", "created_at": "2025-09-12T10:45:50Z", "updated_at": "2025-09-17T04:59:05Z" }Changes Confirmed:
"created"→"active"["[email protected]"]→["[email protected]", "[email protected]"]true→false"2025-09-12T10:45:50Z"→"2025-09-17T04:59:05Z"Status: ✅ PASS
Test 2: PUT GroupsIO Mailing List Update ✅
Initial State Query
Request:
Response:
Successful Mailing List Update ✅
Request:
Response:
{ "uid": "760d34ee-4810-4b35-b9a9-a8c0b7727df5", "group_name": "announce", "public": false, "type": "announcement", "description": "Updated: Official announcements for the ML Test Project - critical project news and important updates", "title": "Updated ML Test Announcements Channel", "service_uid": "684ea133-ce2f-4fbe-9a68-9751ef419f65", "project_uid": "c1e7ce27-1952-424c-a999-4432f54dcfb0", "project_name": "Mailing List Test Project 1756812310", "project_slug": "ml-test-project-1756812310", "created_at": "2025-09-12T10:46:27Z", "updated_at": "2025-09-17T05:00:03Z", "writers": ["[email protected]", "[email protected]"], "auditors": ["[email protected]", "[email protected]"] }Changes Confirmed:
true→false"ML Test Announcements"→"Updated ML Test Announcements Channel"["[email protected]"]→["[email protected]", "[email protected]"]["[email protected]"]→["[email protected]", "[email protected]"]"2025-09-12T10:46:27Z"→"2025-09-17T05:00:03Z"Status: ✅ PASS
Test 3: PUT GroupsIO Member Update ✅
Member Creation for Testing ✅
Request:
Response:
{ "uid": "1f46afe6-0aec-43d3-b3bc-75141b4ece27", "mailing_list_uid": "760d34ee-4810-4b35-b9a9-a8c0b7727df5", "first_name": "Test", "last_name": "Member", "email": "[email protected]", "organization": "PUT Test Corp", "job_title": "Test Engineer", "member_type": "direct", "delivery_mode": "normal", "mod_status": "none", "status": "normal", "created_at": "2025-09-17T05:00:17Z", "updated_at": "2025-09-17T05:00:17Z" }Initial Member State Query
Request:
Response:
Successful Member Update ✅
Request:
Response:
{ "uid": "1f46afe6-0aec-43d3-b3bc-75141b4ece27", "mailing_list_uid": "760d34ee-4810-4b35-b9a9-a8c0b7727df5", "username": "updated_test_user", "first_name": "Updated Test", "last_name": "PUT Member", "email": "[email protected]", "organization": "Updated PUT Test Corporation", "job_title": "Senior Test Engineer", "member_type": "direct", "delivery_mode": "digest", "mod_status": "moderator", "status": "normal", "created_at": "2025-09-17T05:00:17Z", "updated_at": "2025-09-17T05:00:43Z" }Changes Confirmed:
"Test"→"Updated Test""Member"→"PUT Member""Test Engineer"→"Senior Test Engineer""PUT Test Corp"→"Updated PUT Test Corporation""normal"→"digest""none"→"moderator"""→"updated_test_user""[email protected]""2025-09-17T05:00:17Z"→"2025-09-17T05:00:43Z"Status: ✅ PASS
🔑 OpenFGA Authorization Evidence
Service Update OpenFGA Messages ✅
Service Messages Published:
{ "time": "2025-09-17T04:59:05.922588636Z", "level": "DEBUG", "msg": "message published successfully", "subject": "lfx.index.groupsio_service", "message_type": "indexer", "message_size": 879, "X-Request-Id": "c6b7580c-e910-4174-a681-eae840d0db4d" }{ "time": "2025-09-17T04:59:05.922351261Z", "level": "DEBUG", "msg": "message published successfully", "subject": "lfx.update_access.groupsio_service", "message_type": "access", "message_size": 223, "X-Request-Id": "c6b7580c-e910-4174-a681-eae840d0db4d" }Mailing List Update OpenFGA Messages ✅
Mailing List Messages Published:
{ "time": "2025-09-17T05:00:03.951706195Z", "level": "DEBUG", "msg": "message published successfully", "subject": "lfx.index.groupsio_mailing_list", "message_type": "indexer", "message_size": 1155, "X-Request-Id": "315f1ec3-b5b9-4a71-9782-e5cb0c744a61" }{ "time": "2025-09-17T05:00:03.952113029Z", "level": "DEBUG", "msg": "message published successfully", "subject": "lfx.update_access.groupsio_mailing_list", "message_type": "access", "message_size": 296, "X-Request-Id": "315f1ec3-b5b9-4a71-9782-e5cb0c744a61" }Member Update Messages ✅
Member Messages Published (Indexer Only per LFXV2-459):
{ "time": "2025-09-17T05:00:43.274873738Z", "level": "DEBUG", "msg": "message published successfully", "subject": "lfx.index.groupsio_member", "message_type": "indexer", "message_size": 930, "X-Request-Id": "05c27b70-190c-4598-a885-ba69752a744d" }OpenFGA Integration Confirmed:
📡 Indexer Message Publishing Evidence
Message Publishing Success Logs ✅
Service Update Indexer
{ "time": "2025-09-17T04:59:05.922627719Z", "level": "DEBUG", "msg": "service messages published successfully", "service_id": "684ea133-ce2f-4fbe-9a68-9751ef419f65", "action": "updated", "X-Request-Id": "c6b7580c-e910-4174-a681-eae840d0db4d" }Mailing List Update Indexer
{ "time": "2025-09-17T05:00:03.95232457Z", "level": "INFO", "msg": "successfully updated mailing list", "mailing_list_uid": "760d34ee-4810-4b35-b9a9-a8c0b7727df5", "revision": 63, "X-Request-Id": "315f1ec3-b5b9-4a71-9782-e5cb0c744a61" }Member Update Indexer
{ "time": "2025-09-17T05:00:43.274916196Z", "level": "DEBUG", "msg": "messages published successfully", "member_uid": "1f46afe6-0aec-43d3-b3bc-75141b4ece27", "action": "updated", "X-Request-Id": "05c27b70-190c-4598-a885-ba69752a744d" }NATS Subject Publishing Confirmed ✅
lfx.index.groupsio_serviceandlfx.update_access.groupsio_servicelfx.index.groupsio_mailing_listandlfx.update_access.groupsio_mailing_listlfx.index.groupsio_member(indexer only)🔍 OpenSearch Record Verification
Updated Service Record in OpenSearch ✅
Query: Retrieved from
http://lfx-platform-opensearch.lfx.svc.cluster.local:9200/resourcesRecord Found:
{ "object_ref": "groupsio_service:684ea133-ce2f-4fbe-9a68-9751ef419f65", "object_type": "groupsio_service", "object_id": "684ea133-ce2f-4fbe-9a68-9751ef419f65", "parent_refs": [ "project:c1e7ce27-1952-424c-a999-4432f54dcfb0" ], "tags": [ "project_uid:c1e7ce27-1952-424c-a999-4432f54dcfb0", "project_slug:ml-test-project-1756812310", "684ea133-ce2f-4fbe-9a68-9751ef419f65", "service_uid:684ea133-ce2f-4fbe-9a68-9751ef419f65", "service_type:primary" ], "access_check_object": "groupsio_service:684ea133-ce2f-4fbe-9a68-9751ef419f65", "access_check_relation": "viewer", "latest": true, "updated_at": "2025-09-17T04:59:05.943173141Z", "data": { "global_owners": [ "[email protected]", "[email protected]" ], "status": "active", "public": false } }Verification Confirmed:
2025-09-17T04:59:05.943173141Z[email protected]and[email protected]"active"falseUpdated Mailing List Record in OpenSearch ✅
Record Found:
{ "object_ref": "groupsio_mailing_list:760d34ee-4810-4b35-b9a9-a8c0b7727df5", "object_type": "groupsio_mailing_list", "object_id": "760d34ee-4810-4b35-b9a9-a8c0b7727df5", "parent_refs": [ "service:684ea133-ce2f-4fbe-9a68-9751ef419f65", "project:c1e7ce27-1952-424c-a999-4432f54dcfb0" ], "sort_name": "Updated ML Test Announcements Channel", "name_and_aliases": [ "Updated ML Test Announcements Channel" ], "tags": [ "project_uid:c1e7ce27-1952-424c-a999-4432f54dcfb0", "service_uid:684ea133-ce2f-4fbe-9a68-9751ef419f65", "type:announcement", "public:false", "groupsio_mailing_list_uid:760d34ee-4810-4b35-b9a9-a8c0b7727df5", "group_name:announce" ], "latest": true, "updated_at": "2025-09-17T05:00:03.954054362Z", "data": { "title": "Updated ML Test Announcements Channel", "description": "Updated: Official announcements for the ML Test Project - critical project news and important updates", "public": false, "auditors": [ "[email protected]", "[email protected]" ], "writers": [ "[email protected]", "[email protected]" ] } }Verification Confirmed:
2025-09-17T05:00:03.954054362Z"Updated ML Test Announcements Channel"false🧩 ETag-Based Optimistic Concurrency Control Evidence
ETag Usage Verification ✅
All PUT requests successfully used ETag headers for optimistic concurrency control:
If-Match: "12"- Successfully processedIf-Match: "47"- Successfully processedIf-Match: "52"- Successfully processedRevision Increment Evidence ✅
From Service Logs:
266353🧬 Pure PUT Semantics Validation (Branch LFXV2-512)
PUT Semantics Test Results ✅
Test 1: Service PUT with Field Clearing ✅
Created Test Service:
{ "uid": "9555c630-b6b5-4e3a-9755-fd9bc3e35395", "type": "formation", "writers": ["[email protected]", "[email protected]"], "auditors": ["[email protected]"], "public": false }PUT Request (ETag: 28):
{ "type": "formation", "project_uid": "c1e7ce27-1952-424c-a999-4432f54dcfb0", "public": true }Result - Pure PUT Semantics Confirmed ✅:
{ "uid": "9555c630-b6b5-4e3a-9755-fd9bc3e35395", "writers": null, // ✅ CLEARED - not provided in payload "auditors": null, // ✅ CLEARED - not provided in payload "public": true, // ✅ UPDATED - provided in payload "updated_at": "2025-09-17T08:58:54Z" }Test 2: Mailing List PUT with Field Clearing ✅
Created Test Mailing List:
{ "uid": "a928f565-dedd-4c51-9d62-fb8e50ab23ec", "writers": ["[email protected]"], "auditors": ["[email protected]"], "subject_tag": "[PUT-TEST]", "public": true, "type": "discussion_moderated" }PUT Request (ETag: 65):
{ "group_name": "put-test-list", "public": false, "type": "announcement", "description": "Updated description", "title": "Updated Title", "service_uid": "9555c630-b6b5-4e3a-9755-fd9bc3e35395" }Result - Pure PUT Semantics Confirmed ✅:
{ "uid": "a928f565-dedd-4c51-9d62-fb8e50ab23ec", "writers": null, // ✅ CLEARED - not provided "auditors": null, // ✅ CLEARED - not provided "subject_tag": null, // ✅ CLEARED - not provided "public": false, // ✅ UPDATED - provided in payload "type": "announcement", // ✅ UPDATED - provided in payload "description": "Updated description", // ✅ UPDATED "title": "Updated Title", // ✅ UPDATED "updated_at": "2025-09-17T08:59:36Z" }Test 3: Member PUT with Field Clearing ✅
Created Test Member:
{ "uid": "830463ef-fa87-4c92-9941-684730bed71a", "username": "puttestuser", "organization": "Test Corp", "job_title": "Software Engineer", "delivery_mode": "normal", "mod_status": "none" }PUT Request (ETag: 55):
{ "first_name": "Updated", "last_name": "Name", "delivery_mode": "digest", "mod_status": "moderator" }Result - Pure PUT Semantics Confirmed ✅:
{ "uid": "830463ef-fa87-4c92-9941-684730bed71a", "username": "", // ✅ CLEARED - not provided "organization": "", // ✅ CLEARED - not provided "job_title": "", // ✅ CLEARED - not provided "first_name": "Updated", // ✅ UPDATED - provided "last_name": "Name", // ✅ UPDATED - provided "delivery_mode": "digest", // ✅ UPDATED - provided "mod_status": "moderator", // ✅ UPDATED - provided "email": "[email protected]", // ✅ IMMUTABLE - preserved "updated_at": "2025-09-17T09:00:13Z" }Test 4: Business Rule Validation with PUT ✅
Created Primary Service:
{ "uid": "906cd68c-456d-43ae-bb94-8a2df2e9b467", "type": "primary", "global_owners": ["[email protected]"] }PUT Request Attempting to Clear global_owners (ETag: 31):
{ "type": "primary", "project_uid": "b9c32526-2e2b-471e-a710-e2aab6540b44", "public": false }Result - Business Rule Protection ✅:
{ "message": "global_owners must contain at least one email address for primary service type" }Valid PUT Request with global_owners:
{ "type": "primary", "project_uid": "b9c32526-2e2b-471e-a710-e2aab6540b44", "public": false, "global_owners": ["[email protected]", "[email protected]"] }Result - Valid Update Successful ✅:
{ "uid": "906cd68c-456d-43ae-bb94-8a2df2e9b467", "global_owners": ["[email protected]", "[email protected]"], "public": false, // ✅ UPDATED "updated_at": "2025-09-17T09:01:01Z" }PUT Semantics Implementation Verification ✅
Key Validation Points ✅