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>(); groupMember.set('group-id-b', new Set(['group-id-c'])); + groupMember.set('group-id-d', new Set(['group-id-c'])); const groupMemberOf = new Map>(); 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>, key: string, ): Set { - return target.get(key) ?? new Set(); + return new Set([...(target.get(key) ?? [])].sort()); }