fix: simplified member role in configuration

README.md

@@ -35,15 +35,15 @@ To configure your Ubiquity Kernel to run this plugin, add the following to the `
     reviewDelayTolerance: "3 Days"
     taskStaleTimeoutDuration: "30 Days"
     maxConcurrentTasks: # Default concurrent task limits per role.
-      member: 5
+      collaborator: 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"]
     requiredLabelsToStart:
       - name: "Priority: 5 (Emergency)"
-        roles: ["admin", "collaborator", "write"]
+        allowedRoles: ["contributor", "collaborator"]
 ```
 
 # Testing

manifest.json

@@ -51,21 +51,23 @@
         "type": "boolean"
       },
       "maxConcurrentTasks": {
-        "default": {
-          "member": 10,
-          "contributor": 2
-        },
         "description": "The maximum number of tasks a user can have assigned to them at once, based on their role.",
         "examples": [
           {
-            "member": 5,
-            "contributor": 1
+            "collaborator": 10,
+            "contributor": 2
           }
         ],
+        "default": {},
         "type": "object",
-        "patternProperties": {
-          "^(.*)$": {
-            "type": "integer"
+        "properties": {
+          "collaborator": {
+            "default": 10,
+            "type": "number"
+          },
+          "contributor": {
+            "default": 2,
+            "type": "number"
           }
         }
       },
@@ -135,44 +137,21 @@
               "description": "The name of the required labels to start the task.",
               "type": "string"
             },
-            "roles": {
+            "allowedRoles": {
               "description": "The list of allowed roles to start the task with the given label.",
               "uniqueItems": true,
-              "default": ["admin", "member", "collaborator", "contributor", "owner", "billing_manager", "write", "read"],
+              "default": ["collaborator", "contributor"],
+              "examples": [["collaborator", "contributor"]],
               "type": "array",
               "items": {
                 "anyOf": [
-                  {
-                    "const": "admin",
-                    "type": "string"
-                  },
-                  {
-                    "const": "member",
-                    "type": "string"
-                  },
                   {
                     "const": "collaborator",
                     "type": "string"
                   },
                   {
                     "const": "contributor",
                     "type": "string"
-                  },
-                  {
-                    "const": "owner",
-                    "type": "string"
-                  },
-                  {
-                    "const": "billing_manager",
-                    "type": "string"
-                  },
-                  {
-                    "const": "read",
-                    "type": "string"
-                  },
-                  {
-                    "const": "write",
-                    "type": "string"
                   }
                 ]
               }

package.json

@@ -16,7 +16,7 @@
     "knip": "knip --config .github/knip.ts",
     "knip-ci": "knip --no-exit-code --reporter json --config .github/knip.ts",
     "prepare": "husky install",
-    "test": "jest --setupFiles dotenv/config --coverage",
+    "test": "cross-env NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules\" jest --setupFiles dotenv/config --coverage",
     "worker": "wrangler dev --env dev --port 4000"
   },
   "keywords": [
@@ -51,6 +51,7 @@
     "@types/jest": "29.5.12",
     "@types/ms": "^0.7.34",
     "@types/node": "20.14.5",
+    "cross-env": "^7.0.3",
     "cspell": "8.9.0",
     "eslint": "9.14.0",
     "eslint-config-prettier": "9.1.0",

src/handlers/shared/get-user-task-limit-and-role.ts

@@ -1,20 +1,33 @@
-import { Context } from "../../types";
+import { ADMIN_ROLES, COLLABORATOR_ROLES, Context, PluginSettings } from "../../types";
 
 interface MatchingUserProps {
   role: string;
   limit: number;
 }
 
+export function isAdminRole(role: string) {
+  return ADMIN_ROLES.includes(role.toLowerCase());
+}
+
+export function isCollaboratorRole(role: string) {
+  return COLLABORATOR_ROLES.includes(role.toLowerCase());
+}
+
+export function getUserTaskLimit(maxConcurrentTasks: PluginSettings["maxConcurrentTasks"], role: string) {
+  if (isAdminRole(role)) {
+    return Infinity;
+  }
+  if (isCollaboratorRole(role)) {
+    return maxConcurrentTasks.collaborator;
+  }
+  return maxConcurrentTasks.contributor;
+}
+
 export async function getUserRoleAndTaskLimit(context: Context, user: string): Promise<MatchingUserProps> {
   const orgLogin = context.payload.organization?.login;
   const { config, logger, octokit } = context;
   const { maxConcurrentTasks } = config;
 
-  const minUserTaskLimit = Object.entries(maxConcurrentTasks).reduce((minTask, [role, limit]) => (limit < minTask.limit ? { role, limit } : minTask), {
-    role: "",
-    limit: Infinity,
-  } as MatchingUserProps);
-
   try {
     // Validate the organization login
     if (typeof orgLogin !== "string" || orgLogin.trim() === "") {
@@ -30,7 +43,7 @@ export async function getUserRoleAndTaskLimit(context: Context, user: string): P
         username: user,
       });
       role = response.data.role.toLowerCase();
-      limit = maxConcurrentTasks[role] ?? Infinity;
+      limit = getUserTaskLimit(maxConcurrentTasks, role);
       return { role, limit };
     } catch (err) {
       logger.error("Could not get user membership", { err });
@@ -51,11 +64,11 @@ export async function getUserRoleAndTaskLimit(context: Context, user: string): P
       role,
       data: permissionLevel.data,
     });
-    limit = maxConcurrentTasks[role] ?? Infinity;
+    limit = getUserTaskLimit(maxConcurrentTasks, role);
 
     return { role, limit };
   } catch (err) {
     logger.error("Could not get user role", { err });
-    return minUserTaskLimit;
+    return { role: "unknown", limit: maxConcurrentTasks.contributor };
   }
 }

src/handlers/shared/start.ts

@@ -5,7 +5,7 @@ import { HttpStatusCode, Result } from "../result-types";
 import { hasUserBeenUnassigned } from "./check-assignments";
 import { checkTaskStale } from "./check-task-stale";
 import { generateAssignmentComment } from "./generate-assignment-comment";
-import { getUserRoleAndTaskLimit } from "./get-user-task-limit-and-role";
+import { getUserRoleAndTaskLimit, isAdminRole, isCollaboratorRole } from "./get-user-task-limit-and-role";
 import structuredMetadata from "./structured-metadata";
 import { assignTableComment } from "./table";
 
@@ -31,7 +31,11 @@ async function checkRequirements(context: Context, issue: Context<"issue_comment
           issue: issue.html_url,
         }
       );
-    } else if (!currentLabelConfiguration.roles.includes(userAssociation.role.toLowerCase() as (typeof currentLabelConfiguration.roles)[number])) {
+    } else if (
+      !currentLabelConfiguration.allowedRoles.includes("contributor") &&
+      !isAdminRole(userAssociation.role) &&
+      !isCollaboratorRole(userAssociation.role)
+    ) {
       // If we found the label in the allowed list, but the user role does not match the allowed roles, then the user cannot start this task.
       throw logger.error("You must be a core team member to start this task", {
         currentLabelConfiguration,

src/types/plugin-input.ts

@@ -1,4 +1,4 @@
-import { StaticDecode, TLiteral, Type as T, Union } from "@sinclair/typebox";
+import { StaticDecode, Type as T } from "@sinclair/typebox";
 
 export enum AssignedIssueScope {
   ORG = "org",
@@ -13,6 +13,11 @@ export enum Role {
   COLLABORATOR = "COLLABORATOR",
 }
 
+// These correspond to getMembershipForUser and getCollaboratorPermissionLevel for a user.
+// Anything outside these values is considered to be a contributor (external user).
+export const ADMIN_ROLES = ["admin", "owner", "billing_manager"];
+export const COLLABORATOR_ROLES = ["write", "member", "collaborator"];
+
 const rolesWithReviewAuthority = T.Array(T.Enum(Role), {
   default: [Role.OWNER, Role.ADMIN, Role.MEMBER, Role.COLLABORATOR],
   uniqueItems: true,
@@ -23,47 +28,24 @@ const rolesWithReviewAuthority = T.Array(T.Enum(Role), {
   ],
 });
 
-const maxConcurrentTasks = T.Transform(
-  T.Record(T.String(), T.Integer(), {
-    default: { member: 10, contributor: 2 },
+const maxConcurrentTasks = T.Object(
+  { collaborator: T.Number({ default: 10 }), contributor: T.Number({ default: 2 }) },
+  {
     description: "The maximum number of tasks a user can have assigned to them at once, based on their role.",
-    examples: [{ member: 5, contributor: 1 }],
-  })
-)
-  .Decode((obj) => {
-    // normalize the role keys to lowercase
-    obj = Object.keys(obj).reduce(
-      (acc, key) => {
-        acc[key.toLowerCase()] = obj[key];
-        return acc;
-      },
-      {} as Record<string, number>
-    );
-
-    // If admin is omitted, defaults to infinity
-    if (!obj["admin"]) {
-      obj["admin"] = Infinity;
-    }
-
-    return obj;
-  })
-  .Encode((value) => value);
-
-type IntoStringLiteralUnion<T> = { [K in keyof T]: T[K] extends string ? TLiteral<T[K]> : never };
-
-export function stringLiteralUnion<T extends string[]>(values: [...T]): Union<IntoStringLiteralUnion<T>> {
-  const literals = values.map((value) => T.Literal(value));
-  return T.Union(literals) as Union<IntoStringLiteralUnion<T>>;
-}
+    examples: [{ collaborator: 10, contributor: 2 }],
+    default: {},
+  }
+);
 
-const roles = stringLiteralUnion(["admin", "member", "collaborator", "contributor", "owner", "billing_manager", "read", "write"]);
+const roles = T.KeyOf(maxConcurrentTasks);
 
 const requiredLabel = T.Object({
   name: T.String({ description: "The name of the required labels to start the task." }),
-  roles: T.Array(roles, {
+  allowedRoles: T.Array(roles, {
     description: "The list of allowed roles to start the task with the given label.",
     uniqueItems: true,
-    default: ["admin", "member", "collaborator", "contributor", "owner", "billing_manager", "write", "read"],
+    default: ["collaborator", "contributor"],
+    examples: [["collaborator", "contributor"]],
   }),
 });
 

tests/__mocks__/valid-configuration.json

@@ -1,35 +1,34 @@
 {
-  "reviewDelayTolerance": "1 Day",
-  "taskStaleTimeoutDuration": "30 Days",
-  "startRequiresWallet": true,
+  "assignedIssueScope": "org",
+  "emptyWalletText": "Please set your wallet address with the /wallet command first and try again.",
   "maxConcurrentTasks": {
-    "admin": 20,
-    "member": 10,
+    "collaborator": 10,
     "contributor": 2
   },
-  "assignedIssueScope": "org",
-  "emptyWalletText": "Please set your wallet address with the /wallet command first and try again.",
-  "rolesWithReviewAuthority": ["OWNER", "ADMIN", "MEMBER"],
   "requiredLabelsToStart": [
     {
-      "name": "Priority: 1 (Normal)",
-      "roles": ["admin", "member", "contributor", "owner", "billing_manager"]
+      "allowedRoles": ["collaborator", "contributor"],
+      "name": "Priority: 1 (Normal)"
     },
     {
-      "name": "Priority: 2 (Medium)",
-      "roles": ["admin", "member", "contributor", "owner", "billing_manager"]
+      "allowedRoles": ["collaborator", "contributor"],
+      "name": "Priority: 2 (Medium)"
     },
     {
-      "name": "Priority: 3 (High)",
-      "roles": ["admin", "member", "contributor", "owner", "billing_manager"]
+      "allowedRoles": ["collaborator", "contributor"],
+      "name": "Priority: 3 (High)"
     },
     {
-      "name": "Priority: 4 (Urgent)",
-      "roles": ["admin", "member", "contributor", "owner", "billing_manager"]
+      "allowedRoles": ["collaborator", "contributor"],
+      "name": "Priority: 4 (Urgent)"
     },
     {
-      "name": "Priority: 5 (Emergency)",
-      "roles": ["admin", "member", "contributor", "owner", "billing_manager"]
+      "allowedRoles": ["collaborator", "contributor"],
+      "name": "Priority: 5 (Emergency)"
     }
-  ]
+  ],
+  "reviewDelayTolerance": "1 Day",
+  "rolesWithReviewAuthority": ["OWNER", "ADMIN", "MEMBER"],
+  "startRequiresWallet": true,
+  "taskStaleTimeoutDuration": "30 Days"
 }

tests/configuration.test.ts

@@ -3,11 +3,11 @@ import { AssignedIssueScope, PluginSettings, pluginSettingsSchema, Role } from "
 import cfg from "./__mocks__/valid-configuration.json";
 
 const PRIORITY_LABELS = [
-  { name: "Priority: 1 (Normal)", roles: ["admin", "member", "contributor", "owner", "billing_manager"] },
-  { name: "Priority: 2 (Medium)", roles: ["admin", "member", "contributor", "owner", "billing_manager"] },
-  { name: "Priority: 3 (High)", roles: ["admin", "member", "contributor", "owner", "billing_manager"] },
-  { name: "Priority: 4 (Urgent)", roles: ["admin", "member", "contributor", "owner", "billing_manager"] },
-  { name: "Priority: 5 (Emergency)", roles: ["admin", "member", "contributor", "owner", "billing_manager"] },
+  { name: "Priority: 1 (Normal)", allowedRoles: ["collaborator", "contributor"] },
+  { name: "Priority: 2 (Medium)", allowedRoles: ["collaborator", "contributor"] },
+  { name: "Priority: 3 (High)", allowedRoles: ["collaborator", "contributor"] },
+  { name: "Priority: 4 (Urgent)", allowedRoles: ["collaborator", "contributor"] },
+  { name: "Priority: 5 (Emergency)", allowedRoles: ["collaborator", "contributor"] },
 ];
 
 describe("Configuration tests", () => {
@@ -18,18 +18,18 @@ describe("Configuration tests", () => {
       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 },
+      maxConcurrentTasks: { collaborator: 10, contributor: 2 },
       rolesWithReviewAuthority: [Role.OWNER, Role.ADMIN, Role.MEMBER],
       requiredLabelsToStart: PRIORITY_LABELS,
     }) as PluginSettings;
     expect(settings).toEqual(cfg);
   });
-  it("Should default the admin to infinity if missing from config when decoded", () => {
+  it("Should give the collaborator limits of PRs", () => {
     const settings = Value.Default(pluginSettingsSchema, {
       requiredLabelsToStart: PRIORITY_LABELS,
     }) as PluginSettings;
     console.dir([...Value.Errors(pluginSettingsSchema, settings)]);
     const decodedSettings = Value.Decode(pluginSettingsSchema, settings);
-    expect(decodedSettings.maxConcurrentTasks["admin"]).toEqual(Infinity);
+    expect(decodedSettings.maxConcurrentTasks["collaborator"]).toEqual(10);
   });
 });

tests/fallbacks.test.ts

@@ -1,3 +1,4 @@
+import { jest } from "@jest/globals";
 import { RestEndpointMethodTypes } from "@octokit/plugin-rest-endpoint-methods";
 import { Logs } from "@ubiquity-os/ubiquity-os-logger";
 import { Context } from "../src/types/context";
@@ -11,13 +12,13 @@ const mockPullRequestData = [
 ] as unknown as RestEndpointMethodTypes["pulls"]["list"]["response"]["data"];
 
 const mockOctokit = {
-  paginate: jest.fn().mockResolvedValue(mockPullRequestData),
+  paginate: jest.fn(() => mockPullRequestData),
   rest: {
     pulls: {
-      list: jest.fn().mockResolvedValue(mockPullRequestData),
+      list: jest.fn(() => mockPullRequestData),
     },
     repos: {
-      listForOrg: jest.fn().mockResolvedValue(mockPullRequestData),
+      listForOrg: jest.fn(() => mockPullRequestData),
     },
   },
 };