diff --git a/README.md b/README.md index a39b428..d43e56f 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,9 @@ To configure your Ubiquibot to run this plugin, add the following to the `.ubiqu with: reviewDelayTolerance: "3 Days" taskStaleTimeoutDuration: "30 Days" - maxConcurrentTasks: 3 + maxConcurrentTasks: # Default concurrent task limits per role. + member: 5 + contributor: 3 startRequiresWallet: true # default is true emptyWalletText: "Please set your wallet address with the /wallet command first and try again." ``` diff --git a/src/handlers/shared/get-user-task-limit-and-role.ts b/src/handlers/shared/get-user-task-limit-and-role.ts new file mode 100644 index 0000000..53b9671 --- /dev/null +++ b/src/handlers/shared/get-user-task-limit-and-role.ts @@ -0,0 +1,37 @@ +import { Context } from "../../types"; + +interface MatchingUserProps { + role: string; + limit: number; +} + +export async function getUserRoleAndTaskLimit(context: Context, user: string): Promise { + const orgLogin = context.payload.organization?.login; + const { config, logger } = context; + const { maxConcurrentTasks } = config; + + const smallestTask = Object.entries(maxConcurrentTasks).reduce((minTask, [role, limit]) => (limit < minTask.limit ? { role, limit } : minTask), { + role: "", + limit: Infinity, + } as MatchingUserProps); + + try { + // Validate the organization login + if (typeof orgLogin !== "string" || orgLogin.trim() === "") { + throw new Error("Invalid organization name"); + } + + const response = await context.octokit.orgs.getMembershipForUser({ + org: orgLogin, + username: user, + }); + + const role = response.data.role.toLowerCase(); + const limit = maxConcurrentTasks[role]; + + return limit ? { role, limit } : smallestTask; + } catch (err) { + logger.error("Could not get user role", { err }); + return smallestTask; + } +} diff --git a/src/handlers/shared/start.ts b/src/handlers/shared/start.ts index a5fe55e..f93b6d9 100644 --- a/src/handlers/shared/start.ts +++ b/src/handlers/shared/start.ts @@ -1,8 +1,9 @@ import { Context, ISSUE_TYPE, Label } from "../../types"; -import { addAssignees, addCommentToIssue, getAssignedIssues, getAvailableOpenedPullRequests, getTimeValue, isParentIssue } from "../../utils/issue"; -import { HttpStatusCode, Result } from "../result-types"; -import { hasUserBeenUnassigned } from "./check-assignments"; +import { isParentIssue, getAvailableOpenedPullRequests, getAssignedIssues, addAssignees, addCommentToIssue, getTimeValue } from "../../utils/issue"; import { checkTaskStale } from "./check-task-stale"; +import { hasUserBeenUnassigned } from "./check-assignments"; +import { getUserRoleAndTaskLimit } from "./get-user-task-limit-and-role"; +import { HttpStatusCode, Result } from "../result-types"; import { generateAssignmentComment, getDeadline } from "./generate-assignment-comment"; import structuredMetadata from "./structured-metadata"; import { assignTableComment } from "./table"; @@ -14,7 +15,7 @@ export async function start( teammates: string[] ): Promise { const { logger, config } = context; - const { maxConcurrentTasks, taskStaleTimeoutDuration } = config; + const { taskStaleTimeoutDuration } = config; // is it a child issue? if (issue.body && isParentIssue(issue.body)) { @@ -62,7 +63,7 @@ export async function start( const toAssign = []; // check max assigned issues for (const user of teammates) { - if (await handleTaskLimitChecks(user, context, maxConcurrentTasks, logger, sender.login)) { + if (await handleTaskLimitChecks(user, context, logger, sender.login)) { toAssign.push(user); } } @@ -139,18 +140,19 @@ async function fetchUserIds(context: Context, username: string[]) { return ids; } -async function handleTaskLimitChecks(username: string, context: Context, maxConcurrentTasks: number, logger: Context["logger"], sender: string) { +async function handleTaskLimitChecks(username: string, context: Context, logger: Context["logger"], sender: string) { const openedPullRequests = await getAvailableOpenedPullRequests(context, username); const assignedIssues = await getAssignedIssues(context, username); + const { limit } = await getUserRoleAndTaskLimit(context, username); // check for max and enforce max - - if (Math.abs(assignedIssues.length - openedPullRequests.length) >= maxConcurrentTasks) { + if (Math.abs(assignedIssues.length - openedPullRequests.length) >= limit) { logger.error(username === sender ? "You have reached your max task limit" : `${username} has reached their max task limit`, { assignedIssues: assignedIssues.length, openedPullRequests: openedPullRequests.length, - maxConcurrentTasks, + limit, }); + return false; } diff --git a/src/types/payload.ts b/src/types/payload.ts index c0a7787..c6992bb 100644 --- a/src/types/payload.ts +++ b/src/types/payload.ts @@ -7,6 +7,7 @@ export type TimelineEventResponse = RestEndpointMethodTypes["issues"]["listEvent export type TimelineEvents = RestEndpointMethodTypes["issues"]["listEventsForTimeline"]["response"]["data"][0]; export type Assignee = Issue["assignee"]; export type GitHubIssueSearch = RestEndpointMethodTypes["search"]["issuesAndPullRequests"]["response"]["data"]; + export type Sender = { login: string; id: number }; export const ISSUE_TYPE = { diff --git a/src/types/plugin-input.ts b/src/types/plugin-input.ts index 01a1c95..6278fb6 100644 --- a/src/types/plugin-input.ts +++ b/src/types/plugin-input.ts @@ -15,8 +15,8 @@ export const startStopSchema = T.Object( { reviewDelayTolerance: T.String({ default: "1 Day" }), taskStaleTimeoutDuration: T.String({ default: "30 Days" }), - maxConcurrentTasks: T.Number({ default: 3 }), startRequiresWallet: T.Boolean({ default: true }), + maxConcurrentTasks: T.Record(T.String(), T.Integer(), { default: { admin: Infinity, member: 10, contributor: 2 } }), emptyWalletText: T.String({ default: "Please set your wallet address with the /wallet command first and try again." }), }, { diff --git a/src/utils/issue.ts b/src/utils/issue.ts index 70d9691..fa9dbd9 100644 --- a/src/utils/issue.ts +++ b/src/utils/issue.ts @@ -1,6 +1,6 @@ import ms from "ms"; import { Context, isContextCommentCreated } from "../types/context"; -import { GitHubIssueSearch, Issue, Review } from "../types/payload"; +import { GitHubIssueSearch, Review } from "../types/payload"; import { getLinkedPullRequests, GetLinkedResults } from "./get-linked-prs"; export function isParentIssue(body: string) { @@ -8,13 +8,22 @@ export function isParentIssue(body: string) { return body.match(parentPattern); } -export async function getAssignedIssues(context: Context, username: string): Promise { - const { payload } = context; +export async function getAssignedIssues(context: Context, username: string): Promise { + const payload = context.payload; try { - return (await context.octokit.paginate(context.octokit.rest.search.issuesAndPullRequests, { - q: `is:issue is:open assignee:${username} org:${payload.repository.owner.login}`, - })) as Issue[]; + return await context.octokit + .paginate(context.octokit.search.issuesAndPullRequests, { + q: `org:${payload.repository.owner.login} assignee:${username} is:open is:issue`, + per_page: 100, + order: "desc", + sort: "created", + }) + .then((issues) => + issues.filter((issue) => { + return issue.state === "open" && (issue.assignee?.login === username || issue.assignees?.some((assignee) => assignee.login === username)); + }) + ); } catch (err: unknown) { throw new Error(context.logger.error("Fetching assigned issues failed!", { error: err as Error }).logMessage.raw); } diff --git a/tests/__mocks__/db.ts b/tests/__mocks__/db.ts index 70d2ccb..d81914d 100644 --- a/tests/__mocks__/db.ts +++ b/tests/__mocks__/db.ts @@ -7,6 +7,7 @@ export const db = factory({ users: { id: primaryKey(Number), login: String, + role: String, }, issue: { id: primaryKey(Number), diff --git a/tests/__mocks__/handlers.ts b/tests/__mocks__/handlers.ts index 30131ba..f8fa4b0 100644 --- a/tests/__mocks__/handlers.ts +++ b/tests/__mocks__/handlers.ts @@ -14,6 +14,15 @@ export const handlers = [ } return HttpResponse.json(item); }), + //get member + http.get("https://api.github.com/orgs/:org/memberships/:username", ({ params: { username } }) => { + const user = db.users.findFirst({ where: { login: { equals: username as string } } }); + if (user) { + return HttpResponse.json({ role: user.role }); + } else { + return HttpResponse.json({ role: "collaborator" }); + } + }), // get issue http.get("https://api.github.com/repos/:owner/:repo/issues", ({ params: { owner, repo } }: { params: { owner: string; repo: string } }) => HttpResponse.json(db.issue.findMany({ where: { owner: { equals: owner }, repo: { equals: repo } } })) @@ -34,15 +43,13 @@ export const handlers = [ HttpResponse.json({ owner, repo, issueNumber }) ), // get commit - http.get("https://api.github.com/repos/:owner/:repo/commits/:ref", () => { - const res = { + http.get("https://api.github.com/repos/:owner/:repo/commits/:ref", () => + HttpResponse.json({ data: { sha: "commitHash", }, - }; - - return HttpResponse.json(res); - }), + }) + ), // list pull requests http.get("https://api.github.com/repos/:owner/:repo/pulls", ({ params: { owner, repo } }: { params: { owner: string; repo: string } }) => HttpResponse.json(db.pull.findMany({ where: { owner: { equals: owner }, repo: { equals: repo } } })) @@ -93,10 +100,15 @@ export const handlers = [ http.delete("https://api.github.com/repos/:owner/:repo/issues/:issue_number/assignees", ({ params: { owner, repo, issue_number: issueNumber } }) => HttpResponse.json({ owner, repo, issueNumber }) ), - // search issues - http.get("https://api.github.com/search/issues", () => { - const issues = [db.issue.findFirst({ where: { number: { equals: 1 } } })]; - return HttpResponse.json({ items: issues }); + http.get("https://api.github.com/search/issues", ({ request }) => { + const params = new URL(request.url).searchParams; + const query = params.get("q"); + const hasAssignee = query?.includes("assignee"); + if (hasAssignee) { + return HttpResponse.json(db.issue.getAll()); + } else { + return HttpResponse.json(db.pull.getAll()); + } }), // get issue by number http.get("https://api.github.com/repos/:owner/:repo/issues/:issue_number", ({ params: { owner, repo, issue_number: issueNumber } }) => diff --git a/tests/__mocks__/users-get.json b/tests/__mocks__/users-get.json index 0bb1a0e..b8b7bda 100644 --- a/tests/__mocks__/users-get.json +++ b/tests/__mocks__/users-get.json @@ -1,14 +1,27 @@ [ { "id": 1, - "login": "ubiquity" + "login": "ubiquity", + "role": "admin" }, { "id": 2, - "login": "user2" + "login": "user2", + "role": "contributor" }, { "id": 3, - "login": "user3" + "login": "user3", + "role": "collaborator" + }, + { + "id": 4, + "login": "user4", + "role": "new-start" + }, + { + "id": 5, + "login": "user5", + "role": "member" } ] diff --git a/tests/main.test.ts b/tests/main.test.ts index aa9d60f..3f349df 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -187,16 +187,19 @@ describe("User start/stop", () => { await expect(userStartStop(context)).rejects.toThrow("Skipping '/start' since the issue is a parent issue"); }); - test("User can't start another issue if they have reached the max limit", async () => { + test("should set maxLimits to 6 if the user is a member", async () => { const issue = db.issue.findFirst({ where: { id: { equals: 1 } } }) as unknown as Issue; - const sender = db.users.findFirst({ where: { id: { equals: 2 } } }) as unknown as PayloadSender; + const sender = db.users.findFirst({ where: { id: { equals: 5 } } }) as unknown as Sender; - const context = createContext(issue, sender) as Context<"issue_comment.created">; - context.config.maxConcurrentTasks = 1; + const memberLimit = maxConcurrentDefaults.member; - context.adapters = createAdapters(getSupabase(), context); + createIssuesForMaxAssignment(memberLimit + 4, sender.id); + const context = createContext(issue, sender) as unknown as Context; + context.adapters = createAdapters(getSupabase(), context as unknown as Context); await expect(userStartStop(context)).rejects.toThrow("You have reached your max task limit. Please close out some tasks before assigning new ones."); + + expect(memberLimit).toEqual(6); }); test("User can't start an issue if they have previously been unassigned by an admin", async () => { @@ -557,6 +560,23 @@ async function setupTests() { }); } +function createIssuesForMaxAssignment(n: number, userId: number) { + const user = db.users.findFirst({ where: { id: { equals: userId } } }); + for (let i = 0; i < n; i++) { + db.issue.create({ + ...issueTemplate, + id: i + 7, + assignee: user, + }); + } +} + +const maxConcurrentDefaults = { + admin: Infinity, + member: 6, + contributor: 4, +}; + function createContext( issue: Record, sender: Record, @@ -579,7 +599,7 @@ function createContext( config: { reviewDelayTolerance: "3 Days", taskStaleTimeoutDuration: "30 Days", - maxConcurrentTasks: 3, + maxConcurrentTasks: maxConcurrentDefaults, startRequiresWallet, emptyWalletText: "Please set your wallet address with the /wallet command first and try again.", },