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: the warning for the disqualification is not posted on tasks with no price label #69

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
6 changes: 3 additions & 3 deletions dist/index.js

Large diffs are not rendered by default.

16 changes: 12 additions & 4 deletions src/handlers/watch-user-activity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { RestEndpointMethodTypes } from "@octokit/rest";
import { postComment } from "@ubiquity-os/plugin-sdk";
import { formatMillisecondsToHumanReadable } from "./time-format";
import { getWatchedRepos } from "../helpers/get-watched-repos";
import { parsePriorityLabel } from "../helpers/task-metadata";
import { parsePriceLabel, parsePriorityLabel } from "../helpers/task-metadata";
import { updateTaskReminder } from "../helpers/task-update";
import { ListForOrg } from "../types/github-types";
import { ContextPlugin } from "../types/plugin-input";
Expand Down Expand Up @@ -48,8 +48,16 @@ export async function watchUserActivity(context: ContextPlugin) {
return { message: "OK" };
}

/*
* We ignore the issue if:
* - draft
* - pull request
* - locked
* - not in "open" state
* - not priced (no price label found)
*/
function shouldIgnoreIssue(issue: IssueType) {
return issue.draft || issue.pull_request || issue.locked || issue.state !== "open";
return !!issue.draft || !!issue.pull_request || issue.locked || issue.state !== "open" || parsePriceLabel(issue.labels) === null;
Copy link
Member

Choose a reason for hiding this comment

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

why is !! needed if draft is a boolean?

Copy link
Member Author

Choose a reason for hiding this comment

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

Very interesting question, because I get a warning when I apply !! to locked which is a boolean, but no warning on draft, I suppose because it is boolean | undefined, but even without !! it gets coerced to a boolean. I had added that because this function returned boolean | {} due to pull_request. Will remove !!.

}

async function updateReminders(context: ContextPlugin, repo: ListForOrg["data"][0]) {
Expand All @@ -67,13 +75,13 @@ async function updateReminders(context: ContextPlugin, repo: ListForOrg["data"][

await Promise.all(
issues.map(async (issue) => {
// I think we can safely ignore the following
if (shouldIgnoreIssue(issue)) {
logger.debug(`Skipping issue ${issue.html_url} due to the issue not meeting the right criteria.`, {
logger.info(`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,
priceLabel: parsePriceLabel(issue.labels),
});
return;
}
Expand Down
11 changes: 11 additions & 0 deletions src/helpers/task-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,14 @@ export function parsePriorityLabel(labels: (IssueLabel | string)[]): number {

return 1;
}

export function parsePriceLabel(labels: (IssueLabel | string)[]): number | null {
const priceLabel = labels?.map((label) => (typeof label === "string" ? label : label.name || "")).find((name) => name.toLowerCase().startsWith("price:"));

if (!priceLabel) {
return null;
}

const matched = priceLabel.match(/price:\s*(\d+)/i);
return matched ? Number(matched[1]) : null;
}
28 changes: 24 additions & 4 deletions tests/assign.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { jest } from "@jest/globals";
import { beforeEach, describe, expect, it, jest } from "@jest/globals";
import { Logs } from "@ubiquity-os/ubiquity-os-logger";
import { ContextPlugin } from "../src/types/plugin-input";

Expand All @@ -13,7 +13,7 @@ jest.unstable_mockModule("@ubiquity-os/plugin-sdk", () => ({
const { watchUserActivity } = await import("../src/handlers/watch-user-activity");

describe("watchUserActivity", () => {
const mockContext = {
const mockContextTemplate = {
logger: new Logs("debug"),
eventName: "issues.assigned",
payload: {
Expand Down Expand Up @@ -46,14 +46,34 @@ describe("watchUserActivity", () => {
jest.clearAllMocks();
});

test("posts comment for matching repository issue", async () => {
it("should post comment for matching repository issue", async () => {
const spy = jest.spyOn(console, "warn");
await watchUserActivity(mockContext);
const mockContext = {
...mockContextTemplate,
};
mockContext.payload = {
...mockContextTemplate.payload,
issue: {
assignees: [{ login: "ubiquity-os" }],
title: "Test Issue",
state: "open",
labels: ["Price: 1 USD"],
},
} as unknown as ContextPlugin["payload"];

await watchUserActivity(mockContext);
expect(spy).toHaveBeenNthCalledWith(
1,
expect.stringMatching(/(Reminders will be sent every `1 hour` if there is no activity\.|Assignees will be disqualified after `2 hours` of inactivity\.)/)
);
spy.mockReset();
});

it("should ignore an un-priced task", async () => {
const spy = jest.spyOn(console, "info");

await watchUserActivity(mockContextTemplate);
expect(spy).not.toHaveBeenCalled();
spy.mockReset();
});
});
Loading