From 75977819f642e46df29f57ac8be7830b9218eb73 Mon Sep 17 00:00:00 2001
From: Hellgren Heikki <heikki.hellgren@op.fi>
Date: Thu, 28 Aug 2025 09:11:43 +0300
Subject: [PATCH] fix(catalog): msgraph to use only first parent for groups

the order of the groups from msgraph is random, thus the parent group
for groups having multiple parents can be random. this leads to
situation where the grpup parent is changing over the msgraph
provider runs leading to unnecessary stitching.

other possibility for the fix would be to change the model to allow
multiple parents for groups (like mentioned in the TODO) but that
could have some weird consequences in apps.

Signed-off-by: Hellgren Heikki <heikki.hellgren@op.fi>
---
 .changeset/loose-memes-bathe.md                |  5 +++++
 .../src/microsoftGraph/read.test.ts            | 18 ++++++++++++++++--
 .../src/microsoftGraph/read.ts                 |  9 ++++++++-
 3 files changed, 29 insertions(+), 3 deletions(-)
 create mode 100644 .changeset/loose-memes-bathe.md

diff --git a/.changeset/loose-memes-bathe.md b/.changeset/loose-memes-bathe.md
new file mode 100644
index 0000000000..85eff94399
--- /dev/null
+++ b/.changeset/loose-memes-bathe.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend-module-msgraph': patch
+---
+
+Ensure that msgraph parent group stays same in case the group has multiple parents
diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts
index 33ae04bd76..bd58881728 100644
--- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts
+++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts
@@ -1102,6 +1102,14 @@ describe('read microsoft graph', () => {
           name: 'c',
         },
       });
+      const groupD = group({
+        metadata: {
+          annotations: {
+            'graph.microsoft.com/group-id': 'group-id-d',
+          },
+          name: 'd',
+        },
+      });
       const user1 = user({
         metadata: {
           annotations: {
@@ -1118,16 +1126,17 @@ describe('read microsoft graph', () => {
           name: 'user2',
         },
       });
-      const groups = [rootGroup, groupA, groupB, groupC];
+      const groups = [rootGroup, groupA, groupB, groupC, groupD];
       const users = [user1, user2];
       const groupMember = new Map<string, Set<string>>();
       groupMember.set('group-id-b', new Set(['group-id-c']));
+      groupMember.set('group-id-d', new Set(['group-id-c']));
       const groupMemberOf = new Map<string, Set<string>>();
       groupMemberOf.set('user-id-1', new Set(['group-id-a']));
       groupMemberOf.set('user-id-2', new Set(['group-id-c']));
 
       // We have a root groups
-      // We have three groups: a, b, c. c is child of b
+      // We have three groups: a, b, c. c is child of b and d, b should be picked as it's first
       // we have two users: u1, u2. u1 is member of a, u2 is member of c
       resolveRelations(rootGroup, groups, users, groupMember, groupMemberOf);
 
@@ -1147,6 +1156,11 @@ describe('read microsoft graph', () => {
       expect(groupC.spec.parent).toEqual('group:default/b');
       expect(groupC.spec.children).toEqual(expect.arrayContaining([]));
 
+      expect(groupD.spec.parent).toEqual('group:default/root');
+      expect(groupD.spec.children).toEqual(
+        expect.arrayContaining(['group:default/c']),
+      );
+
       expect(user1.spec.memberOf).toEqual(
         expect.arrayContaining(['group:default/a']),
       );
diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts
index 4761c45ac7..8bc7e6899b 100644
--- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts
+++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts
@@ -359,7 +359,14 @@ export function resolveRelations(
       }
     });
 
+    // TODO: Until we have better support for multiple parents in the model,
+    //       the order of the parents is important as changing it causes
+    //       unnecessary entity stitching randomly.
     retrieveItems(parentGroups, id).forEach(p => {
+      // Only set the parent if it doesn't exist yet
+      if (group.spec.parent) {
+        return;
+      }
       const parentGroup = groupMap.get(p);
       if (parentGroup) {
         // TODO: Only having a single parent group might not match every companies model, but fine for now.
@@ -546,5 +553,5 @@ function retrieveItems(
   target: Map<string, Set<string>>,
   key: string,
 ): Set<string> {
-  return target.get(key) ?? new Set();
+  return new Set([...(target.get(key) ?? [])].sort());
 }