Merge pull request #119 from gentlementlegen/fix/member-role

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": [
@@ -32,7 +32,7 @@
     "@octokit/types": "^13.6.2",
     "@sinclair/typebox": "0.34.3",
     "@supabase/supabase-js": "2.42.0",
-    "@ubiquity-os/plugin-sdk": "^2.0.0",
+    "@ubiquity-os/plugin-sdk": "^2.0.1",
     "@ubiquity-os/ubiquity-os-logger": "^1.3.2",
     "dotenv": "^16.4.7",
     "hono": "^4.6.14",
@@ -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,42 @@
-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 getTransformedRole(role: string) {
+  if (isAdminRole(role)) {
+    return "admin";
+  } else if (isCollaboratorRole(role)) {
+    return "collaborator";
+  }
+  return "contributor";
+}
+
+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 +52,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 +73,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 { getTransformedRole, getUserRoleAndTaskLimit } from "./get-user-task-limit-and-role";
 import structuredMetadata from "./structured-metadata";
 import { assignTableComment } from "./table";
 
@@ -21,6 +21,13 @@ async function checkRequirements(context: Context, issue: Context<"issue_comment
     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") {
+      return;
+    }
+
     if (!currentLabelConfiguration) {
       // If we didn't find the label in the allowed list, then the user cannot start this task.
       throw logger.error(
@@ -31,9 +38,13 @@ 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(userRole)) {
       // 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", {
+      const humanReadableRoles = [
+        ...currentLabelConfiguration.allowedRoles.map((o) => (o === "collaborator" ? "a core team member" : `a ${o}`)),
+        "an administrator",
+      ].join(", or ");
+      throw logger.error(`You must be ${humanReadableRoles} to start this task`, {
         currentLabelConfiguration,
         issueLabels,
         issue: issue.html_url,

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: [],
+    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"
 }