Skip to content

Commit

Permalink
Merge pull request #9 from ubq-test-jordan/max-assignments
Browse files Browse the repository at this point in the history
  • Loading branch information
0x4007 authored Sep 12, 2024
2 parents c13327c + 7b4e031 commit 182998e
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 36 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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."
```
Expand Down
37 changes: 37 additions & 0 deletions src/handlers/shared/get-user-task-limit-and-role.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Context } from "../../types";

interface MatchingUserProps {
role: string;
limit: number;
}

export async function getUserRoleAndTaskLimit(context: Context, user: string): Promise<MatchingUserProps> {
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;
}
}
20 changes: 11 additions & 9 deletions src/handlers/shared/start.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -14,7 +15,7 @@ export async function start(
teammates: string[]
): Promise<Result> {
const { logger, config } = context;
const { maxConcurrentTasks, taskStaleTimeoutDuration } = config;
const { taskStaleTimeoutDuration } = config;

// is it a child issue?
if (issue.body && isParentIssue(issue.body)) {
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions src/types/payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion src/types/plugin-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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." }),
},
{
Expand Down
21 changes: 15 additions & 6 deletions src/utils/issue.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,29 @@
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) {
const parentPattern = /-\s+\[( |x)\]\s+#\d+/;
return body.match(parentPattern);
}

export async function getAssignedIssues(context: Context, username: string): Promise<Issue[]> {
const { payload } = context;
export async function getAssignedIssues(context: Context, username: string): Promise<GitHubIssueSearch["items"]> {
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);
}
Expand Down
1 change: 1 addition & 0 deletions tests/__mocks__/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const db = factory({
users: {
id: primaryKey(Number),
login: String,
role: String,
},
issue: {
id: primaryKey(Number),
Expand Down
32 changes: 22 additions & 10 deletions tests/__mocks__/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } } }))
Expand All @@ -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 } } }))
Expand Down Expand Up @@ -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 } }) =>
Expand Down
19 changes: 16 additions & 3 deletions tests/__mocks__/users-get.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
32 changes: 26 additions & 6 deletions tests/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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<string, unknown>,
sender: Record<string, unknown>,
Expand All @@ -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.",
},
Expand Down

0 comments on commit 182998e

Please sign in to comment.