From 483dcf0210fcf04da214a48925f3fc52a37a8b26 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 21 Apr 2026 09:18:18 -0500 Subject: [PATCH] fix: address issue reference review findings Co-Authored-By: Paperclip --- .../src/migrations/0060_orange_annihilus.sql | 18 +++++++- .../db/src/migrations/meta/0060_snapshot.json | 43 +++++++++++++++++-- .../db/src/schema/issue_reference_mentions.ts | 11 ++++- packages/shared/src/issue-references.test.ts | 6 +++ packages/shared/src/issue-references.ts | 5 ++- server/src/services/issue-references.ts | 40 +++++++++-------- ui/src/components/IssueReferencePill.tsx | 39 ++++++++++++----- 7 files changed, 126 insertions(+), 36 deletions(-) diff --git a/packages/db/src/migrations/0060_orange_annihilus.sql b/packages/db/src/migrations/0060_orange_annihilus.sql index 1fe52c9e176..2536a3ae4be 100644 --- a/packages/db/src/migrations/0060_orange_annihilus.sql +++ b/packages/db/src/migrations/0060_orange_annihilus.sql @@ -31,4 +31,20 @@ END $$;--> statement-breakpoint CREATE INDEX IF NOT EXISTS "issue_reference_mentions_company_source_issue_idx" ON "issue_reference_mentions" USING btree ("company_id","source_issue_id");--> statement-breakpoint CREATE INDEX IF NOT EXISTS "issue_reference_mentions_company_target_issue_idx" ON "issue_reference_mentions" USING btree ("company_id","target_issue_id");--> statement-breakpoint CREATE INDEX IF NOT EXISTS "issue_reference_mentions_company_issue_pair_idx" ON "issue_reference_mentions" USING btree ("company_id","source_issue_id","target_issue_id");--> statement-breakpoint -CREATE UNIQUE INDEX IF NOT EXISTS "issue_reference_mentions_company_source_mention_uq" ON "issue_reference_mentions" USING btree ("company_id","source_issue_id","target_issue_id","source_kind","source_record_id"); +DELETE FROM "issue_reference_mentions" +WHERE "id" IN ( + SELECT "id" + FROM ( + SELECT + "id", + row_number() OVER ( + PARTITION BY "company_id", "source_issue_id", "target_issue_id", "source_kind", "source_record_id" + ORDER BY "created_at", "id" + ) AS "row_number" + FROM "issue_reference_mentions" + ) AS "duplicates" + WHERE "duplicates"."row_number" > 1 +);--> statement-breakpoint +DROP INDEX IF EXISTS "issue_reference_mentions_company_source_mention_uq";--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS "issue_reference_mentions_company_source_mention_record_uq" ON "issue_reference_mentions" USING btree ("company_id","source_issue_id","target_issue_id","source_kind","source_record_id") WHERE "source_record_id" IS NOT NULL;--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS "issue_reference_mentions_company_source_mention_null_record_uq" ON "issue_reference_mentions" USING btree ("company_id","source_issue_id","target_issue_id","source_kind") WHERE "source_record_id" IS NULL; diff --git a/packages/db/src/migrations/meta/0060_snapshot.json b/packages/db/src/migrations/meta/0060_snapshot.json index bb4f32d7f4a..07f676a8852 100644 --- a/packages/db/src/migrations/meta/0060_snapshot.json +++ b/packages/db/src/migrations/meta/0060_snapshot.json @@ -8583,8 +8583,8 @@ "method": "btree", "with": {} }, - "issue_reference_mentions_company_source_mention_uq": { - "name": "issue_reference_mentions_company_source_mention_uq", + "issue_reference_mentions_company_source_mention_record_uq": { + "name": "issue_reference_mentions_company_source_mention_record_uq", "columns": [ { "expression": "company_id", @@ -8620,7 +8620,42 @@ "isUnique": true, "concurrently": false, "method": "btree", - "with": {} + "with": {}, + "where": "\"issue_reference_mentions\".\"source_record_id\" is not null" + }, + "issue_reference_mentions_company_source_mention_null_record_uq": { + "name": "issue_reference_mentions_company_source_mention_null_record_uq", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "source_issue_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "target_issue_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "source_kind", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {}, + "where": "\"issue_reference_mentions\".\"source_record_id\" is null" } }, "foreignKeys": { @@ -13985,4 +14020,4 @@ "schemas": {}, "tables": {} } -} \ No newline at end of file +} diff --git a/packages/db/src/schema/issue_reference_mentions.ts b/packages/db/src/schema/issue_reference_mentions.ts index 263abb703e9..6c44e546734 100644 --- a/packages/db/src/schema/issue_reference_mentions.ts +++ b/packages/db/src/schema/issue_reference_mentions.ts @@ -1,3 +1,4 @@ +import { sql } from "drizzle-orm"; import { index, pgTable, text, timestamp, uniqueIndex, uuid } from "drizzle-orm/pg-core"; import { companies } from "./companies.js"; import { issues } from "./issues.js"; @@ -30,12 +31,18 @@ export const issueReferenceMentions = pgTable( table.sourceIssueId, table.targetIssueId, ), - companySourceMentionUq: uniqueIndex("issue_reference_mentions_company_source_mention_uq").on( + companySourceMentionWithRecordUq: uniqueIndex("issue_reference_mentions_company_source_mention_record_uq").on( table.companyId, table.sourceIssueId, table.targetIssueId, table.sourceKind, table.sourceRecordId, - ), + ).where(sql`${table.sourceRecordId} is not null`), + companySourceMentionWithoutRecordUq: uniqueIndex("issue_reference_mentions_company_source_mention_null_record_uq").on( + table.companyId, + table.sourceIssueId, + table.targetIssueId, + table.sourceKind, + ).where(sql`${table.sourceRecordId} is null`), }), ); diff --git a/packages/shared/src/issue-references.test.ts b/packages/shared/src/issue-references.test.ts index eb1e6d0306f..fa091c412a0 100644 --- a/packages/shared/src/issue-references.test.ts +++ b/packages/shared/src/issue-references.test.ts @@ -39,6 +39,12 @@ describe("issue references", () => { ]); }); + it("trims unmatched square brackets from issue path tokens", () => { + expect(findIssueReferenceMatches("See /issues/PAP-123] for context.")).toEqual([ + { index: 4, length: 15, identifier: "PAP-123", matchedText: "/issues/PAP-123" }, + ]); + }); + it("extracts and dedupes references from markdown", () => { expect(extractIssueReferenceIdentifiers("PAP-1 [again](/issues/pap-1) PAP-2")).toEqual(["PAP-1", "PAP-2"]); }); diff --git a/packages/shared/src/issue-references.ts b/packages/shared/src/issue-references.ts index b8f0a5611b1..eb45ff9a815 100644 --- a/packages/shared/src/issue-references.ts +++ b/packages/shared/src/issue-references.ts @@ -78,7 +78,10 @@ function trimTrailingPunctuation(token: string): string { const last = trimmed[trimmed.length - 1]!; if (!".,!?;:".includes(last) && last !== ")" && last !== "]") break; - if ((last === ")" || last === "]") && (trimmed.match(/\(/g)?.length ?? 0) >= (trimmed.match(/\)/g)?.length ?? 0)) { + if ( + (last === ")" && (trimmed.match(/\(/g)?.length ?? 0) >= (trimmed.match(/\)/g)?.length ?? 0)) + || (last === "]" && (trimmed.match(/\[/g)?.length ?? 0) >= (trimmed.match(/\]/g)?.length ?? 0)) + ) { break; } trimmed = trimmed.slice(0, -1); diff --git a/server/src/services/issue-references.ts b/server/src/services/issue-references.ts index 843f4ebb1e2..505461bef7b 100644 --- a/server/src/services/issue-references.ts +++ b/server/src/services/issue-references.ts @@ -181,26 +181,30 @@ export function issueReferenceService(db: Db) { } async function syncIssue(issueId: string, dbOrTx: any = db) { - const issue = await issueById(issueId, dbOrTx); - if (!issue) throw notFound("Issue not found"); + const runSync = async (tx: any) => { + const issue = await issueById(issueId, tx); + if (!issue) throw notFound("Issue not found"); - await replaceSourceMentions({ - companyId: issue.companyId, - sourceIssueId: issue.id, - sourceKind: "title", - sourceRecordId: null, - documentKey: null, - text: issue.title, - }, dbOrTx); + await replaceSourceMentions({ + companyId: issue.companyId, + sourceIssueId: issue.id, + sourceKind: "title", + sourceRecordId: null, + documentKey: null, + text: issue.title, + }, tx); - await replaceSourceMentions({ - companyId: issue.companyId, - sourceIssueId: issue.id, - sourceKind: "description", - sourceRecordId: null, - documentKey: null, - text: issue.description, - }, dbOrTx); + await replaceSourceMentions({ + companyId: issue.companyId, + sourceIssueId: issue.id, + sourceKind: "description", + sourceRecordId: null, + documentKey: null, + text: issue.description, + }, tx); + }; + + return dbOrTx === db ? db.transaction(runSync) : runSync(dbOrTx); } async function syncComment(commentId: string, dbOrTx: any = db) { diff --git a/ui/src/components/IssueReferencePill.tsx b/ui/src/components/IssueReferencePill.tsx index 665b1ff7576..64f2c04968a 100644 --- a/ui/src/components/IssueReferencePill.tsx +++ b/ui/src/components/IssueReferencePill.tsx @@ -13,24 +13,43 @@ export function IssueReferencePill({ strikethrough?: boolean; className?: string; }) { - const issueLabel = issue.identifier ?? issue.id; + const issueLabel = issue.identifier ?? issue.title; + const classNames = cn( + "paperclip-mention-chip paperclip-mention-chip--issue", + "inline-flex items-center gap-1 rounded-full border border-border px-2 py-0.5 text-xs no-underline", + issue.identifier && "hover:bg-accent/50 focus-visible:outline-none focus-visible:ring-[3px] focus-visible:ring-ring", + strikethrough && "opacity-60 line-through decoration-muted-foreground", + className, + ); + const content = ( + <> + {issue.status ? : null} + {issue.identifier ?? issue.title} + + ); + + if (!issue.identifier) { + return ( + + {content} + + ); + } return ( - {issue.status ? : null} - {issue.identifier ?? issue.title} + {content} ); }