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

feat: task limit improvement #66

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5497d7e
feat: task limit improvement
ariesgun Oct 19, 2024
f00a9c6
feat: task limit improvement
ariesgun Oct 19, 2024
3e18c1b
fix: fixed failing test
ariesgun Oct 20, 2024
cd96022
fix: used API to query orgs where the app is installed
ariesgun Oct 20, 2024
0b63098
fix: added www. prefix
ariesgun Oct 20, 2024
58fd4da
fix: query network in getAllPRs
ariesgun Oct 21, 2024
734f97b
feat: update style
ariesgun Oct 22, 2024
ad20e32
fix: removed debug lines
ariesgun Oct 22, 2024
044f8a8
fix: fix format and type
ariesgun Oct 23, 2024
72cc120
fix: alternative way to list issues within network
ariesgun Oct 26, 2024
0953019
fix: update mock handlers
ariesgun Oct 26, 2024
2474312
fix: handle fetch error
ariesgun Oct 27, 2024
d09f2d4
fix: handle 404
ariesgun Oct 28, 2024
8bf9f6f
fix: revert change in package.json
ariesgun Oct 29, 2024
910ebf0
fix: update yarn.lock
ariesgun Oct 29, 2024
239d26e
fix: cleanup
ariesgun Oct 29, 2024
32e9280
feat: make issues and PRs search configurable
ubiquity-os[bot] Nov 3, 2024
d10db4a
Merge remote-tracking branch 'original/development' into task-limit-i…
ubiquity-os[bot] Nov 3, 2024
0a6bd74
fix: updated yarn.lock
ubiquity-os[bot] Nov 3, 2024
8fcedd9
fix: process review comments
ubiquity-os[bot] Nov 4, 2024
87e7b16
fix: renamed checkIssues to assignedIssueScope
ubiquity-os[bot] Nov 4, 2024
986261a
fix: convert AssignedIssueScope to enum
ubiquity-os[bot] Nov 4, 2024
d8fead7
fix: handle search API private repo
ubiquity-os[bot] Nov 4, 2024
3827d28
fix: undo changes in manifest
ubiquity-os[bot] Nov 5, 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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ To configure your Ubiquibot to run this plugin, add the following to the `.ubiqu
member: 5
contributor: 3
startRequiresWallet: true # default is true
assignedIssueScope: "org" # or "org" or "network". Default is org
emptyWalletText: "Please set your wallet address with the /wallet command first and try again."
rolesWithReviewAuthority: ["MEMBER", "OWNER"]
```
Expand Down
50 changes: 42 additions & 8 deletions src/handlers/shared/start.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Context, ISSUE_TYPE, Label } from "../../types";
import { AssignedIssue, Context, ISSUE_TYPE, Label } from "../../types";
import { isUserCollaborator } from "../../utils/get-user-association";
import { addAssignees, addCommentToIssue, getAssignedIssues, getAvailableOpenedPullRequests, getTimeValue, isParentIssue } from "../../utils/issue";
import { HttpStatusCode, Result } from "../result-types";
Expand Down Expand Up @@ -64,23 +64,51 @@ export async function start(
teammates.push(sender.login);

const toAssign = [];
let assignedIssues: AssignedIssue[] = [];
// check max assigned issues
for (const user of teammates) {
if (await handleTaskLimitChecks(user, context, logger, sender.login)) {
const { isWithinLimit, issues } = await handleTaskLimitChecks(user, context, logger, sender.login);
if (isWithinLimit) {
toAssign.push(user);
} else {
issues.forEach((issue) => {
assignedIssues = assignedIssues.concat({
title: issue.title,
html_url: issue.html_url,
});
});
}
}

let error: string | null = null;

if (toAssign.length === 0 && teammates.length > 1) {
error = "All teammates have reached their max task limit. Please close out some tasks before assigning new ones.";
throw logger.error(error, { issueNumber: issue.number });
} else if (toAssign.length === 0) {
error = "You have reached your max task limit. Please close out some tasks before assigning new ones.";
}
let issues = "";
const urlPattern = /https:\/\/(github.com\/(\S+)\/(\S+)\/issues\/(\d+))/;
assignedIssues.forEach((el) => {
const match = el.html_url.match(urlPattern);
if (match) {
issues = issues.concat(`- ###### [${match[2]}/${match[3]} - ${el.title} #${match[4]}](https://www.${match[1]})\n`);
} else {
issues = issues.concat(`- ###### [${el.title}](${el.html_url})\n`);
}
});

if (error) {
throw logger.error(error, { issueNumber: issue.number });
await addCommentToIssue(
context,
`

> [!WARNING]
> ${error}

${issues}

`
);
return { content: error, status: HttpStatusCode.NOT_MODIFIED };
}

const labels = issue.labels ?? [];
Expand Down Expand Up @@ -168,12 +196,18 @@ async function handleTaskLimitChecks(username: string, context: Context, logger:
limit,
});

return false;
return {
isWithinLimit: false,
issues: assignedIssues,
};
}

if (await hasUserBeenUnassigned(context, username)) {
throw logger.error(`${username} you were previously unassigned from this task. You cannot be reassigned.`, { username });
}

return true;
return {
isWithinLimit: true,
issues: [],
};
}
5 changes: 5 additions & 0 deletions src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { createAdapters } from "./adapters";
import { userPullRequest, userSelfAssign, userStartStop, userUnassigned } from "./handlers/user-start-stop";
import { Context, Env, PluginInputs } from "./types";
import { addCommentToIssue } from "./utils/issue";
import { listOrganizations } from "./utils/list-organizations";

export async function startStopTask(inputs: PluginInputs, env: Env) {
const customOctokit = Octokit.plugin(paginateGraphQL);
Expand All @@ -16,6 +17,7 @@ export async function startStopTask(inputs: PluginInputs, env: Env) {
eventName: inputs.eventName,
payload: inputs.eventPayload,
config: inputs.settings,
organizations: [],
Copy link
Member

Choose a reason for hiding this comment

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

This should not be here but in the configuration.

Copy link
Member

Choose a reason for hiding this comment

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

I think the reasoning is that it's needed at multiple places so they just put it in the context

Copy link
Member

Choose a reason for hiding this comment

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

The spec states:

Only check assigned issues within the network, but this should be configurable to either be just within the repo, org, or network

Shouldn't this be configurable? And even if not, putting it in the config also allows to add defaults on decode which would ensure it is properly initialized and types as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. It mentions the scope is configurable: repo, org , or network.. Dont understand what you meant

Copy link
Member

Choose a reason for hiding this comment

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

maybe I'm missing something, but the config item assignedIssueScope can be set to repo, org or network, this configuration property gets filled based on assignedIssueScope when the plugin starts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah that's how it behaves.. Based on assignedIssueScope, it will fill in the organization accordingly.

octokit,
env,
logger: new Logs("info"),
Expand All @@ -25,6 +27,9 @@ export async function startStopTask(inputs: PluginInputs, env: Env) {
context.adapters = createAdapters(supabase, context);

try {
const organizations = await listOrganizations(context);
context.organizations = organizations;

switch (context.eventName) {
case "issue_comment.created":
return await userStartStop(context);
Expand Down
1 change: 1 addition & 0 deletions src/types/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface Context<T extends SupportedEventsU = SupportedEventsU, TU exten
octokit: InstanceType<typeof Octokit> & paginateGraphQLInterface;
adapters: ReturnType<typeof createAdapters>;
config: PluginSettings;
organizations: string[];
env: Env;
logger: Logs;
}
6 changes: 6 additions & 0 deletions src/types/payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ 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 RepoIssues = RestEndpointMethodTypes["issues"]["listForRepo"]["response"]["data"];

export type AssignedIssue = {
title: string;
html_url: string;
};

export type Sender = { login: string; id: number };

Expand Down
7 changes: 7 additions & 0 deletions src/types/plugin-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ export interface PluginInputs<T extends SupportedEventsU = SupportedEventsU, TU
ref: string;
}

export enum AssignedIssueScope {
ORG = "org",
REPO = "repo",
NETWORK = "network",
}

const rolesWithReviewAuthority = T.Array(T.String(), { default: ["COLLABORATOR", "OWNER", "MEMBER", "ADMIN"] });

function maxConcurrentTasks() {
Expand Down Expand Up @@ -41,6 +47,7 @@ export const pluginSettingsSchema = T.Object(
taskStaleTimeoutDuration: T.String({ default: "30 Days" }),
startRequiresWallet: T.Boolean({ default: true }),
maxConcurrentTasks: maxConcurrentTasks(),
assignedIssueScope: T.Enum(AssignedIssueScope, { default: AssignedIssueScope.ORG }),
emptyWalletText: T.String({ default: "Please set your wallet address with the /wallet command first and try again." }),
rolesWithReviewAuthority: T.Transform(rolesWithReviewAuthority)
.Decode((value) => value.map((role) => role.toUpperCase()))
Expand Down
47 changes: 30 additions & 17 deletions src/utils/issue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,36 @@ import { RestEndpointMethodTypes } from "@octokit/rest";
import { Endpoints } from "@octokit/types";
import ms from "ms";
import { Context } from "../types/context";
import { GitHubIssueSearch, Review } from "../types/payload";
import { GitHubIssueSearch, RepoIssues, Review } from "../types/payload";
import { getLinkedPullRequests, GetLinkedResults } from "./get-linked-prs";
import { getAllPullRequestsFallback, getAssignedIssuesFallback } from "./get-pull-requests-fallback";
import { AssignedIssueScope } from "../types";

export function isParentIssue(body: string) {
const parentPattern = /-\s+\[( |x)\]\s+#\d+/;
return body.match(parentPattern);
}

export async function getAssignedIssues(context: Context, username: string) {
const payload = context.payload;
export async function getAssignedIssues(context: Context, username: string): Promise<GitHubIssueSearch["items"] | RepoIssues> {
let repoOrgQuery = "";
if (context.config.assignedIssueScope === AssignedIssueScope.REPO) {
repoOrgQuery = `repo:${context.payload.repository.full_name}`;
} else {
context.organizations.forEach((org) => {
repoOrgQuery += `org:${org} `;
});
}

try {
return await context.octokit
.paginate(context.octokit.rest.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));
})
);
const issues = await context.octokit.paginate(context.octokit.rest.search.issuesAndPullRequests, {
q: `${repoOrgQuery} is:open is:issue assignee:${username}`,
per_page: 100,
order: "desc",
sort: "created",
});
return issues.filter((issue) => {
return issue.assignee?.login === username || issue.assignees?.some((assignee) => assignee.login === username);
});
} catch (err) {
context.logger.info("Will try re-fetching assigned issues...", { error: err as Error });
return getAssignedIssuesFallback(context, username);
Expand Down Expand Up @@ -174,9 +179,17 @@ export async function addAssignees(context: Context, issueNo: number, assignees:
}

async function getAllPullRequests(context: Context, state: Endpoints["GET /repos/{owner}/{repo}/pulls"]["parameters"]["state"] = "open", username: string) {
const { payload } = context;
let repoOrgQuery = "";
if (context.config.assignedIssueScope === AssignedIssueScope.REPO) {
repoOrgQuery = `repo:${context.payload.repository.full_name}`;
} else {
context.organizations.forEach((org) => {
repoOrgQuery += `org:${org} `;
});
}

const query: RestEndpointMethodTypes["search"]["issuesAndPullRequests"]["parameters"] = {
q: `org:${payload.repository.owner.login} author:${username} state:${state}`,
q: `${repoOrgQuery} author:${username} state:${state} is:pr`,
per_page: 100,
order: "desc",
sort: "created",
Expand Down
38 changes: 38 additions & 0 deletions src/utils/list-organizations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { AssignedIssueScope, Context, GitHubIssueSearch } from "../types";

export async function listOrganizations(context: Context): Promise<string[]> {
const {
config: { assignedIssueScope },
logger,
payload,
} = context;

if (assignedIssueScope === AssignedIssueScope.REPO || assignedIssueScope === AssignedIssueScope.ORG) {
return [payload.repository.owner.login];
} else if (assignedIssueScope === AssignedIssueScope.NETWORK) {
const orgsSet: Set<string> = new Set();
const urlPattern = /https:\/\/github\.com\/(\S+)\/\S+\/issues\/\d+/;

const url = "https://raw.githubusercontent.com/ubiquity/devpool-directory/refs/heads/__STORAGE__/devpool-issues.json";
const response = await fetch(url);
if (!response.ok) {
if (response.status === 404) {
throw logger.error(`Error 404: unable to fetch file devpool-issues.json ${url}`);
} else {
throw logger.error("Error fetching file devpool-issues.json.", { status: response.status });
}
}

const devpoolIssues: GitHubIssueSearch["items"] = await response.json();
devpoolIssues.forEach((issue) => {
const match = issue.html_url.match(urlPattern);
if (match) {
orgsSet.add(match[1]);
}
});

return [...orgsSet];
}

throw new Error("Unknown assignedIssueScope value. Supported values: ['org', 'repo', 'network']");
}
1 change: 1 addition & 0 deletions tests/__mocks__/valid-configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"member": 10,
"contributor": 2
},
"assignedIssueScope": "org",
"emptyWalletText": "Please set your wallet address with the /wallet command first and try again.",
"rolesWithReviewAuthority": ["OWNER", "ADMIN", "MEMBER"]
}
3 changes: 2 additions & 1 deletion tests/configuration.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Value } from "@sinclair/typebox/value";
import { PluginSettings, pluginSettingsSchema } from "../src/types";
import { AssignedIssueScope, PluginSettings, pluginSettingsSchema } from "../src/types";
import cfg from "./__mocks__/valid-configuration.json";

describe("Configuration tests", () => {
Expand All @@ -8,6 +8,7 @@ describe("Configuration tests", () => {
reviewDelayTolerance: "1 Day",
taskStaleTimeoutDuration: "30 Days",
startRequiresWallet: true,
assignedIssueScope: AssignedIssueScope.ORG,
emptyWalletText: "Please set your wallet address with the /wallet command first and try again.",
maxConcurrentTasks: { admin: 20, member: 10, contributor: 2 },
rolesWithReviewAuthority: ["OWNER", "ADMIN", "MEMBER"],
Expand Down
12 changes: 8 additions & 4 deletions tests/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import { cleanLogString, Logs } from "@ubiquity-os/ubiquity-os-logger";
import dotenv from "dotenv";
import { createAdapters } from "../src/adapters";
import { userStartStop, userUnassigned } from "../src/handlers/user-start-stop";
import { Context, envConfigValidator, Sender, SupportedEventsU } from "../src/types";
import { AssignedIssueScope, Context, envConfigValidator, Sender, SupportedEventsU } from "../src/types";
import { db } from "./__mocks__/db";
import issueTemplate from "./__mocks__/issue-template";
import { server } from "./__mocks__/node";
import usersGet from "./__mocks__/users-get.json";
import { HttpStatusCode } from "../src/handlers/result-types";

dotenv.config();

Expand Down Expand Up @@ -221,9 +222,10 @@ describe("User start/stop", () => {
const context = createContext(issue, sender) as unknown as Context;

context.adapters = createAdapters(getSupabase(), context as unknown as Context);
await expect(userStartStop(context)).rejects.toMatchObject({
logMessage: { raw: "You have reached your max task limit. Please close out some tasks before assigning new ones." },
});

const { content, status } = await userStartStop(context);
expect(content).toEqual("You have reached your max task limit. Please close out some tasks before assigning new ones.");
expect(status).toEqual(HttpStatusCode.NOT_MODIFIED);

expect(memberLimit).toEqual(6);
});
Expand Down Expand Up @@ -629,11 +631,13 @@ export function createContext(
taskStaleTimeoutDuration: "30 Days",
maxConcurrentTasks: maxConcurrentDefaults,
startRequiresWallet,
assignedIssueScope: AssignedIssueScope.ORG,
emptyWalletText: "Please set your wallet address with the /wallet command first and try again.",
rolesWithReviewAuthority: ["ADMIN", "OWNER", "MEMBER"],
},
octokit: new octokit.Octokit(),
eventName: "issue_comment.created" as SupportedEventsU,
organizations: ["ubiquity"],
0x4007 marked this conversation as resolved.
Show resolved Hide resolved
env: {
SUPABASE_KEY: "key",
SUPABASE_URL: "url",
Expand Down
Loading