mgmt API: fix feedbag group order on buddy add/delete, align tests

server/http/mgmt_api.go

@@ -1375,17 +1375,7 @@ func putFeedbagBuddyHandler(w http.ResponseWriter, r *http.Request, buddyBroadca
 		return
 	}
 
-	if order, hasOrder := group.Bytes(wire.FeedbagAttributesOrder); hasOrder {
-		var memberIDs []uint16
-		if err := wire.UnmarshalBE(&memberIDs, bytes.NewReader(order)); err != nil {
-			logger.Error("error decoding order TLV", "err", err.Error())
-			errorMsg(w, "internal server error", http.StatusInternalServerError)
-			return
-		}
-		group.Replace(wire.NewTLVBE(wire.FeedbagAttributesOrder, append(memberIDs, buddyItem.ItemID)))
-	} else {
-		group.Append(wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{buddyItem.ItemID}))
-	}
+	group.AppendOrderMembers(buddyItem.ItemID)
 
 	updates := []wire.FeedbagItem{
 		buddyItem,
@@ -1499,18 +1489,18 @@ func deleteFeedbagBuddyHandler(w http.ResponseWriter, r *http.Request, buddyBroa
 	}
 
 	var itemToDelete *wire.FeedbagItem
-	var groupFound bool
+	var parentGroup *wire.FeedbagItem
 	for _, item := range items {
 		switch {
 		case item.ClassID == wire.FeedbagClassIdGroup && item.GroupID == groupID:
-			groupFound = true
+			parentGroup = &item
 		case item.ClassID == wire.FeedbagClassIdBuddy && item.Name == buddyScreenName && item.GroupID == groupID:
 			itemToDelete = &item
 		}
 	}
 
 	switch {
-	case !groupFound:
+	case parentGroup == nil:
 		errorMsg(w, "group not found", http.StatusNotFound)
 		return
 	case itemToDelete == nil:
@@ -1518,6 +1508,14 @@ func deleteFeedbagBuddyHandler(w http.ResponseWriter, r *http.Request, buddyBroa
 		return
 	}
 
+	// Remove the buddy from the parent group's order TLV (same as TOC feedbag_list.DeleteBuddy).
+	parentGroup.RemoveOrderMembers(itemToDelete.ItemID)
+	if err := feedbagManager.FeedbagUpsert(r.Context(), me, []wire.FeedbagItem{*parentGroup}); err != nil {
+		logger.Error("error updating feedbag group order", "err", err.Error())
+		errorMsg(w, "internal server error", http.StatusInternalServerError)
+		return
+	}
+
 	if err := feedbagManager.FeedbagDelete(r.Context(), me, []wire.FeedbagItem{*itemToDelete}); err != nil {
 		logger.Error("error deleting feedbag item", "err", err.Error())
 		errorMsg(w, "internal server error", http.StatusInternalServerError)
@@ -1536,6 +1534,16 @@ func deleteFeedbagBuddyHandler(w http.ResponseWriter, r *http.Request, buddyBroa
 				Items: []wire.FeedbagItem{*itemToDelete},
 			},
 		})
+		messageRelayer.RelayToScreenName(r.Context(), me, wire.SNACMessage{
+			Frame: wire.SNACFrame{
+				FoodGroup: wire.Feedbag,
+				SubGroup:  wire.FeedbagUpdateItem,
+				RequestID: wire.ReqIDFromServer,
+			},
+			Body: wire.SNAC_0x13_0x09_FeedbagUpdateItem{
+				Items: []wire.FeedbagItem{*parentGroup},
+			},
+		})
 		instances := session.Instances()
 		if len(instances) > 0 {
 			if err := buddyBroadcaster.BroadcastVisibility(r.Context(), instances[0], []state.IdentScreenName{deleteBuddy}, true); err != nil {

server/http/mgmt_api_test.go

@@ -3056,12 +3056,22 @@ func TestFeedbagBuddyHandler_GET(t *testing.T) {
 									ClassID: wire.FeedbagClassIdGroup,
 									Name:    "Friends",
 									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{10, 11}),
+										},
+									},
 								},
 								{
 									ItemID:  2,
 									ClassID: wire.FeedbagClassIdGroup,
 									Name:    "Work",
 									GroupID: 2,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{20}),
+										},
+									},
 								},
 								{
 									ItemID:  10,
@@ -3672,11 +3682,20 @@ func TestFeedbagBuddyHandler_PUT(t *testing.T) {
 						{
 							screenName: state.NewIdentScreenName("userA"),
 							result: func() []wire.FeedbagItem {
+								order := make([]uint16, 30)
+								for i := 1; i <= 30; i++ {
+									order[i-1] = uint16(i)
+								}
 								items := []wire.FeedbagItem{
 									{
 										ClassID: wire.FeedbagClassIdGroup,
 										Name:    "Friends",
 										GroupID: 1,
+										TLVLBlock: wire.TLVLBlock{
+											TLVList: wire.TLVList{
+												wire.NewTLVBE(wire.FeedbagAttributesOrder, order),
+											},
+										},
 									},
 								}
 								// Add 30 buddies to the group
@@ -3873,6 +3892,11 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 									ClassID: wire.FeedbagClassIdGroup,
 									Name:    "Friends",
 									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{10}),
+										},
+									},
 								},
 								{
 									ItemID:  10,
@@ -3884,6 +3908,25 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 							err: nil,
 						},
 					},
+					feedbagUpsertParams: feedbagUpsertParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							items: []wire.FeedbagItem{
+								{
+									ItemID:  1,
+									ClassID: wire.FeedbagClassIdGroup,
+									Name:    "Friends",
+									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{}),
+										},
+									},
+								},
+							},
+							err: nil,
+						},
+					},
 					feedbagDeleteParams: feedbagDeleteParams{
 						{
 							screenName: state.NewIdentScreenName("userA"),
@@ -3909,6 +3952,31 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 				},
 				messageRelayerParams: messageRelayerParams{
 					relayToScreenNameParams: relayToScreenNameParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							msg: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.Feedbag,
+									SubGroup:  wire.FeedbagUpdateItem,
+									RequestID: wire.ReqIDFromServer,
+								},
+								Body: wire.SNAC_0x13_0x09_FeedbagUpdateItem{
+									Items: []wire.FeedbagItem{
+										{
+											ItemID:  1,
+											ClassID: wire.FeedbagClassIdGroup,
+											Name:    "Friends",
+											GroupID: 1,
+											TLVLBlock: wire.TLVLBlock{
+												TLVList: wire.TLVList{
+													wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{}),
+												},
+											},
+										},
+									},
+								},
+							},
+						},
 						{
 							screenName: state.NewIdentScreenName("userA"),
 							msg: wire.SNACMessage{
@@ -3950,6 +4018,11 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 									ClassID: wire.FeedbagClassIdGroup,
 									Name:    "Friends",
 									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{10}),
+										},
+									},
 								},
 								{
 									ItemID:  10,
@@ -3961,6 +4034,25 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 							err: nil,
 						},
 					},
+					feedbagUpsertParams: feedbagUpsertParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							items: []wire.FeedbagItem{
+								{
+									ItemID:  1,
+									ClassID: wire.FeedbagClassIdGroup,
+									Name:    "Friends",
+									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{}),
+										},
+									},
+								},
+							},
+							err: nil,
+						},
+					},
 					feedbagDeleteParams: feedbagDeleteParams{
 						{
 							screenName: state.NewIdentScreenName("userA"),
@@ -3992,6 +4084,31 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 				},
 				messageRelayerParams: messageRelayerParams{
 					relayToScreenNameParams: relayToScreenNameParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							msg: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.Feedbag,
+									SubGroup:  wire.FeedbagUpdateItem,
+									RequestID: wire.ReqIDFromServer,
+								},
+								Body: wire.SNAC_0x13_0x09_FeedbagUpdateItem{
+									Items: []wire.FeedbagItem{
+										{
+											ItemID:  1,
+											ClassID: wire.FeedbagClassIdGroup,
+											Name:    "Friends",
+											GroupID: 1,
+											TLVLBlock: wire.TLVLBlock{
+												TLVList: wire.TLVList{
+													wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{}),
+												},
+											},
+										},
+									},
+								},
+							},
+						},
 						{
 							screenName: state.NewIdentScreenName("userA"),
 							msg: wire.SNACMessage{
@@ -4065,6 +4182,25 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 							err: nil,
 						},
 					},
+					feedbagUpsertParams: feedbagUpsertParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							items: []wire.FeedbagItem{
+								{
+									ItemID:  1,
+									ClassID: wire.FeedbagClassIdGroup,
+									Name:    "Friends",
+									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{20}),
+										},
+									},
+								},
+							},
+							err: nil,
+						},
+					},
 					feedbagDeleteParams: feedbagDeleteParams{
 						{
 							screenName: state.NewIdentScreenName("userA"),
@@ -4090,6 +4226,31 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 				},
 				messageRelayerParams: messageRelayerParams{
 					relayToScreenNameParams: relayToScreenNameParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							msg: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.Feedbag,
+									SubGroup:  wire.FeedbagUpdateItem,
+									RequestID: wire.ReqIDFromServer,
+								},
+								Body: wire.SNAC_0x13_0x09_FeedbagUpdateItem{
+									Items: []wire.FeedbagItem{
+										{
+											ItemID:  1,
+											ClassID: wire.FeedbagClassIdGroup,
+											Name:    "Friends",
+											GroupID: 1,
+											TLVLBlock: wire.TLVLBlock{
+												TLVList: wire.TLVList{
+													wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{20}),
+												},
+											},
+										},
+									},
+								},
+							},
+						},
 						{
 							screenName: state.NewIdentScreenName("userA"),
 							msg: wire.SNACMessage{
@@ -4231,6 +4392,62 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:           "internal server error on group order upsert",
+			screenName:     "userA",
+			groupID:        "1",
+			requestBody:    `{"name":"buddy1"}`,
+			wantStatusCode: http.StatusInternalServerError,
+			wantResponse:   `{"message":"internal server error"}`,
+			mockParams: mockParams{
+				feedbagManagerParams: feedbagManagerParams{
+					feedbagParams: feedbagParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							result: []wire.FeedbagItem{
+								{
+									ItemID:  1,
+									ClassID: wire.FeedbagClassIdGroup,
+									Name:    "Friends",
+									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{10}),
+										},
+									},
+								},
+								{
+									ItemID:  10,
+									ClassID: wire.FeedbagClassIdBuddy,
+									Name:    "buddy1",
+									GroupID: 1,
+								},
+							},
+							err: nil,
+						},
+					},
+					feedbagUpsertParams: feedbagUpsertParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							items: []wire.FeedbagItem{
+								{
+									ItemID:  1,
+									ClassID: wire.FeedbagClassIdGroup,
+									Name:    "Friends",
+									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{}),
+										},
+									},
+								},
+							},
+							err: errors.New("database error"),
+						},
+					},
+				},
+			},
+		},
 		{
 			name:           "internal server error on delete",
 			screenName:     "userA",
@@ -4249,6 +4466,11 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 									ClassID: wire.FeedbagClassIdGroup,
 									Name:    "Friends",
 									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{10}),
+										},
+									},
 								},
 								{
 									ItemID:  10,
@@ -4260,6 +4482,25 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 							err: nil,
 						},
 					},
+					feedbagUpsertParams: feedbagUpsertParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							items: []wire.FeedbagItem{
+								{
+									ItemID:  1,
+									ClassID: wire.FeedbagClassIdGroup,
+									Name:    "Friends",
+									GroupID: 1,
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{}),
+										},
+									},
+								},
+							},
+							err: nil,
+						},
+					},
 					feedbagDeleteParams: feedbagDeleteParams{
 						{
 							screenName: state.NewIdentScreenName("userA"),
@@ -4311,6 +4552,11 @@ func TestFeedbagBuddyHandler_DELETE(t *testing.T) {
 					Feedbag(matchContext(), params.screenName).
 					Return(params.result, params.err)
 			}
+			for _, params := range tc.mockParams.feedbagManagerParams.feedbagUpsertParams {
+				feedbagManager.EXPECT().
+					FeedbagUpsert(matchContext(), params.screenName, params.items).
+					Return(params.err)
+			}
 			for _, params := range tc.mockParams.feedbagManagerParams.feedbagDeleteParams {
 				feedbagManager.EXPECT().
 					FeedbagDelete(matchContext(), params.screenName, params.items).