AAP-82996 Unify activity stream format for old and new role assignment APIs (#16616)

Unify activity stream format for role assignments across old and new APIs

The old API (/api/v2/users/<pk>/roles/) and new API
(/api/v2/role_user_assignments/) were producing different activity stream
entries for the same logical action. The old API created entries via the
Role.members m2m_changed signal in the legacy format, while the new API
used the new-format handler added in AAP-82996.

This change makes the old API produce the same new-format entry by:
- Skipping the legacy activity_stream_associate for Role_members changes
- Letting the mirrored RoleUserAssignment creation fire the new-format
  record_role_assignment_activity_stream handler instead

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

awx/main/models/rbac.py

@@ -667,8 +667,6 @@ def give_creator_permissions(user, obj):
 
 
 def sync_members_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs):
-    from awx.main.signals import disable_activity_stream
-
     if action.startswith('pre_'):
         return
     if not rbac_sync_enabled.enabled:
@@ -692,11 +690,7 @@ def sync_members_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs)
         else:
             user = get_user_model().objects.get(pk=user_or_role_id)
         rd = get_role_definition(role)
-        # This mirrors an already-recorded legacy Role.members change into the new RBAC
-        # system. Suppress activity stream here so the mirrored RoleUserAssignment/
-        # RoleTeamAssignment write isn't recorded a second time (see record_role_assignment_activity_stream).
-        with disable_activity_stream():
-            assignment = give_or_remove_permission(role, user, giving=is_giving, rd=rd)
+        assignment = give_or_remove_permission(role, user, giving=is_giving, rd=rd)
 
         # sync to resource server
         if rbac_sync_enabled.enabled:
@@ -707,8 +701,6 @@ def sync_members_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs)
 
 
 def sync_parents_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs):
-    from awx.main.signals import disable_activity_stream
-
     if action.startswith('pre_'):
         return
 
@@ -749,10 +741,7 @@ def sync_parents_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs)
 
             team = Team.objects.get(pk=parent_role.object_id)
             rd = get_role_definition(child_role)
-            # See comment in sync_members_to_new_rbac: suppress activity stream for this
-            # mirrored write to avoid double-recording an already-recorded legacy change.
-            with disable_activity_stream():
-                assignment = give_or_remove_permission(child_role, team, giving=is_giving, rd=rd)
+            assignment = give_or_remove_permission(child_role, team, giving=is_giving, rd=rd)
 
             # sync to resource server
             if rbac_sync_enabled.enabled:

awx/main/signals.py

@@ -56,8 +56,6 @@ from awx.main.models import (
 from awx.main.utils import model_instance_diff, model_to_dict, camelcase_to_underscore, get_current_apps
 from awx.main.utils import ignore_inventory_computed_fields, ignore_inventory_group_removal, _inventory_updates
 from awx.main.tasks.system import update_inventory_computed_fields, handle_removed_image
-from awx.main.fields import is_implicit_parent
-
 from awx.main import consumers
 
 from awx.conf.utils import conf_to_dict
@@ -155,34 +153,6 @@ def sync_rbac_to_superuser_status(instance, sender, **kwargs):
                 user.save(update_fields=['is_superuser'])
 
 
-def rbac_activity_stream(instance, sender, **kwargs):
-    # Only if we are associating/disassociating
-    if kwargs['action'] in ['pre_add', 'pre_remove']:
-        if hasattr(instance, 'content_type'):  # Duck typing, migration-independent isinstance(instance, Role)
-            if instance.content_type_id is None and instance.singleton_name == ROLE_SINGLETON_SYSTEM_ADMINISTRATOR:
-                # Skip entries for the system admin role because user serializer covers it
-                # System auditor role is shown in the serializer, but its relationship is
-                # managed separately, its value is incorrect, and a correction entry is needed
-                return
-            # This juggles which role to use, because could be A->B or B->A association
-            if sender.__name__ == 'Role_parents':
-                role = kwargs['model'].objects.filter(pk__in=kwargs['pk_set']).first()
-                # don't record implicit creation / parents in activity stream
-                if role is not None and is_implicit_parent(parent_role=role, child_role=instance):
-                    return
-            else:
-                role = instance
-            # If a singleton role is the instance, the singleton role is acted on
-            # otherwise the related object is considered to be acted on
-            if instance.content_object:
-                instance = instance.content_object
-        else:
-            # Association with actor, like role->user
-            role = kwargs['model'].objects.filter(pk__in=kwargs['pk_set']).first()
-
-        activity_stream_associate(sender, instance, role=role, **kwargs)
-
-
 def _record_role_assignment_activity_stream(instance, operation):
     if not activity_stream_enabled:
         return
@@ -284,8 +254,6 @@ def connect_computed_field_signals():
 connect_computed_field_signals()
 
 m2m_changed.connect(rebuild_role_ancestor_list, Role.parents.through)
-m2m_changed.connect(rbac_activity_stream, Role.members.through)
-m2m_changed.connect(rbac_activity_stream, Role.parents.through)
 post_save.connect(sync_superuser_status_to_rbac, sender=User)
 m2m_changed.connect(sync_rbac_to_superuser_status, Role.members.through)
 pre_delete.connect(cleanup_detached_labels_on_deleted_parent, sender=UnifiedJob)
@@ -572,17 +540,11 @@ def activity_stream_associate(sender, instance, **kwargs):
             getattr(activity_entry, object1).add(obj1.pk)
             getattr(activity_entry, object2).add(obj2_actual.pk)
 
-            # Record the role for RBAC changes
             if 'role' in kwargs:
                 role = kwargs['role']
                 if role.content_object is not None:
                     obj_rel = '.'.join([role.content_object.__module__, role.content_object.__class__.__name__, role.role_field])
 
-                # If the m2m is from the User side we need to
-                # set the content_object of the Role for our entry.
-                if type(instance) == User and role.content_object is not None:
-                    getattr(activity_entry, role.content_type.name.replace(' ', '_')).add(role.content_object)
-
                 activity_entry.role.add(role)
                 activity_entry.object_relationship_type = obj_rel
                 activity_entry.save()

awx/main/tests/functional/api/test_activity_streams.py

@@ -64,8 +64,8 @@ def test_rbac_stream_resource_roles(activity_stream_entry, organization, org_adm
     settings.ACTIVITY_STREAM_ENABLED = True
     assert activity_stream_entry.user.first() == org_admin
     assert activity_stream_entry.organization.first() == organization
-    assert activity_stream_entry.role.first() == organization.admin_role
-    assert activity_stream_entry.object_relationship_type == 'awx.main.models.organization.Organization.admin_role'
+    assert activity_stream_entry.object1 == 'organization'
+    assert activity_stream_entry.object2 == 'user'
 
 
 @pytest.mark.django_db
@@ -73,8 +73,8 @@ def test_rbac_stream_user_roles(activity_stream_entry, organization, org_admin,
     settings.ACTIVITY_STREAM_ENABLED = True
     assert activity_stream_entry.user.first() == org_admin
     assert activity_stream_entry.organization.first() == organization
-    assert activity_stream_entry.role.first() == organization.admin_role
-    assert activity_stream_entry.object_relationship_type == 'awx.main.models.organization.Organization.admin_role'
+    assert activity_stream_entry.object1 == 'organization'
+    assert activity_stream_entry.object2 == 'user'
 
 
 @pytest.mark.django_db
@@ -169,11 +169,9 @@ def test_stream_user_direct_role_updates(get, post, organization_factory):
     url = reverse('api:user_roles_list', kwargs={'pk': objects.users.test.pk})
     post(url, dict(id=objects.inventories.inv1.read_role.pk), objects.superusers.admin)
 
-    activity_stream = ActivityStream.objects.filter(
-        inventory__pk=objects.inventories.inv1.pk, user__pk=objects.users.test.pk, role__pk=objects.inventories.inv1.read_role.pk
-    ).first()
+    activity_stream = ActivityStream.objects.filter(inventory__pk=objects.inventories.inv1.pk, user__pk=objects.users.test.pk, operation='associate').first()
     url = reverse('api:activity_stream_detail', kwargs={'pk': activity_stream.pk})
     response = get(url, objects.users.test)
 
-    assert response.data['object1'] == 'user'
-    assert response.data['object2'] == 'inventory'
+    assert response.data['object1'] == 'inventory'
+    assert response.data['object2'] == 'user'

awx/main/tests/functional/models/test_activity_stream.py

@@ -4,7 +4,7 @@ from unittest import mock
 import json
 
 # AWX models
-from awx.main.models import ActivityStream, Organization, JobTemplate, Credential, CredentialType, Inventory, InventorySource, Project, User
+from awx.main.models import ActivityStream, Organization, JobTemplate, Credential, CredentialType, Inventory, InventorySource, User
 
 # other AWX
 from awx.main.utils import model_to_dict, model_instance_diff
@@ -22,8 +22,6 @@ class TestImplicitRolesOmitted:
     """
     Test that there is exactly 1 "create" entry in the activity stream for
     common items in the system.
-    These tests will fail if `rbac_activity_stream` creates
-    false-positive entries.
     """
 
     @pytest.mark.django_db
@@ -68,54 +66,33 @@ class TestImplicitRolesOmitted:
 
 
 @pytest.mark.django_db
-class TestRolesAssociationEntries:
-    """
-    Test that non-implicit role associations have a corresponding
-    activity stream entry.
-    These tests will fail if `rbac_activity_stream` skipping logic
-    in signals is wrong.
-    """
+@pytest.mark.parametrize('value', [True, False])
+def test_auditor_is_recorded(post, value):
+    u = User.objects.create(username='foouser')
+    assert not u.is_system_auditor
+    u.is_system_auditor = value
+    u = User.objects.get(pk=u.pk)  # refresh from db
+    assert u.is_system_auditor == value
+    entry_qs = ActivityStream.objects.filter(user=u)
+    if value:
+        assert len(entry_qs) == 2
+    else:
+        assert len(entry_qs) == 1
+    # unfortunate, the original creation does _not_ set a real is_auditor field
+    assert 'is_system_auditor' not in json.loads(entry_qs[0].changes)  # NOTE: if this fails, see special note
+    # special note - if system auditor flag is moved to user model then we expect this assertion to be changed
+    # make sure that an extra entry is not created, expectation for count would change to 1
+    if value:
+        entry = entry_qs[1]
+        assert json.loads(entry.changes) == {'is_system_auditor': [False, True]}
+        assert entry.object1 == 'user'
 
-    def test_non_implicit_associations_are_recorded(self, project):
-        org2 = Organization.objects.create(name='test-organization2')
-        # check that duplicate adds do not get recorded in 2nd loop
-        for i in range(2):
-            # Not supported, should not be possible via API
-            # org2.admin_role.children.add(project.admin_role)
-            project.admin_role.parents.add(org2.admin_role)
-            assert ActivityStream.objects.filter(role=org2.admin_role, organization=org2, project=project).count() == 1, 'In loop %s' % i
 
-    def test_model_associations_are_recorded(self, organization):
-        proj1 = Project.objects.create(name='proj1', organization=organization)
-        proj2 = Project.objects.create(name='proj2', organization=organization)
-        proj2.use_role.parents.add(proj1.admin_role)
-        assert ActivityStream.objects.filter(role=proj1.admin_role, project=proj2).count() == 1
-
-    @pytest.mark.parametrize('value', [True, False])
-    def test_auditor_is_recorded(self, post, value):
-        u = User.objects.create(username='foouser')
-        assert not u.is_system_auditor
-        u.is_system_auditor = value
-        u = User.objects.get(pk=u.pk)  # refresh from db
-        assert u.is_system_auditor == value
-        entry_qs = ActivityStream.objects.filter(user=u)
-        if value:
-            assert len(entry_qs) == 2
-        else:
-            assert len(entry_qs) == 1
-        # unfortunate, the original creation does _not_ set a real is_auditor field
-        assert 'is_system_auditor' not in json.loads(entry_qs[0].changes)  # NOTE: if this fails, see special note
-        # special note - if system auditor flag is moved to user model then we expect this assertion to be changed
-        # make sure that an extra entry is not created, expectation for count would change to 1
-        if value:
-            entry = entry_qs[1]
-            assert json.loads(entry.changes) == {'is_system_auditor': [False, True]}
-            assert entry.object1 == 'user'
-
-    def test_user_no_op_api(self, system_auditor):
-        as_ct = ActivityStream.objects.count()
-        system_auditor.is_system_auditor = True  # already auditor
-        assert ActivityStream.objects.count() == as_ct
+@pytest.mark.django_db
+def test_user_no_op_api(system_auditor):
+    as_ct = ActivityStream.objects.count()
+    system_auditor.is_system_auditor = True  # already auditor
+    assert ActivityStream.objects.count() == as_ct
 
 
 @pytest.fixture