chore(RHINENG-24199): Allow grouped hosts to move to other groups

Currently, HBI API returns HTTP 400 when trying to add grouped hosts to a different group. The response has this detail: "The following subset of hosts are already associated with another group: [<id>]."

This should now be allowed; the host should be removed from its old group and added to the specified one.

Author: Michael Johnson

lib/group_repository.py

@@ -114,19 +114,6 @@ def validate_add_host_list_to_group_for_group_create(host_id_list: list[str], gr
             title="Invalid request", detail=f"Could not find existing host(s) with ID {nonexistent_hosts}."
         )
 
-    # Check if the hosts are already associated with another (ungrouped) group
-    if assoc_query := (
-        HostGroupAssoc.query.join(Group)
-        .filter(HostGroupAssoc.host_id.in_(host_id_list), Group.ungrouped.is_(False))
-        .all()
-    ):
-        taken_hosts = [str(assoc.host_id) for assoc in assoc_query]
-        log_host_group_add_failed(logger, host_id_list, group_name)
-        raise InventoryException(
-            title="Invalid request",
-            detail=f"The following subset of hosts are already associated with another group: {taken_hosts}.",
-        )
-
 
 def validate_add_host_list_to_group(host_id_list: list[str], group_id: str, org_id: str):
     # Check if the hosts exist in Inventory and have correct org_id
@@ -139,22 +126,6 @@ def validate_add_host_list_to_group(host_id_list: list[str], group_id: str, org_
             title="Invalid request", detail=f"Could not find existing host(s) with ID {nonexistent_hosts}."
         )
 
-    # Check if the hosts are already associated with another (ungrouped) group
-    if assoc_query := (
-        db.session.query(HostGroupAssoc)
-        .join(Group)
-        .filter(
-            HostGroupAssoc.host_id.in_(host_id_list), HostGroupAssoc.group_id != group_id, Group.ungrouped.is_(False)
-        )
-        .all()
-    ):
-        taken_hosts = [str(assoc.host_id) for assoc in assoc_query]
-        log_host_group_add_failed(logger, host_id_list, group_id)
-        raise InventoryException(
-            title="Invalid request",
-            detail=f"The following subset of hosts are already associated with another group: {taken_hosts}.",
-        )
-
 
 def _add_hosts_to_group(group_id: str, host_id_list: list[str], org_id: str):
     # Validate that the hosts exist and can be added to the group

tests/test_api_groups_update.py

@@ -145,23 +145,29 @@ def test_patch_group_existing_name_different_org(
 def test_patch_group_hosts_from_different_group(
     db_create_group_with_hosts, api_patch_group, db_get_hosts_for_group, event_producer, mocker
 ):
+    """Patching a group's host_ids to include a host from another group should move that host (200)."""
     mocker.patch.object(event_producer, "write_event")
-    # Create 2 groups
+    # Create 2 groups: existing_group has 3 hosts, new_group has 1 host
     group_id = db_create_group_with_hosts("existing_group", 3).id
     host_to_move_id = str(db_get_hosts_for_group(group_id)[0].id)
     new_id = db_create_group_with_hosts("new_group", 1).id
 
     patch_doc = {"host_ids": [host_to_move_id]}
 
-    response_status, response_body = api_patch_group(new_id, patch_doc)
+    response_status, _ = api_patch_group(new_id, patch_doc)
 
-    # There's already a group with that name, so we should get an HTTP 400.
-    # Make sure the host ID at fault is mentioned in the response.
-    assert_response_status(response_status, 400)
-    assert str(host_to_move_id) in response_body["detail"]
+    # Moving a host between groups is now allowed; expect 200.
+    assert_response_status(response_status, 200)
 
-    # Make sure no events got produced
-    assert event_producer.write_event.call_count == 0
+    # Host should now be in new_group only; existing_group should have the other 2
+    new_group_host_ids = [str(h.id) for h in db_get_hosts_for_group(new_id)]
+    existing_group_host_ids = [str(h.id) for h in db_get_hosts_for_group(group_id)]
+    assert new_group_host_ids == [host_to_move_id]
+    assert host_to_move_id not in existing_group_host_ids
+    assert len(existing_group_host_ids) == 2
+
+    # Events for the host moved into new_group and for the host removed from new_group (to ungrouped)
+    assert event_producer.write_event.call_count == 2
 
 
 @pytest.mark.usefixtures("enable_rbac", "event_producer")

tests/test_api_host_group.py

@@ -1,3 +1,4 @@
+import json
 import uuid
 from collections.abc import Callable
 from datetime import UTC
@@ -170,30 +171,43 @@ def test_add_associated_host_to_different_group(
     event_producer,
     mocker,
 ):
+    """Adding hosts that are in other groups should move them to the target group (no longer 400)."""
     mocker.patch.object(event_producer, "write_event")
 
-    # Create a group and 3 hosts
+    # Create two groups and 3 hosts
     group1_id = db_create_group("test_group").id
     group2_id = db_create_group("test_group2").id
     host_id_list = [str(db_create_host().id) for _ in range(3)]
 
-    # Add the second 2 hosts to a group
+    # Host 1 in group1, host 2 in group2, host 0 in no group (or ungrouped)
     db_create_host_group_assoc(host_id_list[1], group1_id)
     db_create_host_group_assoc(host_id_list[2], group2_id)
 
-    # Confirm that the association exists
-    hosts_before = db_get_hosts_for_group(group1_id)
-    assert len(hosts_before) == 1
-    hosts_before = db_get_hosts_for_group(group2_id)
-    assert len(hosts_before) == 1
+    # Confirm initial state
+    assert len(db_get_hosts_for_group(group1_id)) == 1
+    assert len(db_get_hosts_for_group(group2_id)) == 1
 
-    # Confirm that the API does not allow these hosts to be added to the group
+    # Add all 3 hosts to group1; hosts should be moved from their previous groups
     response_status, _ = api_add_hosts_to_group(group1_id, host_id_list)
-    assert response_status == 400
+    assert response_status == 200
 
-    # Make sure that everything was rolled back and no events were produced
-    assert len(db_get_hosts_for_group(group1_id)) == 1
-    assert event_producer.write_event.call_count == 0
+    # Verify host-group associations: exact host IDs in each group, old group empty
+    group1_hosts = db_get_hosts_for_group(group1_id)
+    group2_hosts = db_get_hosts_for_group(group2_id)
+    assert sorted(str(h.id) for h in group1_hosts) == sorted(host_id_list)
+    assert group2_hosts == []
+
+    # Update events should have been produced for the moved/added hosts
+    calls = event_producer.write_event.call_args_list
+    assert len(calls) == 3
+    # Each emitted event should correspond to one of the affected hosts (event is 1st positional arg, JSON string)
+    emitted_host_ids = {str(json.loads(call.args[0])["host"]["id"]) for call in calls}
+    assert emitted_host_ids == set(host_id_list)
+    # Each event should be a successful update (not a failure)
+    for call in calls:
+        event = json.loads(call.args[0])
+        assert event["type"] == "updated"
+        assert event.get("status") != "failure"
 
 
 def test_add_host_in_ungrouped_group_to_new_group(