diff --git a/internal/handler/api/v1/inbox_member_handler.go b/internal/handler/api/v1/inbox_member_handler.go index e4bba7d0..8b46df1f 100644 --- a/internal/handler/api/v1/inbox_member_handler.go +++ b/internal/handler/api/v1/inbox_member_handler.go @@ -4,6 +4,7 @@ import ( "net/http" "github.com/gin-gonic/gin" + "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/service" ) @@ -35,10 +36,7 @@ func (h *InboxMemberHandler) ListMembers(c *gin.Context) { return } - c.JSON(http.StatusOK, gin.H{ - "members": members, - "meta": gin.H{"count": len(members)}, - }) + c.JSON(http.StatusOK, inboxMembersPayload(members)) } // AddMember assigns an agent to an inbox (seat assignment). @@ -66,7 +64,7 @@ func (h *InboxMemberHandler) AddMember(c *gin.Context) { return } - c.JSON(http.StatusCreated, member) + c.JSON(http.StatusCreated, inboxMembersPayload([]model.InboxMember{*member})) } // RemoveMember removes an agent from an inbox (unassign seat). @@ -121,7 +119,7 @@ func (h *InboxMemberHandler) UpdateMember(c *gin.Context) { return } - c.JSON(http.StatusOK, member) + c.JSON(http.StatusOK, inboxMembersPayload([]model.InboxMember{*member})) } // UpdateMultiple replaces all members of an inbox with a new set of user IDs (batch seat assignment). @@ -147,10 +145,7 @@ func (h *InboxMemberHandler) UpdateMultiple(c *gin.Context) { return } - c.JSON(http.StatusOK, gin.H{ - "members": members, - "meta": gin.H{"count": len(members)}, - }) + c.JSON(http.StatusOK, inboxMembersPayload(members)) } // ShowAccountScoped retrieves all agents assigned to an inbox using Chatwoot's account-level route. @@ -168,13 +163,25 @@ func (h *InboxMemberHandler) ShowAccountScoped(c *gin.Context) { return } - c.JSON(http.StatusOK, gin.H{"members": members, "meta": gin.H{"count": len(members)}}) + c.JSON(http.StatusOK, inboxMembersPayload(members)) } // CreateAccountScoped adds or replaces inbox members using Chatwoot's account-level create route. // POST /api/v1/accounts/:account_id/inbox_members func (h *InboxMemberHandler) CreateAccountScoped(c *gin.Context) { - h.updateAccountScoped(c) + var req service.UpdateMultipleRequest + if bindErr := c.ShouldBindJSON(&req); bindErr != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": bindErr.Error()}) + return + } + + members, svcErr := h.svc.AddMembers(c.Request.Context(), req) + if svcErr != nil { + c.JSON(http.StatusUnprocessableEntity, gin.H{"error": "failed to update members"}) + return + } + + c.JSON(http.StatusOK, inboxMembersPayload(members)) } // UpdateAccountScoped replaces all members of an inbox using Chatwoot's account-level update route. @@ -196,7 +203,7 @@ func (h *InboxMemberHandler) updateAccountScoped(c *gin.Context) { return } - c.JSON(http.StatusOK, gin.H{"members": members, "meta": gin.H{"count": len(members)}}) + c.JSON(http.StatusOK, inboxMembersPayload(members)) } // DestroyAccountScoped removes selected users from an inbox using Chatwoot's account-level route. @@ -220,3 +227,51 @@ func (h *InboxMemberHandler) DestroyAccountScoped(c *gin.Context) { c.Status(http.StatusOK) } + +func inboxMembersPayload(members []model.InboxMember) gin.H { + payload := make([]gin.H, 0, len(members)) + for _, member := range members { + payload = append(payload, serializeInboxMemberAgent(member)) + } + return gin.H{"payload": payload} +} + +func serializeInboxMemberAgent(member model.InboxMember) gin.H { + user := member.User + availableName := user.DisplayName + if availableName == "" { + availableName = user.Name + } + if availableName == "" { + availableName = user.Email + } + availabilityStatus := member.AvailabilityStatus + if availabilityStatus == "" { + if user.Available { + availabilityStatus = "online" + } else { + availabilityStatus = "offline" + } + } + role := user.Role + if role == "" { + role = member.Role + } + if role == "" { + role = "agent" + } + return gin.H{ + "id": user.ID, + "account_id": member.Inbox.AccountID, + "availability_status": availabilityStatus, + "auto_offline": true, + "confirmed": user.ConfirmedAt != nil, + "email": user.Email, + "provider": user.Provider, + "available_name": availableName, + "name": user.Name, + "role": role, + "thumbnail": user.AvatarURL, + "custom_role_id": user.CustomRoleID, + } +} diff --git a/internal/handler/api/v1/inbox_member_handler_test.go b/internal/handler/api/v1/inbox_member_handler_test.go index 95f2c93b..1df30ea7 100644 --- a/internal/handler/api/v1/inbox_member_handler_test.go +++ b/internal/handler/api/v1/inbox_member_handler_test.go @@ -2,16 +2,19 @@ package v1 import ( "bytes" + "encoding/json" "fmt" "net/http" "net/http/httptest" "testing" + "time" "github.com/gin-gonic/gin" "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/repository" "github.com/gochat/gochat/internal/service" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "gorm.io/driver/sqlite" "gorm.io/gorm" @@ -77,6 +80,10 @@ func (s *InboxMemberHandlerTestSuite) TestListMembers_Success() { r.ServeHTTP(w, req) assert.Equal(s.T(), http.StatusOK, w.Code) + var body map[string]any + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &body)) + s.Require().Contains(body, "payload") + s.Require().NotContains(body, "members") } func (s *InboxMemberHandlerTestSuite) TestAddMember_BadRequest_InvalidInboxID() { @@ -91,6 +98,79 @@ func (s *InboxMemberHandlerTestSuite) TestAddMember_BadRequest_InvalidInboxID() assert.Equal(s.T(), http.StatusBadRequest, w.Code) } +func (s *InboxMemberHandlerTestSuite) TestAccountScopedInboxMembers_ChatwootPayloadAndDiffUpdate() { + s.db.Exec("DELETE FROM inbox_members") + users := []*model.User{ + {AccountID: s.account.ID, Name: "Agent One", DisplayName: "Agent 1", Email: "agent1@example.com", Password: "password", Provider: "email", Role: "agent", ConfirmedAt: ptrTimeNow()}, + {AccountID: s.account.ID, Name: "Agent Two", Email: "agent2@example.com", Password: "password", Provider: "email", Role: "agent"}, + {AccountID: s.account.ID, Name: "Agent Three", Email: "agent3@example.com", Password: "password", Provider: "email", Role: "agent"}, + } + for _, user := range users { + s.Require().NoError(s.db.Create(user).Error) + } + s.Require().NoError(s.db.Create(&model.InboxMember{InboxID: s.inbox.ID, UserID: users[0].ID, Role: "agent", AvailabilityStatus: "online"}).Error) + + r := gin.New() + r.GET("/api/v1/accounts/:account_id/inbox_members/:inbox_id", s.handler.ShowAccountScoped) + r.POST("/api/v1/accounts/:account_id/inbox_members", s.handler.CreateAccountScoped) + r.PATCH("/api/v1/accounts/:account_id/inbox_members", s.handler.UpdateAccountScoped) + r.DELETE("/api/v1/accounts/:account_id/inbox_members", s.handler.DestroyAccountScoped) + + show := httptest.NewRecorder() + req, _ := http.NewRequest("GET", fmt.Sprintf("/api/v1/accounts/%d/inbox_members/%d", s.account.ID, s.inbox.ID), nil) + r.ServeHTTP(show, req) + s.Require().Equal(http.StatusOK, show.Code, show.Body.String()) + payload := inboxMemberPayload(s.T(), show) + s.Require().Len(payload, 1) + s.Require().Equal("Agent 1", payload[0]["available_name"]) + s.Require().Equal("online", payload[0]["availability_status"]) + s.Require().NotContains(payload[0], "inbox_id") + + create := httptest.NewRecorder() + body := fmt.Sprintf(`{"inbox_id":%d,"user_ids":[%d,%d,%d]}`, s.inbox.ID, users[0].ID, users[1].ID, users[1].ID) + req, _ = http.NewRequest("POST", fmt.Sprintf("/api/v1/accounts/%d/inbox_members", s.account.ID), bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(create, req) + s.Require().Equal(http.StatusOK, create.Code, create.Body.String()) + payload = inboxMemberPayload(s.T(), create) + s.Require().Len(payload, 2) + + update := httptest.NewRecorder() + body = fmt.Sprintf(`{"inbox_id":%d,"user_ids":[%d]}`, s.inbox.ID, users[2].ID) + req, _ = http.NewRequest("PATCH", fmt.Sprintf("/api/v1/accounts/%d/inbox_members", s.account.ID), bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(update, req) + s.Require().Equal(http.StatusOK, update.Code, update.Body.String()) + payload = inboxMemberPayload(s.T(), update) + s.Require().Len(payload, 1) + s.Require().Equal(float64(users[2].ID), payload[0]["id"]) + + destroy := httptest.NewRecorder() + body = fmt.Sprintf(`{"inbox_id":%d,"user_ids":[%d]}`, s.inbox.ID, users[2].ID) + req, _ = http.NewRequest("DELETE", fmt.Sprintf("/api/v1/accounts/%d/inbox_members", s.account.ID), bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(destroy, req) + s.Require().Equal(http.StatusOK, destroy.Code, destroy.Body.String()) + + var count int64 + s.Require().NoError(s.db.Model(&model.InboxMember{}).Where("inbox_id = ?", s.inbox.ID).Count(&count).Error) + s.Require().Zero(count) +} + +func inboxMemberPayload(t *testing.T, response *httptest.ResponseRecorder) []map[string]any { + t.Helper() + var body struct { + Payload []map[string]any `json:"payload"` + } + require.NoError(t, json.Unmarshal(response.Body.Bytes(), &body), response.Body.String()) + return body.Payload +} + +func ptrTimeNow() *time.Time { + now := time.Now() + return &now +} + func (s *InboxMemberHandlerTestSuite) TestRemoveMember_BadRequest_InvalidInboxID() { r := gin.New() r.DELETE("/api/v1/accounts/:account_id/inboxes/:inbox_id/members/:user_id", s.handler.RemoveMember) @@ -100,4 +180,4 @@ func (s *InboxMemberHandlerTestSuite) TestRemoveMember_BadRequest_InvalidInboxID r.ServeHTTP(w, req) assert.Equal(s.T(), http.StatusBadRequest, w.Code) -} \ No newline at end of file +} diff --git a/internal/repository/inbox_member_repo.go b/internal/repository/inbox_member_repo.go index e0641ab7..41b20f13 100644 --- a/internal/repository/inbox_member_repo.go +++ b/internal/repository/inbox_member_repo.go @@ -22,7 +22,7 @@ func NewInboxMemberRepo(db *gorm.DB) *InboxMemberRepo { // FindByID retrieves an inbox_member by primary key. func (r *InboxMemberRepo) FindByID(ctx context.Context, id uint) (*model.InboxMember, error) { var im model.InboxMember - err := r.db.WithContext(ctx).First(&im, id).Error + err := r.withAgentPreloads(r.db.WithContext(ctx)).First(&im, id).Error if err != nil { return nil, err } @@ -32,7 +32,7 @@ func (r *InboxMemberRepo) FindByID(ctx context.Context, id uint) (*model.InboxMe // FindByInboxAndUser retrieves an inbox_member by inbox and user. func (r *InboxMemberRepo) FindByInboxAndUser(ctx context.Context, inboxID, userID uint) (*model.InboxMember, error) { var im model.InboxMember - err := r.db.WithContext(ctx).Where("inbox_id = ? AND user_id = ?", inboxID, userID).First(&im).Error + err := r.withAgentPreloads(r.db.WithContext(ctx)).Where("inbox_id = ? AND user_id = ?", inboxID, userID).First(&im).Error if err != nil { return nil, err } @@ -42,7 +42,7 @@ func (r *InboxMemberRepo) FindByInboxAndUser(ctx context.Context, inboxID, userI // FindByInbox retrieves all members (agents) assigned to an inbox. func (r *InboxMemberRepo) FindByInbox(ctx context.Context, inboxID uint) ([]model.InboxMember, error) { var ims []model.InboxMember - err := r.db.WithContext(ctx).Preload("User").Where("inbox_id = ?", inboxID). + err := r.withAgentPreloads(r.db.WithContext(ctx)).Where("inbox_id = ?", inboxID). Order("id ASC").Find(&ims).Error return ims, err } @@ -58,13 +58,17 @@ func (r *InboxMemberRepo) FindByUser(ctx context.Context, userID uint) ([]model. // FindByAccountID retrieves all inbox members for inboxes belonging to a given account. func (r *InboxMemberRepo) FindByAccountID(ctx context.Context, accountID uint) ([]model.InboxMember, error) { var ims []model.InboxMember - err := r.db.WithContext(ctx).Preload("User"). + err := r.withAgentPreloads(r.db.WithContext(ctx)). Joins("JOIN inboxes ON inboxes.id = inbox_members.inbox_id"). Where("inboxes.account_id = ?", accountID). Order("inbox_members.id ASC").Find(&ims).Error return ims, err } +func (r *InboxMemberRepo) withAgentPreloads(db *gorm.DB) *gorm.DB { + return db.Preload("User").Preload("Inbox") +} + // Create inserts a new inbox_member. func (r *InboxMemberRepo) Create(ctx context.Context, im *model.InboxMember) error { return r.db.WithContext(ctx).Create(im).Error @@ -113,4 +117,4 @@ func (r *InboxMemberRepo) UpdateAvailabilityByUser(ctx context.Context, accountI return gorm.ErrRecordNotFound } return nil -} \ No newline at end of file +} diff --git a/internal/service/inbox_member_service.go b/internal/service/inbox_member_service.go index 80cc217c..0e423072 100644 --- a/internal/service/inbox_member_service.go +++ b/internal/service/inbox_member_service.go @@ -2,7 +2,6 @@ package service import ( "context" - "errors" "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/repository" @@ -48,10 +47,9 @@ func (s *InboxMemberService) AddMember(ctx context.Context, req AddMemberRequest return nil, err } - // Check for duplicate existing, err := s.repo.FindByInboxAndUser(ctx, req.InboxID, req.UserID) if err == nil && existing != nil { - return nil, errors.New("user is already a member of this inbox") + return existing, nil } im := &model.InboxMember{ @@ -62,7 +60,7 @@ func (s *InboxMemberService) AddMember(ctx context.Context, req AddMemberRequest applogger.L().Errorf("InboxMemberService.AddMember failed: %v", err) return nil, err } - return im, nil + return s.repo.FindByInboxAndUser(ctx, req.InboxID, req.UserID) } // RemoveMember removes a user from an inbox. @@ -77,7 +75,7 @@ func (s *InboxMemberService) RemoveAllMembers(ctx context.Context, inboxID uint) // UpdateMemberRequest is the DTO for updating a member's role/availability. type UpdateMemberRequest struct { - Role string `json:"role" validate:"omitempty,oneof=agent supervisor"` + Role string `json:"role" validate:"omitempty,oneof=agent supervisor"` AvailabilityStatus string `json:"availability_status" validate:"omitempty,oneof=online offline busy"` } @@ -111,41 +109,79 @@ func (s *InboxMemberService) UpdateMember(ctx context.Context, inboxID, userID u // UpdateMultipleRequest is the DTO for batch-updating inbox members. // Reference: Chatwoot inbox_members_controller#update (batch replace all members) type UpdateMultipleRequest struct { - InboxID uint `json:"inbox_id" validate:"required"` - UserIDs []uint `json:"user_ids" validate:"required,min=1"` + InboxID uint `json:"inbox_id" validate:"required"` + UserIDs []uint `json:"user_ids"` +} + +// AddMembers adds the requested users without removing existing members. +// Reference: Chatwoot inbox_members#create - add_members(agents_to_be_added_ids). +func (s *InboxMemberService) AddMembers(ctx context.Context, req UpdateMultipleRequest) ([]model.InboxMember, error) { + if err := pkgvalidator.ValidateStruct(req); err != nil { + return nil, err + } + + current, err := s.repo.FindByInbox(ctx, req.InboxID) + if err != nil { + return nil, err + } + currentIDs := map[uint]bool{} + for _, member := range current { + currentIDs[member.UserID] = true + } + + for _, userID := range uniqueUintIDs(req.UserIDs) { + if currentIDs[userID] { + continue + } + im := &model.InboxMember{InboxID: req.InboxID, UserID: userID, Role: "agent", AvailabilityStatus: "offline"} + if err := s.repo.Create(ctx, im); err != nil { + applogger.L().Errorf("InboxMemberService.AddMembers: failed to add member user_id=%d: %v", userID, err) + return nil, err + } + } + + return s.repo.FindByInbox(ctx, req.InboxID) } // UpdateMultiple replaces all members of an inbox with the given user list. -// Reference: Chatwoot inbox_members#update — replaces entire member list in one call. -// This is the "seat assignment" batch update: remove all existing, then add new ones. +// Reference: Chatwoot inbox_members#update - replaces entire member list in one call. +// This calculates the add/remove diff instead of blindly recreating rows. func (s *InboxMemberService) UpdateMultiple(ctx context.Context, req UpdateMultipleRequest) ([]model.InboxMember, error) { if err := pkgvalidator.ValidateStruct(req); err != nil { return nil, err } - // Remove all existing members for this inbox - if err := s.RemoveAllMembers(ctx, req.InboxID); err != nil { - applogger.L().Errorf("InboxMemberService.UpdateMultiple: failed to remove existing members: %v", err) + current, err := s.repo.FindByInbox(ctx, req.InboxID) + if err != nil { return nil, err } - - // Add each new member - var members []model.InboxMember - for _, userID := range req.UserIDs { - im := &model.InboxMember{ - InboxID: req.InboxID, - UserID: userID, - Role: "agent", - AvailabilityStatus: "online", + desiredIDs := map[uint]bool{} + for _, userID := range uniqueUintIDs(req.UserIDs) { + desiredIDs[userID] = true + } + currentIDs := map[uint]bool{} + for _, member := range current { + currentIDs[member.UserID] = true + if !desiredIDs[member.UserID] { + if err := s.RemoveMember(ctx, req.InboxID, member.UserID); err != nil { + applogger.L().Errorf("InboxMemberService.UpdateMultiple: failed to remove member user_id=%d: %v", member.UserID, err) + return nil, err + } } - if err := s.repo.Create(ctx, im); err != nil { - applogger.L().Errorf("InboxMemberService.UpdateMultiple: failed to add member user_id=%d: %v", userID, err) - continue - } - members = append(members, *im) } - return members, nil + for userID := range desiredIDs { + if currentIDs[userID] { + continue + } + im := &model.InboxMember{InboxID: req.InboxID, UserID: userID, Role: "agent", AvailabilityStatus: "offline"} + if err := s.repo.Create(ctx, im); err != nil { + applogger.L().Errorf("InboxMemberService.UpdateMultiple: failed to add member user_id=%d: %v", userID, err) + return nil, err + } + } + + return s.repo.FindByInbox(ctx, req.InboxID) } // IsMemberOfInbox checks whether a user is a member (assigned agent) of an inbox. @@ -156,4 +192,17 @@ func (s *InboxMemberService) IsMemberOfInbox(ctx context.Context, inboxID, userI return false } return true -} \ No newline at end of file +} + +func uniqueUintIDs(ids []uint) []uint { + seen := map[uint]bool{} + result := make([]uint, 0, len(ids)) + for _, id := range ids { + if id == 0 || seen[id] { + continue + } + seen[id] = true + result = append(result, id) + } + return result +}