Public: anyone can read this; the crew can change it.
An organization's teams and a team's members answer `{ items, next_cursor }` #4848
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4842-an-organization-s-teams-and-a-team-s-mem → main 2 commits fork 2 threads · 2 unresolved
2 files
changed +142
−5
src/http/api.ts +43 −2
| @@ -194,6 +194,8 @@ import { | ||
| 194 | 194 | listComments, |
| 195 | 195 | listIssues, |
| 196 | 196 | listLabels, |
| 197 | LIST_LIMIT, | |
| 198 | LIST_LIMIT_MAX, | |
| 197 | 199 | pageLimit, |
| 198 | 200 | type Issue, |
| 199 | 201 | type IssueState, |
| @@ -1042,6 +1044,37 @@ function listQuery(ctx: Ctx) { | ||
| 1042 | 1044 | }; |
| 1043 | 1045 | } |
| 1044 | 1046 | |
| 1047 | /** | |
| 1048 | * `?limit=&cursor=` for a list paged by a name: limit 1..200 (default 50), | |
| 1049 | * the cursor the last name of the page before. A bad one is a 422. | |
| 1050 | */ | |
| 1051 | function keyedWindow(ctx: Ctx): { limit: number; cursor: string | null } | Response { | |
| 1052 | const q = ctx.url.searchParams; | |
| 1053 | const rawLimit = q.get("limit"); | |
| 1054 | const limit = rawLimit === null ? LIST_LIMIT : /^\d{1,4}$/.test(rawLimit) ? Number(rawLimit) : 0; | |
| 1055 | if (limit < 1 || limit > LIST_LIMIT_MAX) | |
| 1056 | return apiError("invalid_limit", 422, `limit is a whole number from 1 to ${LIST_LIMIT_MAX}`); | |
| 1057 | const cursor = q.get("cursor"); | |
| 1058 | if (cursor !== null && (cursor.length < 1 || cursor.length > 100)) | |
| 1059 | return apiError("invalid_cursor", 422, "cursor is the next_cursor of a previous page"); | |
| 1060 | return { limit, cursor }; | |
| 1061 | } | |
| 1062 | ||
| 1063 | /** The page of `rows` (sorted by name) after the cursor: `{ items, next_cursor }`. */ | |
| 1064 | function keyedPage<T>( | |
| 1065 | rows: T[], | |
| 1066 | keyOf: (row: T) => string, | |
| 1067 | window: { limit: number; cursor: string | null }, | |
| 1068 | ) { | |
| 1069 | const sorted = [...rows].sort((a, b) => (keyOf(a) < keyOf(b) ? -1 : keyOf(a) > keyOf(b) ? 1 : 0)); | |
| 1070 | const after = window.cursor === null ? sorted : sorted.filter((r) => keyOf(r) > window.cursor!); | |
| 1071 | const items = after.slice(0, window.limit); | |
| 1072 | return { | |
| 1073 | items, | |
| 1074 | next_cursor: after.length > window.limit ? keyOf(items[items.length - 1]!) : null, | |
| 1075 | }; | |
| 1076 | } | |
| 1077 | ||
| 1045 | 1078 | /** `?limit=&cursor=` for repo lists (the cursor is a repo id). */ |
| 1046 | 1079 | function repoQuery(ctx: Ctx) { |
| 1047 | 1080 | const q = ctx.url.searchParams; |
| @@ -2171,7 +2204,11 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 2171 | 2204 | router.add("GET", "/api/v1/orgs/:org/teams", (ctx) => { |
| 2172 | 2205 | const org = getOrg(ctx.app.db, ctx.params.org!); |
| 2173 | 2206 | if (!org) return notFound(); |
| 2174 | return json(listTeamsVisibleTo(ctx.app.db, org.id, ctx.auth.user)); | |
| 2207 | const window = keyedWindow(ctx); | |
| 2208 | if (window instanceof Response) return window; | |
| 2209 | return json( | |
| 2210 | keyedPage(listTeamsVisibleTo(ctx.app.db, org.id, ctx.auth.user), (t) => t.name, window), | |
| 2211 | ); | |
| 2175 | 2212 | }); |
| 2176 | 2213 | |
| 2177 | 2214 | router.add("POST", "/api/v1/orgs/:org/teams", async (ctx) => { |
| @@ -2205,7 +2242,11 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 2205 | 2242 | const found = loadTeam(ctx); |
| 2206 | 2243 | if ("error" in found) return found.error; |
| 2207 | 2244 | if (!teamVisible(ctx, found.org, found.team)) return notFound(); |
| 2208 | return json(listTeamMembers(ctx.app.db, found.team.id).map(publicProfile)); | |
| 2245 | const window = keyedWindow(ctx); | |
| 2246 | if (window instanceof Response) return window; | |
| 2247 | return json( | |
| 2248 | keyedPage(listTeamMembers(ctx.app.db, found.team.id).map(publicProfile), (u) => u.username, window), | |
| 2249 | ); | |
| 2209 | 2250 | }); |
| 2210 | 2251 | |
| 2211 | 2252 | router.add("POST", "/api/v1/orgs/:org/teams/:team/members", async (ctx) => { |
test/parity.test.ts +99 −3
| @@ -782,15 +782,19 @@ describe("parity: full control through actions and the API", () => { | ||
| 782 | 782 | const asBob = await call("GET", "/api/v1/orgs/acme/teams", { |
| 783 | 783 | token: bobRw, |
| 784 | 784 | }); |
| 785 | expect(asBob.list.map((t) => t.name)).toEqual(["devs"]); | |
| 786 | expect((await call("GET", "/api/v1/orgs/acme/teams")).list).toEqual([]); | |
| 785 | expect((asBob.json.items as Json[]).map((t) => t.name)).toEqual(["devs"]); | |
| 786 | expect(asBob.json.next_cursor).toBeNull(); | |
| 787 | expect((await call("GET", "/api/v1/orgs/acme/teams")).json).toEqual({ | |
| 788 | items: [], | |
| 789 | next_cursor: null, | |
| 790 | }); | |
| 787 | 791 | expect((await call("GET", "/api/v1/orgs/alice")).status).toBe(404); |
| 788 | 792 | |
| 789 | 793 | // Members are visible to those who can see the team. |
| 790 | 794 | const members = await call("GET", "/api/v1/orgs/acme/teams/devs/members", { |
| 791 | 795 | token: bobRw, |
| 792 | 796 | }); |
| 793 | expect(members.list.map((m) => m.username)).toEqual(["bob"]); | |
| 797 | expect((members.json.items as Json[]).map((m) => m.username)).toEqual(["bob"]); | |
| 794 | 798 | expect( |
| 795 | 799 | ( |
| 796 | 800 | await call("GET", "/api/v1/orgs/acme/teams/ops/members", { |
| @@ -859,6 +863,98 @@ describe("parity: full control through actions and the API", () => { | ||
| 859 | 863 | expect(kinds()).toContain(kind); |
| 860 | 864 | }); |
| 861 | 865 | |
| 866 | test("teams and members page as { items, next_cursor }", async () => { | |
| 867 | await call("POST", "/api/v1/orgs/acme/teams", { | |
| 868 | token: aliceAdmin, | |
| 869 | body: { name: "zeta", permission: "read" }, | |
| 870 | }); | |
| 871 | const all = await call("GET", "/api/v1/orgs/acme/teams", { token: aliceAdmin }); | |
| 872 | const names = (all.json.items as Json[]).map((t) => t.name as string); | |
| 873 | expect(names.length).toBeGreaterThanOrEqual(3); | |
| 874 | expect(all.json.next_cursor).toBeNull(); | |
| 875 | const first = await call("GET", "/api/v1/orgs/acme/teams?limit=2", { token: aliceAdmin }); | |
| 876 | expect((first.json.items as Json[]).map((t) => t.name)).toEqual(names.slice(0, 2)); | |
| 877 | expect(first.json.next_cursor).toBe(names[1]!); | |
| 878 | const second = await call( | |
| 879 | "GET", | |
| 880 | `/api/v1/orgs/acme/teams?limit=200&cursor=${names[1]}`, | |
| 881 | { token: aliceAdmin }, | |
| 882 | ); | |
| 883 | expect((second.json.items as Json[]).map((t) => t.name)).toEqual(names.slice(2)); | |
| 884 | expect(second.json.next_cursor).toBeNull(); | |
| 885 | ||
| 886 | for (const user of ["bob", "carol"]) | |
| 887 | await call("POST", "/api/v1/orgs/acme/teams/devs/members", { | |
| 888 | token: aliceAdmin, | |
| 889 | body: { user }, | |
| 890 | }); | |
| 891 | const m1 = await call("GET", "/api/v1/orgs/acme/teams/devs/members?limit=1", { | |
| 892 | token: aliceAdmin, | |
| 893 | }); | |
| 894 | expect((m1.json.items as Json[]).map((m) => m.username)).toEqual(["bob"]); | |
| 895 | expect(m1.json.next_cursor).toBe("bob"); | |
| 896 | const m2 = await call("GET", "/api/v1/orgs/acme/teams/devs/members?limit=1&cursor=bob", { | |
| 897 | token: aliceAdmin, | |
| 898 | }); | |
| 899 | expect((m2.json.items as Json[]).map((m) => m.username)).toEqual(["carol"]); | |
| 900 | expect(m2.json.next_cursor).toBeNull(); | |
| 901 | ||
| 902 | // A member of a private team is not shown to an outsider. | |
| 903 | const outsider = await call("GET", "/api/v1/orgs/acme/teams/zeta/members", { token: bobRw }); | |
| 904 | expect(outsider.status).toBe(404); | |
| 905 | expect(outsider.json.items).toBeUndefined(); | |
| 906 | ||
| 907 | for (const path of ["teams", "teams/devs/members"]) | |
Add the cursor half next to this loop: |
||
| 908 | for (const bad of ["0", "201", "-1", "abc", "1.5"]) { | |
| 909 | const res = await call("GET", `/api/v1/orgs/acme/${path}?limit=${bad}`, { | |
| 910 | token: aliceAdmin, | |
| 911 | }); | |
| 912 | expect(res.status).toBe(422); | |
| 913 | expect(res.json.error).toBe("invalid_limit"); | |
| 914 | } | |
| 915 | ||
| 916 | // The cursor is only compared: any 1..100 characters, nothing else. | |
| 917 | for (const path of ["teams", "teams/devs/members"]) | |
| 918 | for (const bad of ["", "x".repeat(101)]) { | |
| 919 | const res = await call("GET", `/api/v1/orgs/acme/${path}?cursor=${bad}`, { | |
| 920 | token: aliceAdmin, | |
| 921 | }); | |
| 922 | expect(res.status).toBe(422); | |
| 923 | expect(res.json.error).toBe("invalid_cursor"); | |
| 924 | } | |
| 925 | ||
| 926 | // A name with a control character pages on: its own cursor is accepted. | |
| 927 | const odd = await call("POST", "/api/v1/orgs/acme/teams", { | |
| 928 | token: aliceAdmin, | |
| 929 | body: { name: "a\u0001b", permission: "read" }, | |
| 930 | }); | |
| 931 | expect(odd.status).toBe(201); | |
| 932 | { | |
| 933 | const list = await call("GET", "/api/v1/orgs/acme/teams", { token: aliceAdmin }); | |
| 934 | const at = (list.json.items as Json[]).findIndex((t) => t.name === "a\u0001b"); | |
| 935 | const page = await call("GET", `/api/v1/orgs/acme/teams?limit=${at + 1}`, { | |
| 936 | token: aliceAdmin, | |
| 937 | }); | |
| 938 | expect(page.json.next_cursor).toBe("a\u0001b"); | |
| 939 | const next = await call( | |
| 940 | "GET", | |
| 941 | `/api/v1/orgs/acme/teams?cursor=${encodeURIComponent("a\u0001b")}`, | |
| 942 | { token: aliceAdmin }, | |
| 943 | ); | |
| 944 | expect(next.status).toBe(200); | |
| 945 | } | |
| 946 | ||
| 947 | // The default page is 50. | |
| 948 | for (let i = 0; i < 50; i++) | |
| 949 | await call("POST", "/api/v1/orgs/acme/teams", { | |
| 950 | token: aliceAdmin, | |
| 951 | body: { name: `p${String(i).padStart(2, "0")}`, permission: "read" }, | |
| 952 | }); | |
| 953 | const dflt = await call("GET", "/api/v1/orgs/acme/teams", { token: aliceAdmin }); | |
| 954 | expect((dflt.json.items as Json[]).length).toBe(50); | |
| 955 | expect(dflt.json.next_cursor).not.toBeNull(); | |
| 956 | }); | |
| 957 | ||
| 862 | 958 | test("a team member sees the org's private repos in listings", async () => { |
| 863 | 959 | await call("POST", "/api/v1/repos", { |
| 864 | 960 | token: aliceRw, |
new above
offline
Medium: this refuses cursors the route issues itself. Team names are only trimmed and capped at 60 characters (
createTeam), so a name containing a control character becomes anext_cursorthat this line answers with 422, and paging stops there. The cursor is only compared with>in memory, so a length check is enough:cursor.length < 1 || cursor.length > 100.