test: added test for closing tasks

src/handlers/shared/start.ts

@@ -1,5 +1,4 @@
 import { AssignedIssue, Context, ISSUE_TYPE, Label } from "../../types";
-import { PricingError } from "../../types/errors";
 import { isUserCollaborator } from "../../utils/get-user-association";
 import { addAssignees, addCommentToIssue, getAssignedIssues, getPendingOpenedPullRequests, getTimeValue, isParentIssue } from "../../utils/issue";
 import { HttpStatusCode, Result } from "../result-types";
@@ -10,19 +9,21 @@ import { getTransformedRole, getUserRoleAndTaskLimit } from "./get-user-task-lim
 import structuredMetadata from "./structured-metadata";
 import { assignTableComment } from "./table";
 
-async function checkRequirements(context: Context, issue: Context<"issue_comment.created">["payload"]["issue"], login: string): Promise<Error | null> {
+async function checkRequirements(
+  context: Context,
+  issue: Context<"issue_comment.created">["payload"]["issue"],
+  userRole: ReturnType<typeof getTransformedRole>
+): Promise<Error | null> {
   const {
     config: { requiredLabelsToStart },
     logger,
   } = context;
   const issueLabels = issue.labels.map((label) => label.name.toLowerCase());
-  const userAssociation = await getUserRoleAndTaskLimit(context, login);
 
   if (requiredLabelsToStart.length) {
     const currentLabelConfiguration = requiredLabelsToStart.find((label) =>
       issueLabels.some((issueLabel) => label.name.toLowerCase() === issueLabel.toLowerCase())
     );
-    const userRole = getTransformedRole(userAssociation.role);
 
     // Admins can start any task
     if (userRole === "admin") {
@@ -49,7 +50,7 @@ async function checkRequirements(context: Context, issue: Context<"issue_comment
         currentLabelConfiguration,
         issueLabels,
         issue: issue.html_url,
-        userAssociation,
+        userRole,
       });
       return new Error(errorText);
     }
@@ -72,14 +73,17 @@ export async function start(
 
   const labels = issue.labels ?? [];
   const priceLabel = labels.find((label: Label) => label.name.startsWith("Price: "));
+  const userAssociation = await getUserRoleAndTaskLimit(context, sender.login);
+  const userRole = getTransformedRole(userAssociation.role);
 
   const startErrors: Error[] = [];
 
-  if (!priceLabel) {
-    startErrors.push(new PricingError(logger.error("No price label is set to calculate the duration", { issueNumber: issue.number }).logMessage.raw));
+  // Collaborators and admins can start un-priced tasks
+  if (!priceLabel && userRole === "contributor") {
+    startErrors.push(new Error(logger.error("No price label is set to calculate the duration", { issueNumber: issue.number }).logMessage.raw));
   }
 
-  const checkRequirementsError = await checkRequirements(context, issue, sender.login);
+  const checkRequirementsError = await checkRequirements(context, issue, userRole);
   if (checkRequirementsError) {
     startErrors.push(checkRequirementsError);
   }

src/handlers/user-start-stop.ts

@@ -1,6 +1,5 @@
 import { Repository } from "@octokit/graphql-schema";
 import { Context, isIssueCommentEvent, Label } from "../types";
-import { PricingError } from "../types/errors";
 import { QUERY_CLOSING_ISSUE_REFERENCES } from "../utils/get-closing-issue-references";
 import { closePullRequest, closePullRequestForAnIssue, getOwnerRepoFromHtmlUrl } from "../utils/issue";
 import { HttpStatusCode, Result } from "./result-types";
@@ -108,10 +107,7 @@ export async function userPullRequest(context: Context<"pull_request.opened" | "
         try {
           return await start(context, issueWithComment, pull_request.user ?? payload.sender, []);
         } catch (error) {
-          // We want to close the pull-request only if the PricingError is not present
-          if (!(error instanceof AggregateError) || !error.errors.some((e) => e instanceof PricingError)) {
-            await closePullRequest(context, { number: pull_request.number });
-          }
+          await closePullRequest(context, { number: pull_request.number });
           // Makes sure to concatenate error messages on AggregateError for proper display
           throw error instanceof AggregateError ? context.logger.error(error.errors.map(String).join("\n"), { error }) : error;
         }

tests/start.test.ts

@@ -11,6 +11,8 @@ import { createContext, getSupabase } from "./main.test";
 
 dotenv.config();
 
+const userLogin = "ubiquity-os-author";
+
 type Issue = Context<"issue_comment.created">["payload"]["issue"];
 type PayloadSender = Context["payload"]["sender"];
 
@@ -89,7 +91,7 @@ describe("Collaborator tests", () => {
       number: 1,
       user: {
         id: 1,
-        login: "ubiquity-os-author",
+        login: userLogin,
       },
     } as unknown as Context<"pull_request.edited">["payload"]["pull_request"];
     context.octokit = {
@@ -130,7 +132,81 @@ describe("Collaborator tests", () => {
     const { startStopTask } = await import("../src/plugin");
     await startStopTask(context);
     // Make sure the author is the one who starts and not the sender who modified the comment
-    expect(start).toHaveBeenCalledWith(expect.anything(), expect.anything(), { id: 1, login: "ubiquity-os-author" }, []);
+    expect(start).toHaveBeenCalledWith(expect.anything(), expect.anything(), { id: 1, login: userLogin }, []);
     start.mockReset();
   });
+  it("Should properly update the close status of a linked pull-request", async () => {
+    const issue = db.issue.findFirst({ where: { id: { equals: 1 } } }) as unknown as Issue;
+    issue.labels = [];
+    const sender = db.users.findFirst({ where: { id: { equals: 1 } } }) as unknown as PayloadSender;
+
+    const context = createContext(issue, sender, "") as Context<"pull_request.opened">;
+    context.eventName = "pull_request.opened";
+    context.payload.pull_request = {
+      html_url: "https://github.com/ubiquity-os-marketplace/command-start-stop",
+      number: 1,
+      user: {
+        id: 1,
+        login: userLogin,
+      },
+    } as unknown as Context<"pull_request.edited">["payload"]["pull_request"];
+    context.octokit = {
+      rest: {
+        pulls: {
+          update: jest.fn(),
+        },
+      },
+      graphql: {
+        paginate: jest.fn(() =>
+          Promise.resolve({
+            repository: {
+              pullRequest: {
+                closingIssuesReferences: {
+                  nodes: [
+                    {
+                      assignees: {
+                        nodes: [],
+                      },
+                      labels: {
+                        nodes: [{ name: "Time: <1 Hour" }],
+                      },
+                    },
+                  ],
+                },
+              },
+            },
+          })
+        ),
+      },
+    } as unknown as Context<"pull_request.edited">["octokit"];
+    jest.unstable_mockModule("@supabase/supabase-js", () => ({
+      createClient: jest.fn(),
+    }));
+    jest.unstable_mockModule("../src/adapters", () => ({
+      createAdapters: jest.fn(),
+    }));
+    const { startStopTask } = await import("../src/plugin");
+    await expect(startStopTask(context)).rejects.toMatchObject({
+      logMessage: {
+        raw: expect.stringContaining("No price label is set to calculate the duration"),
+      },
+    });
+    context.octokit = {
+      ...context.octokit,
+      //@ts-expect-error partial mock of the endpoint
+      paginate: jest.fn(() => []),
+      rest: {
+        ...context.octokit.rest,
+        orgs: {
+          //@ts-expect-error partial mock of the endpoint
+          getMembershipForUser: jest.fn(() => ({ data: { role: "member" } })),
+        },
+      },
+    };
+    await expect(startStopTask(context)).rejects.toMatchObject({
+      logMessage: {
+        raw: expect.stringContaining("This task does not reflect a business priority at the moment"),
+      },
+    });
+  });
 });