Skip to content

Commit 22ae6f0

Browse files
committed
Fixed the review comments
Assisted by [Claude Code](https://claude.ai/code) Signed-off-by: Prabodh Chaudhari <[email protected]>
1 parent fe612b3 commit 22ae6f0

File tree

4 files changed

+24
-16
lines changed

4 files changed

+24
-16
lines changed

cmd/mailing-list-api/service/service_response_converters.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ func (s *mailingListService) convertGrpsIOMailingListDomainToResponse(ml *model.
137137

138138
// convertGrpsIOMailingListDomainToStandardResponse converts a domain mailing list to GOA standard response type
139139
func (s *mailingListService) convertGrpsIOMailingListDomainToStandardResponse(mailingList *model.GrpsIOMailingList) *mailinglistservice.GrpsIoMailingListWithReadonlyAttributes {
140+
if mailingList == nil {
141+
return &mailinglistservice.GrpsIoMailingListWithReadonlyAttributes{}
142+
}
143+
140144
response := &mailinglistservice.GrpsIoMailingListWithReadonlyAttributes{
141145
UID: &mailingList.UID,
142146
GroupName: &mailingList.GroupName,

cmd/mailing-list-api/service/service_validators.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ func validateMailingListUpdate(ctx context.Context, existing *model.GrpsIOMailin
275275
}
276276

277277
// Validate main group restrictions (critical business rule from Groups.io)
278-
if parentService != nil && existing.IsMainGroup(parentService) {
278+
if parentService != nil && isMainGroupForService(existing, parentService) {
279279
// Main groups must remain public announcement lists
280280
if payload.Type != nil && *payload.Type != "announcement" {
281281
return errors.NewValidation("main group must be an announcement list")
@@ -360,20 +360,8 @@ func validateMailingListUpdate(ctx context.Context, existing *model.GrpsIOMailin
360360
// validateMailingListDeleteProtection validates deletion protection rules
361361
func validateMailingListDeleteProtection(mailingList *model.GrpsIOMailingList, parentService *model.GrpsIOService) error {
362362
// Check if it's a main group (any service type)
363-
if parentService != nil {
364-
isMainGroup := false
365-
366-
switch parentService.Type {
367-
case "primary":
368-
isMainGroup = mailingList.GroupName == parentService.GroupName
369-
case "formation", "shared":
370-
// Formation and shared services use prefix as main group identifier
371-
isMainGroup = mailingList.GroupName == parentService.Prefix
372-
}
373-
374-
if isMainGroup {
375-
return errors.NewValidation(fmt.Sprintf("cannot delete the main group of a %s service", parentService.Type))
376-
}
363+
if parentService != nil && isMainGroupForService(mailingList, parentService) {
364+
return errors.NewValidation(fmt.Sprintf("cannot delete the main group of a %s service", parentService.Type))
377365
}
378366

379367
// Protect announcement lists (typically used for critical communications)
@@ -436,6 +424,18 @@ func contains(slice []string, item string) bool {
436424
return false
437425
}
438426

427+
// isMainGroupForService determines if a list is the "main" group for its parent.
428+
func isMainGroupForService(ml *model.GrpsIOMailingList, svc *model.GrpsIOService) bool {
429+
switch svc.Type {
430+
case "primary":
431+
return ml.GroupName == svc.GroupName
432+
case "formation", "shared":
433+
return ml.GroupName == svc.Prefix
434+
default:
435+
return false
436+
}
437+
}
438+
439439
// validateMemberUpdate validates that immutable fields are not changed during updates
440440
func validateMemberUpdate(existing, updated *model.GrpsIOMember) error {
441441
if existing == nil || updated == nil {

internal/infrastructure/nats/storage.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,10 @@ func (s *storage) UpdateGrpsIOMember(ctx context.Context, uid string, member *mo
805805

806806
// DeleteGrpsIOMember deletes a member with optimistic concurrency control
807807
func (s *storage) DeleteGrpsIOMember(ctx context.Context, uid string, expectedRevision uint64, member *model.GrpsIOMember) error {
808+
if member == nil {
809+
return errs.NewValidation("member is required for deletion")
810+
}
811+
808812
slog.DebugContext(ctx, "nats storage: deleting member",
809813
"member_uid", uid,
810814
"expected_revision", expectedRevision)

pkg/utils/time_helpers.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func ValidateRFC3339(timestamp string) (time.Time, error) {
1818
return time.Time{}, fmt.Errorf(constants.ErrEmptyTimestamp)
1919
}
2020

21-
t, err := time.Parse(constants.TimestampFormat, timestamp)
21+
t, err := time.Parse(time.RFC3339Nano, timestamp)
2222
if err != nil {
2323
return time.Time{}, fmt.Errorf("%s: %w", constants.ErrInvalidTimestampFormat, err)
2424
}

0 commit comments

Comments
 (0)