Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Max assignments #9

Merged
Merged
Show file tree
Hide file tree
Changes from 76 commits
Commits
Show all changes
82 commits
Select commit Hold shift + click to select a range
5b17673
fix: add validation to check for duplicate roles
jordan-ae Aug 15, 2024
61424ec
chore: update readme to match changes
jordan-ae Aug 15, 2024
07734d4
fix: user t.record and loop by object.entries
jordan-ae Aug 15, 2024
359a9a9
fix: use role as key and limit as value
jordan-ae Aug 15, 2024
3c2b73d
fix: get rid of unnecesary operstions on the role
jordan-ae Aug 15, 2024
542b57c
chore: use correct cases
jordan-ae Aug 18, 2024
3fd93c1
Update src/types/plugin-input.ts
jordan-ae Aug 24, 2024
20fb503
Update src/types/plugin-input.ts
jordan-ae Aug 27, 2024
2d559ed
Update src/types/plugin-input.ts
jordan-ae Aug 27, 2024
3897833
chore: revert to staticDecode
jordan-ae Aug 28, 2024
5faa1a0
chore: cleanup
jordan-ae Aug 28, 2024
57d0089
chore: rebase to development
jordan-ae Aug 29, 2024
744b559
chore: refactor isTaskStale using ms
Keyrxng Aug 13, 2024
1b4d509
chore: eslint
Keyrxng Aug 13, 2024
b7fc11b
chore: fix default, refactor isTaskStale
Keyrxng Aug 13, 2024
e834a7f
chore: remove redundant check
Keyrxng Aug 19, 2024
874fa10
chore: teamate assignment
Keyrxng Jul 15, 2024
3e7a586
chore: unassign command user
Keyrxng Jul 15, 2024
7a34e68
chore: teams test
Keyrxng Jul 15, 2024
b6f2490
chore: correct comment
Keyrxng Jul 15, 2024
535b4f5
chore: max assigned check all users
Keyrxng Jul 16, 2024
cb28796
chore: fix eslint naming convention
Keyrxng Jul 16, 2024
3826f9c
chore: update logs and test
Keyrxng Jul 29, 2024
a9236a8
chore: remove redunant logic
Keyrxng Aug 5, 2024
402666c
chore: throw errors and paginate
Keyrxng Aug 6, 2024
829a134
feat: custom message for private issues without plan
Keyrxng Aug 6, 2024
9a00cd0
chore: use octokit.rest
Keyrxng Aug 8, 2024
53fa77b
chore: throw logReturn and catch-all error comment
Keyrxng Aug 12, 2024
792627a
chore: catch other errors
Keyrxng Aug 12, 2024
ebff871
chore: update tests
Keyrxng Aug 12, 2024
087f088
chore: fix test by throwing Error
Keyrxng Aug 13, 2024
0708d75
chore: sanitize metadata comment
Keyrxng Aug 14, 2024
6405b1a
chore: improve sanity checks
Keyrxng Aug 14, 2024
fc97c1d
chore: log message
Keyrxng Aug 19, 2024
059010c
chore: bump logger and fix bubble-up error comment
Keyrxng Aug 21, 2024
1ab9112
fix: hide deadline if no time label
Keyrxng Aug 21, 2024
84a6d26
chore: formatting
Keyrxng Aug 24, 2024
13e0b4c
feat: added worker deploy / delete capabilities
gentlementlegen Aug 28, 2024
8d5c56a
feat: max task assignment for collaborators
Jul 14, 2024
9b7bbdf
feat: make task limits configurable
Jul 15, 2024
32b0456
fix: get contributors from config
Jul 22, 2024
09c5337
fix: remove union type
Jul 22, 2024
b4696c8
fix: return lowest task limit if user role doesnt match any of those …
Jul 23, 2024
c251f5b
chore: code cleanups
Jul 23, 2024
dce8531
test: add test to cover functionality
Jul 23, 2024
4b9dbaa
test: cleanups
Jul 29, 2024
c10accd
test: made requested changes
Jul 30, 2024
2a35496
test: cleanpup test
Jul 30, 2024
f308aca
fix: assignee issue and open pr fetching
Keyrxng Jul 31, 2024
675ccf1
chore: tests
Keyrxng Jul 31, 2024
a13b2ea
chore: ci
Keyrxng Jul 31, 2024
d62cfe5
fix: add validation to check for duplicate roles
jordan-ae Aug 15, 2024
a5bd14d
fix: user t.record and loop by object.entries
jordan-ae Aug 15, 2024
61294ab
chore: revert to staticDecode
jordan-ae Aug 28, 2024
0cd8a49
chore: rebase to development
jordan-ae Aug 29, 2024
ea30c4e
chore: rebase to development
jordan-ae Aug 29, 2024
c0658ee
Merge branch 'development' into max-assignments
jordan-ae Aug 29, 2024
d318b3b
chore: cleanups
jordan-ae Aug 29, 2024
a927772
Merge branch 'development' into max-assignments
jordan-ae Aug 30, 2024
579e7b4
Merge branch 'development' into max-assignments
jordan-ae Sep 2, 2024
0097358
clean up merge
jordan-ae Sep 2, 2024
1837930
test: fixed test to reflect merge
jordan-ae Sep 2, 2024
84c8465
fix: fix failling knip and double logging max limit error message
jordan-ae Sep 3, 2024
14f8e5f
fix: remove openedPullRequest checks for maxlimit
jordan-ae Sep 3, 2024
99ccab3
fix: remove duplicate maxTask checks
jordan-ae Sep 3, 2024
d0e17be
test: fix failing test
jordan-ae Sep 5, 2024
0a4ed48
Update src/types/plugin-input.ts
jordan-ae Sep 5, 2024
c2c924f
Update src/handlers/shared/start.ts
jordan-ae Sep 5, 2024
c36c956
chore: fix read me example indentation
jordan-ae Sep 5, 2024
0bfa5e0
fix: address duplicate logging on max limit
jordan-ae Sep 5, 2024
acb2388
chore: fix readme
jordan-ae Sep 5, 2024
3a80686
chore: cleanup code to avoid tenaries
jordan-ae Sep 6, 2024
94b7723
Merge branch 'development' into max-assignments
jordan-ae Sep 7, 2024
9416b25
fix: check limit per role correctly
jordan-ae Sep 7, 2024
bd4d1cb
chore: revery change in package json
jordan-ae Sep 7, 2024
ba92dd7
chore: remove optional access to the LogReturn
jordan-ae Sep 7, 2024
14f3ceb
chore: validate org login
jordan-ae Sep 7, 2024
b313fa8
chore: format with prettier
jordan-ae Sep 11, 2024
27156e1
Merge branch 'development' into max-assignments
jordan-ae Sep 11, 2024
106a7e5
Merge branch 'development' into max-assignments
jordan-ae Sep 11, 2024
a4df1b9
fix: cleanup merge
jordan-ae Sep 11, 2024
7b4e031
chore: remove limits for admins
jordan-ae Sep 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion README.md
jordan-ae marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ 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.
admin: 10
Copy link
Member

@0x4007 0x4007 Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like admins shouldn't have limits but to be honest I feel this feature has no teeth because we can just use the GitHub UI.

It would be more interesting if it unassigns when collaborators self assign and they hit the limit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0x4007 we can change UI assignments to follow the same logic as /start so user would get unassigned if they are doing too many tasks, even when forcing it through the UI.

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
33 changes: 33 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,33 @@
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 {
const response = await context.octokit.orgs.getMembershipForUser({
org: orgLogin as string,
jordan-ae marked this conversation as resolved.
Show resolved Hide resolved
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;
}
}
34 changes: 21 additions & 13 deletions src/handlers/shared/start.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
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";

export async function start(context: Context, issue: Context["payload"]["issue"], sender: Context["payload"]["sender"], 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 @@ -57,7 +58,7 @@ export async function start(context: Context, issue: Context["payload"]["issue"]
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 @@ -134,19 +135,26 @@ 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) >= limit) {
if (username !== sender) {
const logMessage = `${username} has reached their max task limit`;
const log = logger.error(logMessage, {
assignedIssues: assignedIssues.length,
openedPullRequests: openedPullRequests.length,
limit,
});

if (log.logMessage?.diff) {
await addCommentToIssue(context, log.logMessage.diff as string);
}
}

if (Math.abs(assignedIssues.length - openedPullRequests.length) >= maxConcurrentTasks) {
const log = 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,
});
await addCommentToIssue(context, log?.logMessage.diff as string);
return false;
}

Expand Down
5 changes: 4 additions & 1 deletion src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,8 @@ export async function startStopTask(inputs: PluginInputs, env: Env) {
}

function sanitizeMetadata(obj: LogReturn["metadata"]): string {
return JSON.stringify(obj, null, 2).replace(/</g, "&lt;").replace(/>/g, "&gt;").replace(/--/g, "&#45;&#45;");
return JSON.stringify(obj, null, 2)
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/--/g, "&#45;&#45;")
jordan-ae marked this conversation as resolved.
Show resolved Hide resolved
}
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
6 changes: 3 additions & 3 deletions src/types/plugin-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ 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: 20, member: 10, contributor: 2 } }),
emptyWalletText: T.String({ default: "Please set your wallet address with the /wallet command first and try again." }),
},
{
default: {},
default: {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this Prettier correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
default: {}
default: {},

}
);

export type StartStopSettings = StaticDecode<typeof startStopSchema>;
export const startStopSettingsValidator = new StandardValidator(startStopSchema);
export const startStopSettingsValidator = new StandardValidator(startStopSchema);
23 changes: 18 additions & 5 deletions src/utils/issue.ts
jordan-ae marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,31 @@ export function isParentIssue(body: string) {
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) => {
jordan-ae marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}


export async function addCommentToIssue(context: Context, message: string | null) {
const { payload, logger } = context;
if (!message) {
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 } }) => {
gentlementlegen marked this conversation as resolved.
Show resolved Hide resolved
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"
}
]
49 changes: 42 additions & 7 deletions tests/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,16 +172,34 @@ 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("should set maxLimits to 8 if the user is an admin", async () => {
const issue = db.issue.findFirst({ where: { id: { equals: 1 } } }) as unknown as Issue;
const sender = db.users.findFirst({ where: { id: { equals: 1 } } }) as unknown as Sender;

const adminLimit = maxConcurrentDefaults.admin;

createIssuesForMaxAssignment(adminLimit + 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(adminLimit).toEqual(8);
});

test("User can't start an issue if they have previously been unassigned by an admin", async () => {
Expand All @@ -208,7 +226,7 @@ describe("User start/stop", () => {
errorDetails.push(`${error.path}: ${error.message}`);
}

expect(errorDetails).toContain("/APP_ID: Required property");
expect(errorDetails).toContain("/APP_ID: Expected union value");
}
});

Expand Down Expand Up @@ -542,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: 8,
member: 6,
contributor: 4,
};

function createContext(
issue: Record<string, unknown>,
sender: Record<string, unknown>,
Expand All @@ -564,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