Skip to content

Commit edc3692

Browse files
committed
fix(create-issue): address cubic-dev-ai review comments on PR #53
- gate the fields flyout badge/store on canApply so a staged-then-cleared value (or an unselectable radio) no longer overcounts - carry the native dialog's assignees/labels into the create payload and resolve them to ids before createIssue, instead of silently dropping them - require the discard-confirmation dialog's text to match /discard/i before clicking its danger button, so an unrelated confirmation is never dismissed - value-gate the omnibar re-clear during the post-create watch window so a freshly-typed draft isn't wiped - add a compile-time key-set assertion between the hand-written ProtocolMap and the schema-derived ProtocolMapFromSchemas so the two can't drift
1 parent 6642088 commit edc3692

5 files changed

Lines changed: 138 additions & 10 deletions

File tree

src/background/create-issue.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { onMessage } from '@/lib/messages'
88
import type { CreateIssueWithFieldsMessageData } from '@/lib/messages'
99
import { gql } from '@/lib/graphql-client'
1010
import { CLONE_ISSUE, ATTACH_TO_PROJECT, UPDATE_PROJECT_FIELD } from '@/lib/graphql-mutations'
11+
import { GET_REPO_ASSIGNEES, GET_REPO_LABELS } from '@/lib/graphql-queries'
1112
import { processQueue, sleep } from '@/lib/queue'
1213
import type { QueueTask } from '@/lib/queue'
1314
import { logger } from '@/lib/debug-logger'
@@ -16,6 +17,37 @@ import { isBulkFull, acquireBulk, releaseBulk } from '@/background/concurrency'
1617
import { broadcastQueue, withRateLimitRetry } from '@/background/rest-helpers'
1718
import { getRepositoryId } from '@/background/project-helpers'
1819

20+
// Resolves the dialog's assignee logins / label names to node ids before
21+
// create. Best-effort: names that don't resolve are dropped rather than
22+
// failing the create.
23+
async function resolveAssigneeIds(
24+
owner: string,
25+
name: string,
26+
logins: string[],
27+
): Promise<string[]> {
28+
if (logins.length === 0) return []
29+
const result = await gql<{
30+
repository: { assignableUsers: { nodes: { id: string; login: string }[] } }
31+
}>(GET_REPO_ASSIGNEES, { owner, name, q: '' })
32+
const byLogin = new Map(
33+
(result.repository?.assignableUsers?.nodes || []).map((u) => [u.login, u.id]),
34+
)
35+
return logins.map((login) => byLogin.get(login)).filter((id): id is string => Boolean(id))
36+
}
37+
38+
async function resolveLabelIds(
39+
owner: string,
40+
name: string,
41+
labelNames: string[],
42+
): Promise<string[]> {
43+
if (labelNames.length === 0) return []
44+
const result = await gql<{
45+
repository: { labels: { nodes: { id: string; name: string }[] } }
46+
}>(GET_REPO_LABELS, { owner, name, q: '' })
47+
const byName = new Map((result.repository?.labels?.nodes || []).map((l) => [l.name, l.id]))
48+
return labelNames.map((n) => byName.get(n)).filter((id): id is string => Boolean(id))
49+
}
50+
1951
function toFieldValue(value: unknown): Record<string, unknown> {
2052
const { singleSelectOptionId, iterationId, text, date, number: num } = value as any
2153
if (singleSelectOptionId) return { singleSelectOptionId }
@@ -51,6 +83,10 @@ async function runCreateIssue(data: CreateIssueWithFieldsMessageData, tabId?: nu
5183
detail: data.title,
5284
run: async () => {
5385
const repositoryId = await getRepositoryId(data.repoOwner, data.repoName)
86+
const [assigneeIds, labelIds] = await Promise.all([
87+
resolveAssigneeIds(data.repoOwner, data.repoName, data.assignees ?? []),
88+
resolveLabelIds(data.repoOwner, data.repoName, data.labels ?? []),
89+
])
5490
logger.log('[rgp:bg] creating issue', { repositoryId, title: data.title })
5591
interface CreateResult {
5692
createIssue: { issue: { id: string; databaseId: number; number: number } }
@@ -61,6 +97,8 @@ async function runCreateIssue(data: CreateIssueWithFieldsMessageData, tabId?: nu
6197
repositoryId,
6298
title: data.title,
6399
body: data.body,
100+
assigneeIds,
101+
labelIds,
64102
})
65103
newIssueId = result.createIssue.issue.id
66104
await sleep(1000)

src/features/create-issue-field-flyout.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { primerCss } from '@/lib/primer-css-helper'
1212
import { ListCheckIcon } from '@/ui/icons'
1313
import { EDITABLE_PROJECT_FIELD_DATATYPES, type ProjectData } from '@/features/bulk-edit-utils'
1414
import { ValuePicker } from '@/features/bulk-edit-value-picker'
15+
import { canApply } from '@/features/bulk-edit-flyout-helpers'
1516
import { createIssueFieldsStore } from '@/lib/create-issue-fields-store'
1617

1718
const chipSx = primerCss.chipButton()
@@ -116,7 +117,11 @@ export function CreateIssueFieldsChip({ getFields }: CreateIssueFieldsChipProps)
116117
key={field.id}
117118
field={field}
118119
value={staged?.value ?? null}
119-
onChange={(next) => createIssueFieldsStore.set(field, next)}
120+
onChange={(next) =>
121+
canApply(next)
122+
? createIssueFieldsStore.set(field, next)
123+
: createIssueFieldsStore.remove(field.id)
124+
}
120125
metaQuery=""
121126
setMetaQuery={noop}
122127
metaResults={[]}

src/features/create-issue-injections.tsx

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ const TITLE_INPUT_SELECTORS = [
4747

4848
const BODY_INPUT_SELECTORS = ['textarea[aria-label="Markdown value"]']
4949

50+
// The dialog's metadata sidebar renders the currently-picked assignees/labels
51+
// as tokens once selected via their own pickers. Best-effort read: if none of
52+
// these match, we simply omit assignees/labels from the create payload rather
53+
// than failing the create.
54+
const ASSIGNEES_CONTAINER_SELECTORS = ['[data-testid="assignees-select-menu"]']
55+
const LABELS_CONTAINER_SELECTORS = ['[data-testid="labels-select-menu"]']
56+
5057
// The board's native "+ Add item" omnibar row that (when typed into) spawns
5158
// this dialog with its text as the initial title. Closing the dialog through
5259
// our intercepted Create button skips GitHub's own handler, which normally
@@ -56,8 +63,9 @@ const OMNIBAR_INPUT_SELECTORS = ['[class*="omnibarInput"]']
5663
// After a "Create more" cycle, GitHub's dialog considers its form dirty and,
5764
// on Close, shows this native confirmation instead of closing directly. RGP's
5865
// own Close click doesn't know about it, so the dialog appears stuck.
59-
const CONFIRM_DISCARD_SELECTOR =
60-
'[data-component="ConfirmationDialog"] button[data-variant="danger"]'
66+
const CONFIRM_DISCARD_DIALOG_SELECTOR = '[data-component="ConfirmationDialog"]'
67+
const CONFIRM_DISCARD_BUTTON_SELECTOR = 'button[data-variant="danger"]'
68+
const CONFIRM_DISCARD_TEXT_PATTERN = /discard/i
6169

6270
// The board re-renders once the new item lands (the background's
6371
// create→attach→field-update chain, several seconds after dispatch), and that
@@ -119,6 +127,30 @@ function readCreateMore(dialog: Element): boolean {
119127
return dialog.querySelector<HTMLInputElement>(CREATE_MORE_SELECTOR)?.checked ?? false
120128
}
121129

130+
function readAssignees(dialog: Element): string[] {
131+
for (const sel of ASSIGNEES_CONTAINER_SELECTORS) {
132+
const container = dialog.querySelector(sel)
133+
if (!container) continue
134+
const logins = Array.from(container.querySelectorAll<HTMLImageElement>('img[alt]'))
135+
.map((img) => img.alt.replace(/^@/, '').trim())
136+
.filter(Boolean)
137+
if (logins.length) return logins
138+
}
139+
return []
140+
}
141+
142+
function readLabels(dialog: Element): string[] {
143+
for (const sel of LABELS_CONTAINER_SELECTORS) {
144+
const container = dialog.querySelector(sel)
145+
if (!container) continue
146+
const names = Array.from(container.querySelectorAll('[data-testid="label-token"]'))
147+
.map((el) => el.textContent?.trim() ?? '')
148+
.filter(Boolean)
149+
if (names.length) return names
150+
}
151+
return []
152+
}
153+
122154
function readRepo(dialog: Element): { owner: string; name: string } | null {
123155
for (const sel of REPO_HEADING_SELECTORS) {
124156
const el = dialog.querySelector(sel)
@@ -164,6 +196,9 @@ function buildCreatePayload(
164196
}
165197
}
166198

199+
const assignees = readAssignees(dialog)
200+
const labels = readLabels(dialog)
201+
167202
return {
168203
projectId,
169204
repoOwner: repo.owner,
@@ -173,6 +208,8 @@ function buildCreatePayload(
173208
createMore: readCreateMore(dialog),
174209
updates,
175210
fieldMeta,
211+
...(assignees.length ? { assignees } : {}),
212+
...(labels.length ? { labels } : {}),
176213
}
177214
}
178215

@@ -186,23 +223,46 @@ export function setupCreateIssueFieldInjector(
186223
let rafId: number | null = null
187224
let intercepting = false
188225
let omnibarWatchUntil: number | null = null
226+
let omnibarSeedText: string | null = null
227+
228+
function readOmnibarValue(): string {
229+
for (const sel of OMNIBAR_INPUT_SELECTORS) {
230+
const el = document.querySelector<HTMLInputElement>(sel)
231+
if (el) return el.value
232+
}
233+
return ''
234+
}
189235

190236
function clearOmnibar(): void {
191237
for (const sel of OMNIBAR_INPUT_SELECTORS) {
192238
document.querySelectorAll<HTMLInputElement>(sel).forEach(clearInput)
193239
}
194240
}
195241

242+
// Same as clearOmnibar, but only touches inputs whose value still matches
243+
// the draft text captured at dispatch time — a value the user has since
244+
// typed over is left alone instead of being wiped by the re-render watch.
245+
function clearOmnibarIfUnchanged(seed: string): void {
246+
for (const sel of OMNIBAR_INPUT_SELECTORS) {
247+
document.querySelectorAll<HTMLInputElement>(sel).forEach((el) => {
248+
if (el.value === seed) clearInput(el)
249+
})
250+
}
251+
}
252+
196253
// GitHub's "Discard changes?" confirmation (if our Close click triggers one)
197254
// mounts asynchronously — it isn't in the DOM yet on the same tick as the
198255
// click. Poll a few animation frames instead of checking once.
199256
async function dismissDiscardConfirm(timeoutMs = 5000): Promise<void> {
200257
const deadline = Date.now() + timeoutMs
201258
while (Date.now() < deadline) {
202-
const btn = document.querySelector<HTMLButtonElement>(CONFIRM_DISCARD_SELECTOR)
203-
if (btn) {
204-
btn.click()
205-
return
259+
const dialog = document.querySelector<HTMLElement>(CONFIRM_DISCARD_DIALOG_SELECTOR)
260+
if (dialog && CONFIRM_DISCARD_TEXT_PATTERN.test(dialog.textContent ?? '')) {
261+
const btn = dialog.querySelector<HTMLButtonElement>(CONFIRM_DISCARD_BUTTON_SELECTOR)
262+
if (btn) {
263+
btn.click()
264+
return
265+
}
206266
}
207267
await new Promise((resolve) => requestAnimationFrame(resolve))
208268
}
@@ -241,13 +301,15 @@ export function setupCreateIssueFieldInjector(
241301

242302
if (!payload.createMore) {
243303
createIssueFieldsStore.clearAll()
304+
omnibarSeedText = readOmnibarValue()
244305
dialog.querySelector<HTMLButtonElement>('[data-component="Dialog.CloseButton"]')?.click()
245306
// Fallback in case some other field still marks the form dirty.
246307
await dismissDiscardConfirm()
247308
clearOmnibar()
248309
// The board re-renders once the new item lands (async, seconds later),
249310
// and that re-render restores the omnibar's original draft text once.
250-
// check() re-clears it on every subsequent mutation until this expires.
311+
// check() re-clears it on every subsequent mutation until this expires,
312+
// but only if the user hasn't since typed a new draft into it.
251313
omnibarWatchUntil = Date.now() + OMNIBAR_WATCH_MS
252314
}
253315
} catch {
@@ -309,8 +371,9 @@ export function setupCreateIssueFieldInjector(
309371
if (omnibarWatchUntil !== null) {
310372
if (Date.now() > omnibarWatchUntil) {
311373
omnibarWatchUntil = null
312-
} else {
313-
clearOmnibar()
374+
omnibarSeedText = null
375+
} else if (omnibarSeedText !== null) {
376+
clearOmnibarIfUnchanged(omnibarSeedText)
314377
}
315378
}
316379

src/lib/messages.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { defineExtensionMessaging } from '@webext-core/messaging'
22
import type { ExcludeCondition, SprintSettings } from '@/lib/storage'
33
import type { PatErrorType } from '@/lib/errors'
4+
import type { ProtocolMapFromSchemas } from '@/lib/schemas-messages'
45

56
export interface IssueRelationshipData {
67
nodeId?: string
@@ -110,6 +111,8 @@ export interface CreateIssueWithFieldsMessageData {
110111
createMore: boolean
111112
updates: { fieldId: string; value: unknown }[]
112113
fieldMeta?: BulkUpdateMessageData['fieldMeta']
114+
assignees?: string[]
115+
labels?: string[]
113116
}
114117

115118
export interface ItemPreviewData {
@@ -416,6 +419,23 @@ interface ProtocolMap {
416419
}): void
417420
}
418421

422+
// Compile-time drift guard: `ProtocolMap` above is hand-written (kept, rather
423+
// than replaced by `ProtocolMapFromSchemas`, because the schema record's
424+
// `Schema.Array` fields are `readonly T[]` and swapping the messaging generic
425+
// cascades ~40 readonly/mutable mismatches into unrelated call sites). This
426+
// fails typecheck if the two maps' key sets diverge, so schemas-messages.ts
427+
// stays the source of truth for which messages exist without forcing every
428+
// caller onto `readonly` arrays.
429+
type _KeysEqual<A, B> = [keyof A] extends [keyof B]
430+
? [keyof B] extends [keyof A]
431+
? true
432+
: false
433+
: false
434+
type _AssertProtocolMapMatchesSchema =
435+
_KeysEqual<ProtocolMap, ProtocolMapFromSchemas> extends true ? true : never
436+
437+
const _protocolMapMatchesSchema: _AssertProtocolMapMatchesSchema = true
438+
419439
const _messaging = defineExtensionMessaging<ProtocolMap>()
420440
export const onMessage = _messaging.onMessage
421441

src/lib/schemas-messages.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,8 @@ export const Messages = {
364364
createMore: Schema.Boolean,
365365
updates: Schema.Array(BulkUpdateFieldUpdate),
366366
fieldMeta: Schema.optional(Schema.Record({ key: Schema.String, value: FieldMetaValue })),
367+
assignees: Schema.optional(Schema.Array(Schema.String)),
368+
labels: Schema.optional(Schema.Array(Schema.String)),
367369
}),
368370
output: BulkUpdateDispatchResult,
369371
},

0 commit comments

Comments
 (0)