From 7449f8607c1be948a0b55611c21075757c3d7261 Mon Sep 17 00:00:00 2001 From: Foysal Ahamed Date: Wed, 19 Feb 2025 09:15:58 +0000 Subject: [PATCH] :sparkles: Allow filtering ozone team list by role and status (#3554) * :sparkles: Allow filtering ozone team list by role and status * fix style * changeset --------- Co-authored-by: Matthieu Sieben --- .changeset/lucky-comics-talk.md | 5 + lexicons/tools/ozone/team/listMembers.json | 17 +- packages/api/src/client/lexicons.ts | 9 + .../types/tools/ozone/team/listMembers.ts | 2 + packages/ozone/src/lexicon/lexicons.ts | 9 + .../types/tools/ozone/team/listMembers.ts | 2 + packages/ozone/src/team/index.ts | 20 ++ .../tests/__snapshots__/team.test.ts.snap | 338 ------------------ packages/ozone/tests/team.test.ts | 82 +++-- packages/pds/src/lexicon/lexicons.ts | 9 + .../types/tools/ozone/team/listMembers.ts | 2 + 11 files changed, 129 insertions(+), 366 deletions(-) create mode 100644 .changeset/lucky-comics-talk.md diff --git a/.changeset/lucky-comics-talk.md b/.changeset/lucky-comics-talk.md new file mode 100644 index 00000000000..213d8209787 --- /dev/null +++ b/.changeset/lucky-comics-talk.md @@ -0,0 +1,5 @@ +--- +"@atproto/ozone": patch +--- + +Allow filtering Ozone members based on role and status diff --git a/lexicons/tools/ozone/team/listMembers.json b/lexicons/tools/ozone/team/listMembers.json index 25b3f2a42dc..bb022afe497 100644 --- a/lexicons/tools/ozone/team/listMembers.json +++ b/lexicons/tools/ozone/team/listMembers.json @@ -8,13 +8,24 @@ "parameters": { "type": "params", "properties": { + "disabled": { + "type": "boolean" + }, + "roles": { + "type": "array", + "items": { + "type": "string" + } + }, "limit": { "type": "integer", "minimum": 1, "maximum": 100, "default": 50 }, - "cursor": { "type": "string" } + "cursor": { + "type": "string" + } } }, "output": { @@ -23,7 +34,9 @@ "type": "object", "required": ["members"], "properties": { - "cursor": { "type": "string" }, + "cursor": { + "type": "string" + }, "members": { "type": "array", "items": { diff --git a/packages/api/src/client/lexicons.ts b/packages/api/src/client/lexicons.ts index c3c55ebce4e..6f9981c88b7 100644 --- a/packages/api/src/client/lexicons.ts +++ b/packages/api/src/client/lexicons.ts @@ -13953,6 +13953,15 @@ export const schemaDict = { parameters: { type: 'params', properties: { + disabled: { + type: 'boolean', + }, + roles: { + type: 'array', + items: { + type: 'string', + }, + }, limit: { type: 'integer', minimum: 1, diff --git a/packages/api/src/client/types/tools/ozone/team/listMembers.ts b/packages/api/src/client/types/tools/ozone/team/listMembers.ts index 0bbdffcc38d..5a3c6862ce6 100644 --- a/packages/api/src/client/types/tools/ozone/team/listMembers.ts +++ b/packages/api/src/client/types/tools/ozone/team/listMembers.ts @@ -13,6 +13,8 @@ const is$typed = _is$typed, const id = 'tools.ozone.team.listMembers' export interface QueryParams { + disabled?: boolean + roles?: string[] limit?: number cursor?: string } diff --git a/packages/ozone/src/lexicon/lexicons.ts b/packages/ozone/src/lexicon/lexicons.ts index c3c55ebce4e..6f9981c88b7 100644 --- a/packages/ozone/src/lexicon/lexicons.ts +++ b/packages/ozone/src/lexicon/lexicons.ts @@ -13953,6 +13953,15 @@ export const schemaDict = { parameters: { type: 'params', properties: { + disabled: { + type: 'boolean', + }, + roles: { + type: 'array', + items: { + type: 'string', + }, + }, limit: { type: 'integer', minimum: 1, diff --git a/packages/ozone/src/lexicon/types/tools/ozone/team/listMembers.ts b/packages/ozone/src/lexicon/types/tools/ozone/team/listMembers.ts index 8488cb9b54c..9f1dfd273b9 100644 --- a/packages/ozone/src/lexicon/types/tools/ozone/team/listMembers.ts +++ b/packages/ozone/src/lexicon/types/tools/ozone/team/listMembers.ts @@ -14,6 +14,8 @@ const is$typed = _is$typed, const id = 'tools.ozone.team.listMembers' export interface QueryParams { + disabled?: boolean + roles?: string[] limit: number cursor?: string } diff --git a/packages/ozone/src/team/index.ts b/packages/ozone/src/team/index.ts index 1a220b489b7..db2a8088288 100644 --- a/packages/ozone/src/team/index.ts +++ b/packages/ozone/src/team/index.ts @@ -21,14 +21,34 @@ export class TeamService { async list({ cursor, limit = 25, + roles, + disabled, }: { cursor?: string limit?: number + disabled?: boolean + roles?: string[] }): Promise<{ members: Selectable[]; cursor?: string }> { let builder = this.db.db.selectFrom('member').selectAll() if (cursor) { builder = builder.where('createdAt', '>', new Date(cursor)) } + if (roles !== undefined) { + const knownRoles = roles.filter( + (r) => + r === 'tools.ozone.team.defs#roleAdmin' || + r === 'tools.ozone.team.defs#roleModerator' || + r === 'tools.ozone.team.defs#roleTriage', + ) + + // Optimization: no need to query to know that no values will be returned + if (!knownRoles.length) return { members: [] } + + builder = builder.where('role', 'in', knownRoles) + } + if (disabled !== undefined) { + builder = builder.where('disabled', disabled ? 'is' : 'is not', true) + } const members = await builder .limit(limit) .orderBy('createdAt', 'asc') diff --git a/packages/ozone/tests/__snapshots__/team.test.ts.snap b/packages/ozone/tests/__snapshots__/team.test.ts.snap index baf26bc32dc..a44fd591dc5 100644 --- a/packages/ozone/tests/__snapshots__/team.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/team.test.ts.snap @@ -348,341 +348,3 @@ Array [ }, ] `; - -exports[`team management listMembers allows all members to list all members 3`] = ` -Array [ - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(0)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "role": "tools.ozone.team.defs#roleAdmin", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(2)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "role": "tools.ozone.team.defs#roleTriage", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(3)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "role": "tools.ozone.team.defs#roleModerator", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(4)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "role": "tools.ozone.team.defs#roleModerator", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(1)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "profile": Object { - "associated": Object { - "feedgens": 0, - "labeler": true, - "lists": 0, - "starterPacks": 0, - }, - "description": "The pretend version of mod.bsky.app", - "did": "user(1)", - "displayName": "Dev-env Moderation", - "followersCount": 0, - "followsCount": 0, - "handle": "mod-authority.test", - "indexedAt": "1970-01-01T00:00:00.000Z", - "labels": Array [], - "postsCount": 0, - "viewer": Object { - "blockedBy": false, - "muted": false, - }, - }, - "role": "tools.ozone.team.defs#roleAdmin", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(5)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "profile": Object { - "associated": Object { - "feedgens": 0, - "labeler": false, - "lists": 0, - "starterPacks": 0, - }, - "avatar": "https://bsky.public.url/img/avatar/plain/user(6)/cids(0)@jpeg", - "createdAt": "1970-01-01T00:00:00.000Z", - "description": "its me!", - "did": "user(5)", - "displayName": "ali", - "followersCount": 2, - "followsCount": 3, - "handle": "alice.test", - "indexedAt": "1970-01-01T00:00:00.000Z", - "labels": Array [ - Object { - "cid": "cids(1)", - "cts": "1970-01-01T00:00:00.000Z", - "src": "user(5)", - "uri": "record(0)", - "val": "self-label-a", - }, - Object { - "cid": "cids(1)", - "cts": "1970-01-01T00:00:00.000Z", - "src": "user(5)", - "uri": "record(0)", - "val": "self-label-b", - }, - ], - "postsCount": 4, - "viewer": Object { - "blockedBy": false, - "muted": false, - }, - }, - "role": "tools.ozone.team.defs#roleAdmin", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(7)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "profile": Object { - "associated": Object { - "feedgens": 0, - "labeler": false, - "lists": 0, - "starterPacks": 0, - }, - "avatar": "https://bsky.public.url/img/avatar/plain/user(8)/cids(0)@jpeg", - "createdAt": "1970-01-01T00:00:00.000Z", - "description": "hi im bob label_me", - "did": "user(7)", - "displayName": "bobby", - "followersCount": 2, - "followsCount": 2, - "handle": "bob.test", - "indexedAt": "1970-01-01T00:00:00.000Z", - "labels": Array [], - "postsCount": 3, - "viewer": Object { - "blockedBy": false, - "muted": false, - }, - }, - "role": "tools.ozone.team.defs#roleModerator", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(9)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "profile": Object { - "associated": Object { - "feedgens": 0, - "labeler": false, - "lists": 0, - "starterPacks": 0, - }, - "did": "user(9)", - "followersCount": 2, - "followsCount": 1, - "handle": "carol.test", - "labels": Array [], - "postsCount": 2, - "viewer": Object { - "blockedBy": false, - "muted": false, - }, - }, - "role": "tools.ozone.team.defs#roleTriage", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, -] -`; - -exports[`team management listMembers allows all members to list all members 4`] = ` -Array [ - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(0)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "role": "tools.ozone.team.defs#roleAdmin", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(2)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "role": "tools.ozone.team.defs#roleTriage", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(3)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "role": "tools.ozone.team.defs#roleModerator", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(4)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "role": "tools.ozone.team.defs#roleModerator", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(1)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "profile": Object { - "associated": Object { - "feedgens": 0, - "labeler": true, - "lists": 0, - "starterPacks": 0, - }, - "description": "The pretend version of mod.bsky.app", - "did": "user(1)", - "displayName": "Dev-env Moderation", - "followersCount": 0, - "followsCount": 0, - "handle": "mod-authority.test", - "indexedAt": "1970-01-01T00:00:00.000Z", - "labels": Array [], - "postsCount": 0, - "viewer": Object { - "blockedBy": false, - "muted": false, - }, - }, - "role": "tools.ozone.team.defs#roleAdmin", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(5)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "profile": Object { - "associated": Object { - "feedgens": 0, - "labeler": false, - "lists": 0, - "starterPacks": 0, - }, - "avatar": "https://bsky.public.url/img/avatar/plain/user(6)/cids(0)@jpeg", - "createdAt": "1970-01-01T00:00:00.000Z", - "description": "its me!", - "did": "user(5)", - "displayName": "ali", - "followersCount": 2, - "followsCount": 3, - "handle": "alice.test", - "indexedAt": "1970-01-01T00:00:00.000Z", - "labels": Array [ - Object { - "cid": "cids(1)", - "cts": "1970-01-01T00:00:00.000Z", - "src": "user(5)", - "uri": "record(0)", - "val": "self-label-a", - }, - Object { - "cid": "cids(1)", - "cts": "1970-01-01T00:00:00.000Z", - "src": "user(5)", - "uri": "record(0)", - "val": "self-label-b", - }, - ], - "postsCount": 4, - "viewer": Object { - "blockedBy": false, - "muted": false, - }, - }, - "role": "tools.ozone.team.defs#roleAdmin", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(7)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "profile": Object { - "associated": Object { - "feedgens": 0, - "labeler": false, - "lists": 0, - "starterPacks": 0, - }, - "avatar": "https://bsky.public.url/img/avatar/plain/user(8)/cids(0)@jpeg", - "createdAt": "1970-01-01T00:00:00.000Z", - "description": "hi im bob label_me", - "did": "user(7)", - "displayName": "bobby", - "followersCount": 2, - "followsCount": 2, - "handle": "bob.test", - "indexedAt": "1970-01-01T00:00:00.000Z", - "labels": Array [], - "postsCount": 3, - "viewer": Object { - "blockedBy": false, - "muted": false, - }, - }, - "role": "tools.ozone.team.defs#roleModerator", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, - Object { - "createdAt": "1970-01-01T00:00:00.000Z", - "did": "user(9)", - "disabled": false, - "lastUpdatedBy": "user(1)", - "profile": Object { - "associated": Object { - "feedgens": 0, - "labeler": false, - "lists": 0, - "starterPacks": 0, - }, - "did": "user(9)", - "followersCount": 2, - "followsCount": 1, - "handle": "carol.test", - "labels": Array [], - "postsCount": 2, - "viewer": Object { - "blockedBy": false, - "muted": false, - }, - }, - "role": "tools.ozone.team.defs#roleTriage", - "updatedAt": "1970-01-01T00:00:00.000Z", - }, -] -`; diff --git a/packages/ozone/tests/team.test.ts b/packages/ozone/tests/team.test.ts index d7532d6952a..2746a6a18d4 100644 --- a/packages/ozone/tests/team.test.ts +++ b/packages/ozone/tests/team.test.ts @@ -1,4 +1,4 @@ -import { AtpAgent } from '@atproto/api' +import { AtpAgent, ToolsOzoneTeamDefs } from '@atproto/api' import { SeedClient, TestNetwork, basicSeed } from '@atproto/dev-env' import { forSnapshot } from './_util' @@ -38,8 +38,8 @@ describe('team management', () => { describe('listMembers', () => { it('allows all members to list all members', async () => { const [{ data: forAdmin }, { data: forTriage }] = await Promise.all([ - adminAgent.api.tools.ozone.team.listMembers({}), - triageAgent.api.tools.ozone.team.listMembers({}), + adminAgent.tools.ozone.team.listMembers({}), + triageAgent.tools.ozone.team.listMembers({}), ]) expect(forSnapshot(forAdmin.members)).toMatchSnapshot() expect(forSnapshot(forTriage.members)).toMatchSnapshot() @@ -47,20 +47,50 @@ describe('team management', () => { expect(forAdmin.members.length).toEqual(forTriage.members.length) }) - }) - describe('listMembers', () => { - it('allows all members to list all members', async () => { - const [{ data: forAdmin }, { data: forTriage }] = await Promise.all([ - adminAgent.api.tools.ozone.team.listMembers({}), - triageAgent.api.tools.ozone.team.listMembers({}), + it('allows filtering members by role', async () => { + const [{ data: onlyAdmins }, { data: onlyTriage }] = await Promise.all([ + adminAgent.tools.ozone.team.listMembers({ + roles: [ToolsOzoneTeamDefs.ROLEADMIN], + }), + adminAgent.tools.ozone.team.listMembers({ + roles: [ToolsOzoneTeamDefs.ROLETRIAGE], + }), ]) - expect(forSnapshot(forAdmin.members)).toMatchSnapshot() - expect(forSnapshot(forTriage.members)).toMatchSnapshot() - // Validate that the list looks the same to both admin and triage members - expect(forAdmin.members.length).toEqual(forTriage.members.length) + expect( + onlyAdmins.members.find( + ({ role }) => role !== ToolsOzoneTeamDefs.ROLEADMIN, + ), + ).toBeUndefined() + + expect( + onlyTriage.members.find( + ({ role }) => role !== ToolsOzoneTeamDefs.ROLETRIAGE, + ), + ).toBeUndefined() + }) + it('allows filtering members by disabled status', async () => { + const [{ data: onlyDisabled }, { data: onlyEnabled }] = await Promise.all( + [ + adminAgent.tools.ozone.team.listMembers({ + disabled: true, + }), + adminAgent.tools.ozone.team.listMembers({ + disabled: false, + }), + ], + ) + + expect( + onlyDisabled.members.find(({ disabled }) => !disabled), + ).toBeUndefined() + + expect( + onlyEnabled.members.find(({ disabled }) => disabled), + ).toBeUndefined() }) }) + describe('addMember', () => { const newMemberData = { did: 'did:plc:newMember', @@ -69,15 +99,15 @@ describe('team management', () => { } it('only allows admins to add member', async () => { await expect( - triageAgent.api.tools.ozone.team.addMember(newMemberData), + triageAgent.tools.ozone.team.addMember(newMemberData), ).rejects.toThrow('Must be an admin to add a member') const { data: newMember } = - await adminAgent.api.tools.ozone.team.addMember(newMemberData) + await adminAgent.tools.ozone.team.addMember(newMemberData) expect(forSnapshot(newMember)).toMatchSnapshot() }) it('throws error when trying to add existing member', async () => { await expect( - adminAgent.api.tools.ozone.team.addMember(newMemberData), + adminAgent.tools.ozone.team.addMember(newMemberData), ).rejects.toThrow('member already exists') }) }) @@ -85,19 +115,19 @@ describe('team management', () => { it('only allows admins to delete members', async () => { const { data: { members: initialMembers }, - } = await adminAgent.api.tools.ozone.team.listMembers({}) + } = await adminAgent.tools.ozone.team.listMembers({}) await expect( - triageAgent.api.tools.ozone.team.deleteMember({ + triageAgent.tools.ozone.team.deleteMember({ did: sc.dids.bob, }), ).rejects.toThrow('Must be an admin to delete a member') - await adminAgent.api.tools.ozone.team.deleteMember({ + await adminAgent.tools.ozone.team.deleteMember({ did: sc.dids.bob, }) const { data: { members: membersAfterDelete }, - } = await adminAgent.api.tools.ozone.team.listMembers({}) + } = await adminAgent.tools.ozone.team.listMembers({}) expect(membersAfterDelete.length).toEqual(initialMembers.length - 1) expect(membersAfterDelete.map(({ did }) => did)).not.toContain( @@ -107,7 +137,7 @@ describe('team management', () => { it('throws error when trying to remove non-existent member', async () => { await expect( - adminAgent.api.tools.ozone.team.deleteMember({ + adminAgent.tools.ozone.team.deleteMember({ did: 'did:plc:test', }), ).rejects.toThrow('member not found') @@ -118,19 +148,19 @@ describe('team management', () => { const getCarol = async () => { const { data: { members }, - } = await adminAgent.api.tools.ozone.team.listMembers({}) + } = await adminAgent.tools.ozone.team.listMembers({}) return members.find(({ did }) => did === sc.dids.carol) } await expect( - triageAgent.api.tools.ozone.team.updateMember({ + triageAgent.tools.ozone.team.updateMember({ disabled: false, did: sc.dids.carol, role: 'tools.ozone.team.defs#roleAdmin', }), ).rejects.toThrow('Must be an admin to update a member') - await adminAgent.api.tools.ozone.team.updateMember({ + await adminAgent.tools.ozone.team.updateMember({ did: sc.dids.carol, role: 'tools.ozone.team.defs#roleAdmin', }) @@ -141,7 +171,7 @@ describe('team management', () => { // Verify that params that we didn't send did not get updated expect(carolAfterRoleChange?.disabled).toEqual(false) - await adminAgent.api.tools.ozone.team.updateMember({ + await adminAgent.tools.ozone.team.updateMember({ did: sc.dids.carol, disabled: true, }) @@ -152,7 +182,7 @@ describe('team management', () => { }) it('throws error when trying to update non-existent member', async () => { await expect( - adminAgent.api.tools.ozone.team.updateMember({ + adminAgent.tools.ozone.team.updateMember({ disabled: false, did: 'did:plc:test', role: 'tools.ozone.team.defs#roleAdmin', diff --git a/packages/pds/src/lexicon/lexicons.ts b/packages/pds/src/lexicon/lexicons.ts index c3c55ebce4e..6f9981c88b7 100644 --- a/packages/pds/src/lexicon/lexicons.ts +++ b/packages/pds/src/lexicon/lexicons.ts @@ -13953,6 +13953,15 @@ export const schemaDict = { parameters: { type: 'params', properties: { + disabled: { + type: 'boolean', + }, + roles: { + type: 'array', + items: { + type: 'string', + }, + }, limit: { type: 'integer', minimum: 1, diff --git a/packages/pds/src/lexicon/types/tools/ozone/team/listMembers.ts b/packages/pds/src/lexicon/types/tools/ozone/team/listMembers.ts index 8488cb9b54c..9f1dfd273b9 100644 --- a/packages/pds/src/lexicon/types/tools/ozone/team/listMembers.ts +++ b/packages/pds/src/lexicon/types/tools/ozone/team/listMembers.ts @@ -14,6 +14,8 @@ const is$typed = _is$typed, const id = 'tools.ozone.team.listMembers' export interface QueryParams { + disabled?: boolean + roles?: string[] limit: number cursor?: string }