Transform issue and pull_request to unfurl shape in plugin (#9006)

* Transform issue and pull_request to unfurl shape in plugin * better typings * add todo
2025-12-20 01:59:56 -06:00 · 2025-04-22 16:30:44 +05:30
parent f86225c332
commit d37b7fa31e
4 changed files with 151 additions and 61 deletions
--- a/plugins/github/server/github.ts
+++ b/plugins/github/server/github.ts
@@ -4,36 +4,44 @@ import {
  type OAuthWebFlowAuthOptions,
  type InstallationAuthOptions,
 } from "@octokit/auth-app";
+import { Endpoints, OctokitResponse } from "@octokit/types";
 import { Octokit } from "octokit";
 import pluralize from "pluralize";
 import {
  IntegrationService,
  IntegrationType,
-  JSONObject,
  UnfurlResourceType,
 } from "@shared/types";
 import Logger from "@server/logging/Logger";
 import { Integration, User } from "@server/models";
-import { UnfurlSignature } from "@server/types";
+import { UnfurlIssueAndPR, UnfurlSignature } from "@server/types";
+import { GitHubUtils } from "../shared/GitHubUtils";
 import env from "./env";

+type PR =
+  Endpoints["GET /repos/{owner}/{repo}/pulls/{pull_number}"]["response"]["data"];
+type Issue =
+  Endpoints["GET /repos/{owner}/{repo}/issues/{issue_number}"]["response"]["data"];
+
 const requestPlugin = (octokit: Octokit) => ({
-  requestPR: async (params: ReturnType<typeof GitHub.parseUrl>) =>
-    octokit.request(`GET /repos/{owner}/{repo}/pulls/{id}`, {
-      owner: params?.owner,
-      repo: params?.repo,
-      id: params?.id,
+  requestPR: async (params: NonNullable<ReturnType<typeof GitHub.parseUrl>>) =>
+    octokit.request(`GET /repos/{owner}/{repo}/pulls/{pull_number}`, {
+      owner: params.owner,
+      repo: params.repo,
+      pull_number: params.id,
      headers: {
        Accept: "application/vnd.github.text+json",
        "X-GitHub-Api-Version": "2022-11-28",
      },
    }),

-  requestIssue: async (params: ReturnType<typeof GitHub.parseUrl>) =>
-    octokit.request(`GET /repos/{owner}/{repo}/issues/{id}`, {
-      owner: params?.owner,
-      repo: params?.repo,
-      id: params?.id,
+  requestIssue: async (
+    params: NonNullable<ReturnType<typeof GitHub.parseUrl>>
+  ) =>
+    octokit.request(`GET /repos/{owner}/{repo}/issues/{issue_number}`, {
+      owner: params.owner,
+      repo: params.repo,
+      issue_number: params.id,
      headers: {
        Accept: "application/vnd.github.text+json",
        "X-GitHub-Api-Version": "2022-11-28",
@@ -56,14 +64,14 @@ const requestPlugin = (octokit: Octokit) => ({
   */
  requestResource: async function requestResource(
    resource: ReturnType<typeof GitHub.parseUrl>
-  ): Promise<{ data?: JSONObject }> {
+  ): Promise<OctokitResponse<Issue | PR> | undefined> {
    switch (resource?.type) {
      case UnfurlResourceType.PR:
-        return this.requestPR(resource);
+        return this.requestPR(resource) as Promise<OctokitResponse<PR>>;
      case UnfurlResourceType.Issue:
-        return this.requestIssue(resource);
+        return this.requestIssue(resource) as Promise<OctokitResponse<Issue>>;
      default:
-        return { data: undefined };
+        return;
    }
  },

@@ -91,7 +99,10 @@ export class GitHub {

  private static appOctokit: Octokit;

-  private static supportedResources = Object.values(UnfurlResourceType);
+  private static supportedResources = [
+    UnfurlResourceType.Issue,
+    UnfurlResourceType.PR,
+  ];

  /**
   * Parses a given URL and returns resource identifiers for GitHub specific URLs
@@ -111,7 +122,7 @@ export class GitHub {
    const type = parts[3]
      ? (pluralize.singular(parts[3]) as UnfurlResourceType)
      : undefined;
-    const id = parts[4];
+    const id = Number(parts[4]);

    if (!type || !GitHub.supportedResources.includes(type)) {
      return;
@@ -204,14 +215,63 @@ export class GitHub {
      const client = await GitHub.authenticateAsInstallation(
        integration.settings.github!.installation.id
      );
-      const { data } = await client.requestResource(resource);
-      if (!data) {
+
+      const res = await client.requestResource(resource);
+      if (!res) {
        return { error: "Resource not found" };
      }
-      return { ...data, type: resource.type };
+
+      return GitHub.transformData(res.data, resource.type);
    } catch (err) {
      Logger.warn("Failed to fetch resource from GitHub", err);
      return { error: err.message || "Unknown error" };
    }
  };
+
+  private static transformData(data: Issue | PR, type: UnfurlResourceType) {
+    if (type === UnfurlResourceType.Issue) {
+      const issue = data as Issue;
+      return {
+        type: UnfurlResourceType.Issue,
+        url: issue.html_url,
+        id: `#${issue.number}`,
+        title: issue.title,
+        description: issue.body_text ?? null,
+        author: {
+          name: issue.user?.login ?? "",
+          avatarUrl: issue.user?.avatar_url ?? "",
+        },
+        labels: issue.labels.map((label: { name: string; color: string }) => ({
+          name: label.name,
+          color: `#${label.color}`,
+        })),
+        state: {
+          name: issue.state,
+          color: GitHubUtils.getColorForStatus(issue.state),
+        },
+        createdAt: issue.created_at,
+        transformed_unfurl: true,
+      } satisfies UnfurlIssueAndPR;
+    }
+
+    const pr = data as PR;
+    const prState = pr.merged ? "merged" : pr.state;
+    return {
+      type: UnfurlResourceType.PR,
+      url: pr.html_url,
+      id: `#${pr.number}`,
+      title: pr.title,
+      description: pr.body,
+      author: {
+        name: pr.user.login,
+        avatarUrl: pr.user.avatar_url,
+      },
+      state: {
+        name: prState,
+        color: GitHubUtils.getColorForStatus(prState),
+      },
+      createdAt: pr.created_at,
+      transformed_unfurl: true,
+    } satisfies UnfurlIssueAndPR;
+  }
 }
--- a/server/presenters/unfurl.ts
+++ b/server/presenters/unfurl.ts
@@ -71,47 +71,63 @@ const presentDocument = (

 const presentPR = (
  data: Record<string, any>
-): UnfurlResponse[UnfurlResourceType.PR] => ({
-  url: data.html_url,
-  type: UnfurlResourceType.PR,
-  id: `#${data.number}`,
-  title: data.title,
-  description: data.body,
-  author: {
-    name: data.user.login,
-    avatarUrl: data.user.avatar_url,
-  },
-  state: {
-    name: data.merged ? "merged" : data.state,
-    color: GitHubUtils.getColorForStatus(data.merged ? "merged" : data.state),
-  },
-  createdAt: data.created_at,
-});
+): UnfurlResponse[UnfurlResourceType.PR] => {
+  // TODO: For backwards compatibility, remove once cache has expired in next release.
+  if (data.transformed_unfurl) {
+    delete data.transformed_unfurl;
+    return data as UnfurlResponse[UnfurlResourceType.PR]; // this would have been transformed by the unfurl plugin.
+  }
+
+  return {
+    url: data.html_url,
+    type: UnfurlResourceType.PR,
+    id: `#${data.number}`,
+    title: data.title,
+    description: data.body,
+    author: {
+      name: data.user.login,
+      avatarUrl: data.user.avatar_url,
+    },
+    state: {
+      name: data.merged ? "merged" : data.state,
+      color: GitHubUtils.getColorForStatus(data.merged ? "merged" : data.state),
+    },
+    createdAt: data.created_at,
+  };
+};

 const presentIssue = (
  data: Record<string, any>
-): UnfurlResponse[UnfurlResourceType.Issue] => ({
-  url: data.html_url,
-  type: UnfurlResourceType.Issue,
-  id: `#${data.number}`,
-  title: data.title,
-  description: data.body_text,
-  author: {
-    name: data.user.login,
-    avatarUrl: data.user.avatar_url,
-  },
-  labels: data.labels.map((label: { name: string; color: string }) => ({
-    name: label.name,
-    color: `#${label.color}`,
-  })),
-  state: {
-    name: data.state,
-    color: GitHubUtils.getColorForStatus(
-      data.state === "closed" ? "done" : data.state
-    ),
-  },
-  createdAt: data.created_at,
-});
+): UnfurlResponse[UnfurlResourceType.Issue] => {
+  // TODO: For backwards compatibility, remove once cache has expired in next release.
+  if (data.transformed_unfurl) {
+    delete data.transformed_unfurl;
+    return data as UnfurlResponse[UnfurlResourceType.Issue]; // this would have been transformed by the unfurl plugin.
+  }
+
+  return {
+    url: data.html_url,
+    type: UnfurlResourceType.Issue,
+    id: `#${data.number}`,
+    title: data.title,
+    description: data.body_text,
+    author: {
+      name: data.user.login,
+      avatarUrl: data.user.avatar_url,
+    },
+    labels: data.labels.map((label: { name: string; color: string }) => ({
+      name: label.name,
+      color: `#${label.color}`,
+    })),
+    state: {
+      name: data.state,
+      color: GitHubUtils.getColorForStatus(
+        data.state === "closed" ? "done" : data.state
+      ),
+    },
+    createdAt: data.created_at,
+  };
+};

 const presentLastOnlineInfoFor = (user: User) => {
  const locale = dateLocale(user.language);
--- a/server/routes/api/urls/urls.ts
+++ b/server/routes/api/urls/urls.ts
@@ -100,7 +100,7 @@ router.post(
    for (const plugin of plugins) {
      const unfurl = await plugin.value.unfurl(url, actor);
      if (unfurl) {
-        if (unfurl.error) {
+        if ("error" in unfurl) {
          return (ctx.response.status = 204);
        } else {
          const data = unfurl as Unfurl;
--- a/server/types.ts
+++ b/server/types.ts
@@ -10,6 +10,7 @@ import {
  JSONValue,
  UnfurlResourceType,
  ProsemirrorData,
+  UnfurlResponse,
 } from "@shared/types";
 import { BaseSchema } from "@server/routes/api/schema";
 import { AccountProvisionerResult } from "./commands/accountProvisioner";
@@ -576,7 +577,20 @@ export type CollectionJSONExport = {
  };
 };

-export type Unfurl = { [x: string]: JSONValue; type: UnfurlResourceType };
+export type UnfurlIssueAndPR = (
+  | UnfurlResponse[UnfurlResourceType.Issue]
+  | UnfurlResponse[UnfurlResourceType.PR]
+) & { transformed_unfurl: true };
+
+export type Unfurl =
+  | UnfurlIssueAndPR
+  | {
+      type: Exclude<
+        UnfurlResourceType,
+        UnfurlResourceType.Issue | UnfurlResourceType.PR
+      >;
+      [x: string]: JSONValue;
+    };

 export type UnfurlError = { error: string };