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

fix: improvements on follow-up #36

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
8b3a658
chore: debugging
gentlementlegen Oct 14, 2024
b65dc60
chore: debugging
gentlementlegen Oct 14, 2024
4c9f4d4
chore: updated manifest.json and dist build
ubiquity-os[bot] Oct 14, 2024
7615c38
refactor: update context type to ContextPlugin
gentlementlegen Oct 21, 2024
b488cf6
refactor: replace runPlugin with run in tests
gentlementlegen Oct 21, 2024
6b977a8
chore: set default log level using LOG_LEVEL constant
gentlementlegen Oct 21, 2024
2a485d5
chore: remove dist/index.cjs
gentlementlegen Oct 21, 2024
d7577a7
chore: remove dist/index.cjs
gentlementlegen Oct 21, 2024
7fef8f5
fix: changed logic to compute deadline
gentlementlegen Oct 21, 2024
732116f
chore: remove deprecated express_plugin mock
gentlementlegen Oct 21, 2024
501420e
chore: remove deprecated express_plugin mock
gentlementlegen Oct 21, 2024
1273044
chore: upgrade deps and remove unused packages
gentlementlegen Oct 21, 2024
5ffb9f4
chore: updated manifest.json and dist build
ubiquity-os[bot] Oct 21, 2024
9a77914
chore(workflows): add kernel payload signature to compute.yml
gentlementlegen Oct 21, 2024
2975fab
chore: add support for kernel public key
gentlementlegen Oct 21, 2024
9b181ea
chore: updated manifest.json and dist build
ubiquity-os[bot] Oct 21, 2024
0b04064
chore: add support for kernel public key
gentlementlegen Oct 21, 2024
f596f48
Merge remote-tracking branch 'meniole/fix/improvements' into fix/impr…
gentlementlegen Oct 21, 2024
588e667
chore: updated manifest.json and dist build
ubiquity-os[bot] Oct 21, 2024
642caca
chore: remove Node.js setup from compute workflow
gentlementlegen Oct 21, 2024
25152fd
chore: clean manifest.json
gentlementlegen Oct 21, 2024
f563ab4
chore: clean manifest.json
gentlementlegen Oct 21, 2024
cbc6c0b
chore: updated manifest.json and dist build
github-actions[bot] Oct 21, 2024
df3b02a
chore: clean manifest.json
gentlementlegen Oct 21, 2024
dda8ac3
Merge branch 'development' into fix/improvements
gentlementlegen Oct 21, 2024
df06946
chore: remove GitHub Actions payload parsing and validation
gentlementlegen Oct 21, 2024
ad4f799
chore: updated manifest.json and dist build
ubiquity-os[bot] Oct 21, 2024
92de0ec
fix: enable unassigning and reminding users for overdue tasks
gentlementlegen Oct 21, 2024
051cc9b
chore: updated manifest.json and dist build
ubiquity-os[bot] Oct 21, 2024
1aa44d1
refactor: optimize task update and improve reminder handling
gentlementlegen Oct 22, 2024
5d4b9f5
chore: integrate linked PR reminders and clean up logging
gentlementlegen Oct 22, 2024
1b42cb1
refactor: simplify task reminder update handling
gentlementlegen Oct 22, 2024
157cc4d
refactor: streamline task update logic
gentlementlegen Oct 22, 2024
f81afc0
chore: add pullRequestRequired flag to task config
gentlementlegen Oct 22, 2024
96fb39e
chore: updated manifest.json and dist build
github-actions[bot] Oct 22, 2024
f6bb37f
chore: remove task-deadline helper
gentlementlegen Oct 22, 2024
56bed4a
chore: update jest config and test setup for ESM support
gentlementlegen Oct 22, 2024
640b373
chore: enhance createEvent functionality and improve logging
gentlementlegen Oct 22, 2024
39d6718
chore: updated manifest.json and dist build
github-actions[bot] Oct 22, 2024
062771e
chore: handle missing event by logging error instead of throwing
gentlementlegen Oct 22, 2024
3e9f756
chore: updated manifest.json and dist build
github-actions[bot] Oct 22, 2024
0e15941
chore: handle missing event by logging error instead of throwing
gentlementlegen Oct 22, 2024
0a599d2
chore: handle missing event by logging error instead of throwing
gentlementlegen Oct 22, 2024
ab8fa91
Update .github/workflows/compute.yml
gentlementlegen Oct 23, 2024
2d4b355
refactor(tests): improve test event and comment handling
gentlementlegen Oct 23, 2024
a786bdb
refactor(tests): improve test event and comment handling
gentlementlegen Oct 23, 2024
34d0f68
refactor(tests): improve test event and comment handling
gentlementlegen Oct 23, 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
8 changes: 2 additions & 6 deletions .github/workflows/compute.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ on:
ref:
description: "Ref"
signature:
description: "The kernel signature"
description: "Used for authenticating requests from the kernel."

jobs:
compute:
Expand All @@ -27,16 +27,12 @@ jobs:
SUPABASE_URL: ${{ secrets.SUPABASE_URL }}
SUPABASE_KEY: ${{ secrets.SUPABASE_KEY }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
KERNEL_PUBLIC_KEY: ${{ secrets.KERNEL_PUBLIC_KEY }}

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: "20.10.0"

- run: ${{ toJSON(inputs) }}
shell: cat {0}

Expand Down
2 changes: 2 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ inputs:
description: "Auth Token to auth as the Kernel"
ref:
description: "GitHub branch reference for the run"
signature:
description: "The kernel signature"
outputs:
result:
description: "Actions taken by the watcher."
Expand Down
3 changes: 0 additions & 3 deletions dist/index.cjs

This file was deleted.

6 changes: 3 additions & 3 deletions dist/index.js

Large diffs are not rendered by default.

15 changes: 14 additions & 1 deletion jest.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,18 @@
"coverageReporters": ["json", "lcov", "text", "clover", "json-summary"],
"reporters": ["default", "jest-junit", "jest-md-dashboard"],
"coverageDirectory": "coverage",
"setupFiles": ["dotenv/config"]
"setupFiles": ["dotenv/config"],
"extensionsToTreatAsEsm": [".ts"],
"moduleFileExtensions": ["ts", "js", "json", "node"],
"transform": {
"^.+\\.ts?$": [
"ts-jest",
{
"useESM": true
}
]
},
"moduleNameMapper": {
"^(\\.{1,2}/.*)\\.js$": "$1"
}
}
10 changes: 6 additions & 4 deletions manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,23 @@
"type": "string"
}
}
},
"required": ["optOut"]
}
},
"disqualification": {
"default": "7 days",
"type": "string"
},
"pullRequestRequired": {
"default": true,
"type": "boolean"
},
"eventWhitelist": {
"default": ["pull_request.review_requested", "pull_request.ready_for_review", "pull_request_review_comment.created", "issue_comment.created", "push"],
"type": "array",
"items": {
"type": "string"
}
}
},
"required": ["warning", "watch", "disqualification", "eventWhitelist"]
}
}
}
20 changes: 9 additions & 11 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"knip": "knip --config .github/knip.ts",
"knip-ci": "knip --no-exit-code --reporter json --config .github/knip.ts",
"prepare": "husky install",
"test": "jest",
"test": "yarn node --experimental-vm-modules $(yarn bin jest)",
"supabase:generate:local": "supabase gen types typescript --local > src/types/database.ts",
"supabase:generate:remote": "cross-env-shell \"supabase gen types typescript --project-id $SUPABASE_PROJECT_ID --schema public > src/types/database.ts\""
},
Expand All @@ -30,18 +30,16 @@
"open-source"
],
"dependencies": {
"@actions/core": "1.10.1",
"@actions/github": "6.0.0",
"@octokit/graphql-schema": "^15.25.0",
"@octokit/rest": "20.1.1",
"@octokit/webhooks": "13.2.7",
"@sinclair/typebox": "0.32.31",
"@ubiquity-dao/ubiquibot-logger": "^1.3.1",
"@octokit/rest": "^21.0.2",
"@octokit/webhooks": "^13.3.0",
"@sinclair/typebox": "^0.32.35",
"@ubiquity-os/ubiquity-os-kernel": "^2.4.0",
"@ubiquity-os/ubiquity-os-logger": "^1.3.2",
"dotenv": "16.4.5",
"luxon": "3.4.4",
"ms": "2.1.3",
"tsx": "4.11.2",
"typebox-validators": "0.3.5"
"tsx": "4.11.2"
},
"devDependencies": {
"@commitlint/cli": "19.3.0",
Expand All @@ -54,7 +52,7 @@
"@types/jest": "29.5.12",
"@types/luxon": "3.4.2",
"@types/ms": "0.7.34",
"@types/node": "20.13.0",
"@types/node": "^22.7.7",
"cross-env": "7.0.3",
"cspell": "8.8.3",
"dotenv-cli": "7.4.2",
Expand All @@ -73,7 +71,7 @@
"prettier": "3.3.0",
"supabase": "1.176.9",
"ts-jest": "29.1.4",
"typescript": "5.6.2"
"typescript": "5.5.4"
},
"lint-staged": {
"*.ts": [
Expand Down
53 changes: 32 additions & 21 deletions src/handlers/watch-user-activity.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
import { getWatchedRepos } from "../helpers/get-watched-repos";
import { updateTaskReminder } from "../helpers/task-update";
import { Context } from "../types/context";
import { ListForOrg, ListIssueForRepo } from "../types/github-types";
import { ContextPlugin } from "../types/plugin-input";

export async function watchUserActivity(context: Context) {
export async function watchUserActivity(context: ContextPlugin) {
const { logger } = context;

const repos = await getWatchedRepos(context);

if (!repos?.length) {
logger.info("No watched repos have been found, no work to do.");
return false;
return { message: logger.info("No watched repos have been found, no work to do.").logMessage.raw };
}

for (const repo of repos) {
logger.debug(`> Watching user activity for repo: ${repo.name} (${repo.html_url})`);
await updateReminders(context, repo);
}
await Promise.all(
repos.map(async (repo) => {
logger.debug(`> Watching user activity for repo: ${repo.name} (${repo.html_url})`);
await updateReminders(context, repo);
})
);

return true;
return { message: "OK" };
Copy link
Member

Choose a reason for hiding this comment

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

How about a statusCode: 200 instead

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, will change it. Thought it was not useful because the network call already sends a 200, this is the body.

}

async function updateReminders(context: Context, repo: ListForOrg["data"][0]) {
async function updateReminders(context: ContextPlugin, repo: ListForOrg["data"][0]) {
const { logger, octokit, payload } = context;
const owner = payload.repository.owner?.login;
if (!owner) {
Expand All @@ -34,15 +35,25 @@ async function updateReminders(context: Context, repo: ListForOrg["data"][0]) {
state: "open",
})) as ListIssueForRepo[];

for (const issue of issues) {
// I think we can safely ignore the following
if (issue.draft || issue.pull_request || issue.locked || issue.state !== "open") {
continue;
}

if (issue.assignees?.length || issue.assignee) {
logger.debug(`Checking assigned issue: ${issue.html_url}`);
await updateTaskReminder(context, repo, issue);
}
}
await Promise.all(
issues.map(async (issue) => {
// I think we can safely ignore the following
if (issue.draft || issue.pull_request || issue.locked || issue.state !== "open") {
logger.debug(`Skipping issue ${issue.html_url} due to the issue not meeting the right criteria.`, {
draft: issue.draft,
pullRequest: !!issue.pull_request,
locked: issue.locked,
state: issue.state,
});
return;
}

if (issue.assignees?.length || issue.assignee) {
logger.debug(`Checking assigned issue: ${issue.html_url}`);
await updateTaskReminder(context, repo, issue);
} else {
logger.info(`Skipping issue ${issue.html_url} because no user is assigned.`);
}
})
);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Context } from "../types/context";
import { PullRequest, User, validate } from "@octokit/graphql-schema";
import { ContextPlugin } from "../types/plugin-input";

type closedByPullRequestsReferences = {
node: Pick<PullRequest, "url" | "title" | "number" | "state" | "body"> & Pick<User, "login" | "id">;
Expand All @@ -19,7 +19,7 @@ const query = /* GraphQL */ `
query collectLinkedPullRequests($owner: String!, $repo: String!, $issue_number: Int!) {
repository(owner: $owner, name: $repo) {
issue(number: $issue_number) {
closedByPullRequestsReferences(first: 100, includeClosedPrs: true) {
closedByPullRequestsReferences(first: 100, includeClosedPrs: false) {
edges {
node {
url
Expand Down Expand Up @@ -52,7 +52,7 @@ if (queryErrors.length > 1) {
}

export async function collectLinkedPullRequests(
context: Context,
context: ContextPlugin,
issue: {
owner: string;
repo: string;
Expand Down
14 changes: 8 additions & 6 deletions src/helpers/get-assignee-activity.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { DateTime } from "luxon";
import { collectLinkedPullRequests } from "../handlers/collect-linked-pulls";
import { Context } from "../types/context";
import { parseIssueUrl } from "./github-url";
import { collectLinkedPullRequests } from "./collect-linked-pulls";
import { GitHubTimelineEvents, ListIssueForRepo } from "../types/github-types";
import { ContextPlugin } from "../types/plugin-input";
import { parseIssueUrl } from "./github-url";

/**
* Retrieves all the activity for users that are assigned to the issue. Also takes into account linked pull requests.
*/
export async function getAssigneesActivityForIssue(context: Context, issue: ListIssueForRepo, assigneeIds: number[]) {
export async function getAssigneesActivityForIssue(context: ContextPlugin, issue: ListIssueForRepo, assigneeIds: number[]) {
const gitHubUrl = parseIssueUrl(issue.html_url);
const issueEvents: GitHubTimelineEvents[] = await context.octokit.paginate(context.octokit.rest.issues.listEventsForTimeline, {
owner: gitHubUrl.owner,
Expand Down Expand Up @@ -48,14 +48,15 @@ function filterEvents(issueEvents: GitHubTimelineEvents[], assigneeIds: number[]
}
actorId = userIdMap.get(actorLogin);
createdAt = event.created_at;
} else if (event.event === "committed") {
} else if ((event.event === "committed" || event.event === "commented") && "author" in event) {
const commitAuthor = "author" in event ? event.author : null;
const commitCommitter = "committer" in event ? event.committer : null;

if (commitAuthor || commitCommitter) {
assigneeEvents.push({
event: eventName,
created_at: createdAt,
created_at: event.author.date,
author: event.author.email,
});

continue;
Expand All @@ -66,6 +67,7 @@ function filterEvents(issueEvents: GitHubTimelineEvents[], assigneeIds: number[]
assigneeEvents.push({
event: eventName,
created_at: createdAt,
author: actorLogin,
});
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/helpers/get-watched-repos.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Context } from "../types/context";
import { Context } from "@ubiquity-os/ubiquity-os-kernel";
import { ListForOrg } from "../types/github-types";
import { ContextPlugin } from "../types/plugin-input";

export async function getWatchedRepos(context: Context) {
export async function getWatchedRepos(context: ContextPlugin) {
const {
config: {
watch: { optOut },
Expand Down
58 changes: 43 additions & 15 deletions src/helpers/remind-and-remove.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,37 @@
import { Context } from "../types/context";
import { parseIssueUrl } from "./github-url";
import { FOLLOWUP_HEADER, UNASSIGN_HEADER } from "../types/context";
import { ListIssueForRepo } from "../types/github-types";
import { ContextPlugin } from "../types/plugin-input";
import { collectLinkedPullRequests } from "./collect-linked-pulls";
import { parseIssueUrl } from "./github-url";
import { createStructuredMetadata } from "./structured-metadata";

export async function unassignUserFromIssue(context: Context, issue: ListIssueForRepo) {
export async function unassignUserFromIssue(context: ContextPlugin, issue: ListIssueForRepo) {
const { logger, config } = context;

if (config.disqualification <= 0) {
logger.info("The unassign threshold is <= 0, won't unassign users.");
} else {
logger.info(`Passed the deadline on ${issue.html_url} and no activity is detected, removing assignees.`);
await removeAllAssignees(context, issue);
}
}

export async function remindAssigneesForIssue(context: Context, issue: ListIssueForRepo) {
export async function remindAssigneesForIssue(context: ContextPlugin, issue: ListIssueForRepo) {
const { logger, config } = context;
const issueItem = parseIssueUrl(issue.html_url);

const hasLinkedPr = !!(await collectLinkedPullRequests(context, issueItem)).length;
if (config.warning <= 0) {
logger.info("The reminder threshold is <= 0, won't send any reminder.");
} else if (config.pullRequestRequired && !hasLinkedPr) {
await unassignUserFromIssue(context, issue);
} else {
logger.info(`Passed the reminder threshold on ${issue.html_url}, sending a reminder.`);
await remindAssignees(context, issue);
}
}

async function remindAssignees(context: Context, issue: ListIssueForRepo) {
const { octokit, logger } = context;
async function remindAssignees(context: ContextPlugin, issue: ListIssueForRepo) {
const { octokit, logger, config } = context;
const { repo, owner, issue_number } = parseIssueUrl(issue.html_url);

if (!issue?.assignees?.length) {
Expand All @@ -41,18 +47,31 @@ async function remindAssignees(context: Context, issue: ListIssueForRepo) {
taskAssignees: issue.assignees.map((o) => o?.id),
});

const metadata = createStructuredMetadata("Followup", logMessage);
const metadata = createStructuredMetadata(FOLLOWUP_HEADER, logMessage);

await octokit.rest.issues.createComment({
owner,
repo,
issue_number,
body: [logMessage.logMessage.raw, metadata].join("\n"),
});
if (!config.pullRequestRequired) {
await octokit.rest.issues.createComment({
owner,
repo,
issue_number,
body: [logMessage.logMessage.raw, metadata].join("\n"),
});
} else {
const pullRequests = await collectLinkedPullRequests(context, { repo, owner, issue_number });
for (const pullRequest of pullRequests) {
const { owner: prOwner, repo: prRepo, issue_number: prNumber } = parseIssueUrl(pullRequest.url);
await octokit.rest.issues.createComment({
owner: prOwner,
repo: prRepo,
issue_number: prNumber,
body: [logMessage.logMessage.raw, metadata].join("\n"),
});
}
}
return true;
}

async function removeAllAssignees(context: Context, issue: ListIssueForRepo) {
async function removeAllAssignees(context: ContextPlugin, issue: ListIssueForRepo) {
const { octokit, logger } = context;
const { repo, owner, issue_number } = parseIssueUrl(issue.html_url);

Expand All @@ -61,6 +80,15 @@ async function removeAllAssignees(context: Context, issue: ListIssueForRepo) {
return false;
}
const logins = issue.assignees.map((o) => o?.login).filter((o) => !!o) as string[];
const logMessage = logger.info(`Passed the deadline and no activity is detected, removing assignees: ${logins.map((o) => `@${o}`).join(", ")}.`);
const metadata = createStructuredMetadata(UNASSIGN_HEADER, logMessage);

await octokit.rest.issues.createComment({
owner,
repo,
issue_number,
body: [logMessage.logMessage.raw, metadata].join("\n"),
});
await octokit.rest.issues.removeAssignees({
owner,
repo,
Expand Down
Loading