diff --git a/src/main/ipc/jira-ipc-arguments.ts b/src/main/ipc/jira-ipc-arguments.ts new file mode 100644 index 00000000000..e94cfeac6db --- /dev/null +++ b/src/main/ipc/jira-ipc-arguments.ts @@ -0,0 +1,47 @@ +import type { JiraCreateIssueArgs } from '../../shared/jira-types' + +export function normalizeTrimmedArg(value: unknown): string | undefined { + return typeof value === 'string' && value.trim() ? value.trim() : undefined +} + +// IPC args are untrusted, so a malformed key list must read as "nothing declared" +// rather than reaching the resolver and widening what it rewrites. +function normalizeFieldKeyList(value: unknown): string[] | undefined { + if (!Array.isArray(value)) { + return undefined + } + const keys = value.filter( + (key): key is string => typeof key === 'string' && key.trim().length > 0 + ) + return keys.length > 0 ? keys : undefined +} + +export function normalizeJiraCreateIssueArgs( + args: JiraCreateIssueArgs +): { ok: true; args: JiraCreateIssueArgs } | { ok: false; error: string } { + const projectId = normalizeTrimmedArg(args?.projectId) + if (!projectId) { + return { ok: false, error: 'Project is required.' } + } + const issueTypeId = normalizeTrimmedArg(args.issueTypeId) + if (!issueTypeId) { + return { ok: false, error: 'Issue type is required.' } + } + const title = normalizeTrimmedArg(args.title) + if (!title) { + return { ok: false, error: 'Title is required.' } + } + return { + ok: true, + args: { + siteId: normalizeTrimmedArg(args.siteId), + projectId, + issueTypeId, + title, + description: args.description?.trim() || undefined, + customFields: + args.customFields && typeof args.customFields === 'object' ? args.customFields : undefined, + userFieldKeys: normalizeFieldKeyList(args.userFieldKeys) + } + } +} diff --git a/src/main/ipc/jira.ts b/src/main/ipc/jira.ts index b7faa6fd333..a59b4923d39 100644 --- a/src/main/ipc/jira.ts +++ b/src/main/ipc/jira.ts @@ -2,6 +2,7 @@ import { ipcMain } from 'electron' import { connect, disconnect, getStatus, selectSite, testConnection } from '../jira/client' import { _resetPreflightCache } from './preflight' import { JiraCancellableRequests } from './jira-cancellable-requests' +import { normalizeJiraCreateIssueArgs, normalizeTrimmedArg } from './jira-ipc-arguments' import { addIssueComment, createIssue, @@ -16,6 +17,7 @@ import { listPriorities, listProjects, listTransitions, + searchAssignableUsers, searchIssues, updateIssue } from '../jira/issues' @@ -31,12 +33,8 @@ const VALID_FILTERS = new Set(['assigned', 'reported', 'all', ' const issueSummaryRequests = new JiraCancellableRequests() const searchRequests = new JiraCancellableRequests() -function normalizeSiteId(value: unknown): string | undefined { - return typeof value === 'string' && value.trim() ? value.trim() : undefined -} - function normalizeSiteSelection(value: unknown): JiraSiteSelection | undefined { - const siteId = normalizeSiteId(value) + const siteId = normalizeTrimmedArg(value) return siteId as JiraSiteSelection | undefined } @@ -105,7 +103,7 @@ export function registerJiraHandlers(): void { }) ipcMain.handle('jira:disconnect', async (_event, args?: { siteId?: string }) => { - disconnect(normalizeSiteId(args?.siteId)) + disconnect(normalizeTrimmedArg(args?.siteId)) _resetPreflightCache() }) @@ -126,7 +124,7 @@ export function registerJiraHandlers(): void { }) ipcMain.handle('jira:testConnection', async (_event, args?: { siteId?: string }) => { - return testConnection(normalizeSiteId(args?.siteId)) + return testConnection(normalizeTrimmedArg(args?.siteId)) }) ipcMain.handle( @@ -165,7 +163,7 @@ export function registerJiraHandlers(): void { if (typeof args?.key !== 'string' || !args.key.trim()) { return null } - return getIssue(args.key.trim(), normalizeSiteId(args.siteId)) + return getIssue(args.key.trim(), normalizeTrimmedArg(args.siteId)) }) ipcMain.handle( @@ -190,24 +188,8 @@ export function registerJiraHandlers(): void { }) ipcMain.handle('jira:createIssue', async (_event, args: JiraCreateIssueArgs) => { - if (typeof args?.projectId !== 'string' || !args.projectId.trim()) { - return { ok: false, error: 'Project is required.' } - } - if (typeof args?.issueTypeId !== 'string' || !args.issueTypeId.trim()) { - return { ok: false, error: 'Issue type is required.' } - } - if (typeof args?.title !== 'string' || !args.title.trim()) { - return { ok: false, error: 'Title is required.' } - } - return createIssue({ - siteId: normalizeSiteId(args.siteId), - projectId: args.projectId.trim(), - issueTypeId: args.issueTypeId.trim(), - title: args.title.trim(), - description: args.description?.trim() || undefined, - customFields: - args.customFields && typeof args.customFields === 'object' ? args.customFields : undefined - }) + const normalized = normalizeJiraCreateIssueArgs(args) + return normalized.ok ? createIssue(normalized.args) : normalized }) ipcMain.handle( @@ -220,7 +202,7 @@ export function registerJiraHandlers(): void { if (!updates) { return { ok: false, error: 'Updates object is required.' } } - return updateIssue(args.key.trim(), updates, normalizeSiteId(args.siteId)) + return updateIssue(args.key.trim(), updates, normalizeTrimmedArg(args.siteId)) } ) @@ -233,7 +215,7 @@ export function registerJiraHandlers(): void { if (typeof args?.body !== 'string' || !args.body.trim()) { return { ok: false, error: 'Comment body is required.' } } - return addIssueComment(args.key.trim(), args.body.trim(), normalizeSiteId(args.siteId)) + return addIssueComment(args.key.trim(), args.body.trim(), normalizeTrimmedArg(args.siteId)) } ) @@ -241,7 +223,7 @@ export function registerJiraHandlers(): void { if (typeof args?.key !== 'string' || !args.key.trim()) { return [] } - return getIssueComments(args.key.trim(), normalizeSiteId(args.siteId)) + return getIssueComments(args.key.trim(), normalizeTrimmedArg(args.siteId)) }) ipcMain.handle('jira:listProjects', async (_event, args?: { siteId?: JiraSiteSelection }) => { @@ -254,7 +236,7 @@ export function registerJiraHandlers(): void { if (typeof args?.projectIdOrKey !== 'string' || !args.projectIdOrKey.trim()) { return [] } - return listIssueTypes(args.projectIdOrKey.trim(), normalizeSiteId(args.siteId)) + return listIssueTypes(args.projectIdOrKey.trim(), normalizeTrimmedArg(args.siteId)) } ) @@ -270,13 +252,13 @@ export function registerJiraHandlers(): void { return listCreateFields( args.projectIdOrKey.trim(), args.issueTypeId.trim(), - normalizeSiteId(args.siteId) + normalizeTrimmedArg(args.siteId) ) } ) ipcMain.handle('jira:listPriorities', async (_event, args?: { siteId?: string }) => { - return listPriorities(normalizeSiteId(args?.siteId)) + return listPriorities(normalizeTrimmedArg(args?.siteId)) }) ipcMain.handle( @@ -288,7 +270,24 @@ export function registerJiraHandlers(): void { return listAssignableUsers( args.key.trim(), typeof args.query === 'string' ? args.query : undefined, - normalizeSiteId(args.siteId) + normalizeTrimmedArg(args.siteId) + ) + } + ) + + ipcMain.handle( + 'jira:searchUsers', + async ( + _event, + args: { projectIdOrKey?: string; issueKey?: string; query?: string; siteId?: string } + ) => { + return searchAssignableUsers( + { + projectIdOrKey: normalizeTrimmedArg(args?.projectIdOrKey), + issueKey: normalizeTrimmedArg(args?.issueKey) + }, + typeof args?.query === 'string' ? args.query : undefined, + normalizeTrimmedArg(args?.siteId) ) } ) @@ -297,7 +296,7 @@ export function registerJiraHandlers(): void { if (typeof args?.key !== 'string' || !args.key.trim()) { return [] } - return listTransitions(args.key.trim(), normalizeSiteId(args.siteId)) + return listTransitions(args.key.trim(), normalizeTrimmedArg(args.siteId)) }) ipcMain.handle( @@ -306,7 +305,7 @@ export function registerJiraHandlers(): void { if (typeof args?.projectKey !== 'string' || !args.projectKey.trim()) { return { statusIdsByColumn: [] } } - return getProjectStatusOrder(args.projectKey.trim(), normalizeSiteId(args.siteId)) + return getProjectStatusOrder(args.projectKey.trim(), normalizeTrimmedArg(args.siteId)) } ) } diff --git a/src/main/jira/issues.ts b/src/main/jira/issues.ts index 7529c2131da..befcafaa831 100644 --- a/src/main/jira/issues.ts +++ b/src/main/jira/issues.ts @@ -4,10 +4,6 @@ export { getIssue, getIssueSummary } from './jira-issue-read' export { addIssueComment, createIssue, updateIssue } from './jira-issue-mutations' export { getIssueComments } from './jira-issue-comments' export { listProjects } from './jira-project-queries' -export { - listAssignableUsers, - listCreateFields, - listIssueTypes, - listPriorities -} from './jira-issue-create-metadata' +export { listCreateFields, listIssueTypes, listPriorities } from './jira-issue-create-metadata' +export { listAssignableUsers, searchAssignableUsers } from './jira-user-search' export { getProjectStatusOrder, listTransitions } from './jira-transition-queries' diff --git a/src/main/jira/jira-create-reporter-payload.test.ts b/src/main/jira/jira-create-reporter-payload.test.ts new file mode 100644 index 00000000000..c78ea5f5a48 --- /dev/null +++ b/src/main/jira/jira-create-reporter-payload.test.ts @@ -0,0 +1,177 @@ +// STA-2709: creating a Jira issue failed with "Reporter is required." because the +// renderer sent the reporter as text and createIssue forwarded it verbatim. This +// pins the whole seam — picked account id in, `{"reporter":{"id":...}}` out. +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { JiraClientForSite } from './authenticated-request' +import { buildJiraUserFieldValue } from '../../shared/jira-user-field-value' + +const { getClientsMock, isAuthErrorMock, jiraRequestMock } = vi.hoisted(() => ({ + getClientsMock: vi.fn(), + isAuthErrorMock: vi.fn(), + jiraRequestMock: vi.fn() +})) + +vi.mock('./request-queue', () => ({ acquire: vi.fn(), release: vi.fn() })) + +vi.mock('./authenticated-request', () => ({ + apiBasePath: (site: { authType?: string }) => + site.authType === 'server' ? '/rest/api/2' : '/rest/api/3', + jiraRequest: (...args: unknown[]) => jiraRequestMock(...args) +})) + +vi.mock('./client', () => ({ + clearToken: vi.fn(), + getClients: (...args: unknown[]) => getClientsMock(...args), + isAuthError: (...args: unknown[]) => isAuthErrorMock(...args) +})) + +function entry(authType?: 'cloud' | 'server'): JiraClientForSite { + return { + site: { + id: 'site-1', + siteUrl: 'https://example.atlassian.net', + email: 'ada@example.com', + displayName: 'Example Jira', + accountId: 'account-1', + ...(authType ? { authType } : {}) + }, + authorization: 'Basic token' + } +} + +function postedFields(): Record { + const init = jiraRequestMock.mock.calls[0]?.[2] as { body?: string } | undefined + return ( + (JSON.parse(String(init?.body ?? '{}')) as { fields?: Record }).fields ?? {} + ) +} + +async function createWithReporter( + draft: string, + authType?: 'cloud' | 'server' +): Promise> { + getClientsMock.mockReturnValue([entry(authType)]) + jiraRequestMock.mockResolvedValue({ id: '1', key: 'ENG-1', self: 'https://example' }) + const { createIssue } = await import('./jira-issue-mutations') + const result = await createIssue({ + projectId: '100', + issueTypeId: '10001', + title: 'Broken login', + // Exactly the marker the create dialog builds from a picked user; the + // renderer half of that seam is pinned in task-page-jira-create-fields.test. + customFields: { reporter: buildJiraUserFieldValue(draft) }, + // The dialog derives this from Jira's create metadata, which declares + // reporter as schema.type 'user'. + userFieldKeys: ['reporter'] + }) + expect(result.ok).toBe(true) + return postedFields() +} + +describe('Jira create reporter payload', () => { + beforeEach(() => { + vi.clearAllMocks() + isAuthErrorMock.mockReturnValue(false) + jiraRequestMock.mockReset() + }) + + it('sends a picked Cloud account id as {"reporter":{"id":...}}', async () => { + const fields = await createWithReporter('5b10a2844c20165700ede21g') + + expect(fields.reporter).toEqual({ id: '5b10a2844c20165700ede21g' }) + expect(typeof fields.reporter).not.toBe('string') + }) + + it('sends a Server/DC username as {"reporter":{"name":...}}, which has no accountId', async () => { + const fields = await createWithReporter('ada', 'server') + + expect(fields.reporter).toEqual({ name: 'ada' }) + }) + + it('omits the reporter entirely when nothing was picked', async () => { + const fields = await createWithReporter(' ') + + expect('reporter' in fields).toBe(false) + }) + + it('leaves option-shaped custom fields alone', async () => { + getClientsMock.mockReturnValue([entry()]) + jiraRequestMock.mockResolvedValue({ id: '1', key: 'ENG-1', self: 'https://example' }) + const { createIssue } = await import('./jira-issue-mutations') + + await createIssue({ + projectId: '100', + issueTypeId: '10001', + title: 'Broken login', + customFields: { customfield_1: { id: 'opt-1' }, customfield_2: 'free text' } + }) + + const fields = postedFields() + expect(fields.customfield_1).toEqual({ id: 'opt-1' }) + expect(fields.customfield_2).toBe('free text') + }) +}) + +// Thread 1: the {accountId} marker is structural, so without Jira's own verdict on +// which keys are user fields any lookalike object would be rewritten on its way out. +describe('Jira create user-field scoping', () => { + beforeEach(() => { + vi.clearAllMocks() + isAuthErrorMock.mockReturnValue(false) + jiraRequestMock.mockReset() + }) + + async function createWithFields( + customFields: Record, + userFieldKeys?: string[], + authType?: 'cloud' | 'server' + ): Promise> { + getClientsMock.mockReturnValue([entry(authType)]) + jiraRequestMock.mockResolvedValue({ id: '1', key: 'ENG-1', self: 'https://example' }) + const { createIssue } = await import('./jira-issue-mutations') + const result = await createIssue({ + projectId: '100', + issueTypeId: '10001', + title: 'Broken login', + customFields, + userFieldKeys + }) + expect(result.ok).toBe(true) + return postedFields() + } + + it('leaves an accountId-shaped value alone on a field Jira did not declare as a user field', async () => { + const fields = await createWithFields( + { reporter: { accountId: '5abc' }, customfield_1: { accountId: 'not-a-user' } }, + ['reporter'] + ) + + expect(fields.reporter).toEqual({ id: '5abc' }) + expect(fields.customfield_1).toEqual({ accountId: 'not-a-user' }) + }) + + it('leaves an accountId-shaped value alone when no field types were declared at all', async () => { + const fields = await createWithFields({ customfield_1: { accountId: 'not-a-user' } }) + + expect(fields.customfield_1).toEqual({ accountId: 'not-a-user' }) + }) + + it('keeps resolving every entry of a declared array-of-users field', async () => { + const fields = await createWithFields( + { customfield_2: [{ accountId: '5abc' }, { accountId: '5def' }] }, + ['customfield_2'] + ) + + expect(fields.customfield_2).toEqual([{ id: '5abc' }, { id: '5def' }]) + }) + + it('keeps resolving a declared array-of-users field for Server/DC', async () => { + const fields = await createWithFields( + { customfield_2: [{ accountId: 'ada' }, { accountId: 'grace' }] }, + ['customfield_2'], + 'server' + ) + + expect(fields.customfield_2).toEqual([{ name: 'ada' }, { name: 'grace' }]) + }) +}) diff --git a/src/main/jira/jira-issue-create-metadata.ts b/src/main/jira/jira-issue-create-metadata.ts index e3c5338bd49..630a4b9623b 100644 --- a/src/main/jira/jira-issue-create-metadata.ts +++ b/src/main/jira/jira-issue-create-metadata.ts @@ -1,9 +1,4 @@ -import type { - JiraCreateField, - JiraIssueType, - JiraPriority, - JiraUser -} from '../../shared/jira-types' +import type { JiraCreateField, JiraIssueType, JiraPriority } from '../../shared/jira-types' import { acquire, release } from './request-queue' import { apiBasePath, jiraRequest } from './authenticated-request' import { clearToken, getClients, isAuthError } from './client' @@ -11,8 +6,7 @@ import { getCreateFieldRecords, mapCreateField, mapIssueType, - mapPriority, - mapUser + mapPriority } from './jira-issue-mapping' import { asFiniteNumber, @@ -124,37 +118,3 @@ export async function listPriorities(siteId?: string | null): Promise { - const entry = getClients(siteId)[0] - if (!entry) { - return [] - } - const isServer = entry.site.authType === 'server' - const params = new URLSearchParams({ issueKey: key, maxResults: '50' }) - if (query?.trim()) { - // Server/DC filters assignable users by `username`; `query` is Cloud-only. - params.set(isServer ? 'username' : 'query', query.trim()) - } - await acquire() - try { - const response = await jiraRequest( - entry, - `${apiBasePath(entry.site)}/user/assignable/search?${params.toString()}` - ) - return response.map(mapUser).filter((user): user is JiraUser => !!user) - } catch (error) { - if (isAuthError(error)) { - clearToken(entry.site.id) - throw error - } - console.warn('[jira] listAssignableUsers failed:', error) - return [] - } finally { - release() - } -} diff --git a/src/main/jira/jira-issue-mutations.ts b/src/main/jira/jira-issue-mutations.ts index 29b5fd7a679..ad27c061be9 100644 --- a/src/main/jira/jira-issue-mutations.ts +++ b/src/main/jira/jira-issue-mutations.ts @@ -7,6 +7,7 @@ import type { import { acquire, release } from './request-queue' import { apiBasePath, jiraRequest } from './authenticated-request' import { clearToken, getClients, isAuthError } from './client' +import { resolveJiraCreateFieldValue } from '../../shared/jira-user-field-value' import { issueUrl, toBodyText } from './jira-issue-mapping' import type { JiraRecord } from './jira-record-pages' @@ -30,11 +31,19 @@ export async function createIssue(args: JiraCreateIssueArgs): Promise( entry, diff --git a/src/main/jira/jira-user-search.test.ts b/src/main/jira/jira-user-search.test.ts new file mode 100644 index 00000000000..f8429eb4504 --- /dev/null +++ b/src/main/jira/jira-user-search.test.ts @@ -0,0 +1,193 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { JiraClientForSite } from './authenticated-request' + +const { + clearTokenMock, + getClientsMock, + isAuthErrorMock, + jiraRequestMock, + acquireMock, + releaseMock +} = vi.hoisted(() => ({ + clearTokenMock: vi.fn(), + getClientsMock: vi.fn(), + isAuthErrorMock: vi.fn(), + jiraRequestMock: vi.fn(), + acquireMock: vi.fn().mockResolvedValue(undefined), + releaseMock: vi.fn() +})) + +vi.mock('./request-queue', () => ({ acquire: acquireMock, release: releaseMock })) + +vi.mock('./authenticated-request', () => ({ + apiBasePath: (site: { authType?: string }) => + site.authType === 'server' ? '/rest/api/2' : '/rest/api/3', + jiraRequest: (...args: unknown[]) => jiraRequestMock(...args) +})) + +vi.mock('./client', () => ({ + clearToken: (...args: unknown[]) => clearTokenMock(...args), + getClients: (...args: unknown[]) => getClientsMock(...args), + isAuthError: (...args: unknown[]) => isAuthErrorMock(...args) +})) + +function cloudEntry(): JiraClientForSite { + return { + site: { + id: 'site-1', + siteUrl: 'https://example.atlassian.net', + email: 'ada@example.com', + displayName: 'Example Jira', + accountId: 'account-1' + }, + authorization: 'Basic token' + } +} + +function serverEntry(): JiraClientForSite { + return { + site: { + id: 'server-1', + siteUrl: 'https://jira.example.com', + email: '', + displayName: 'Self-hosted Jira', + accountId: 'ada', + authType: 'server' + }, + authorization: 'Bearer pat' + } +} + +function requestedPath(): string { + return String(jiraRequestMock.mock.calls[0]?.[1] ?? '') +} + +describe('searchAssignableUsers', () => { + beforeEach(() => { + vi.clearAllMocks() + isAuthErrorMock.mockReturnValue(false) + getClientsMock.mockReturnValue([cloudEntry()]) + acquireMock.mockResolvedValue(undefined) + jiraRequestMock.mockReset() + }) + + it('scopes a create-time search to the project, since no issue key exists yet', async () => { + jiraRequestMock.mockResolvedValue([ + { accountId: '5abc', displayName: 'Alex Rivera', emailAddress: 'alex@example.com' } + ]) + const { searchAssignableUsers } = await import('./jira-user-search') + + const result = await searchAssignableUsers({ projectIdOrKey: 'ENG' }, 'Alex') + + const path = requestedPath() + expect(path).toContain('/rest/api/3/user/assignable/search?') + expect(path).toContain('project=ENG') + expect(path).toContain('query=Alex') + expect(path).not.toContain('issueKey=') + expect(result).toEqual({ + ok: true, + users: [ + { + accountId: '5abc', + displayName: 'Alex Rivera', + email: 'alex@example.com', + avatarUrl: undefined + } + ] + }) + }) + + it('keeps the issue-scoped search for an existing issue', async () => { + jiraRequestMock.mockResolvedValue([]) + const { searchAssignableUsers } = await import('./jira-user-search') + + await searchAssignableUsers({ issueKey: 'ENG-1' }, 'Alex') + + expect(requestedPath()).toContain('issueKey=ENG-1') + expect(requestedPath()).not.toContain('project=') + }) + + it('filters Server/DC by username, which has no Cloud query parameter', async () => { + getClientsMock.mockReturnValue([serverEntry()]) + jiraRequestMock.mockResolvedValue([]) + const { searchAssignableUsers } = await import('./jira-user-search') + + await searchAssignableUsers({ projectIdOrKey: 'ENG' }, 'ada') + + const path = requestedPath() + expect(path).toContain('/rest/api/2/user/assignable/search?') + expect(path).toContain('username=ada') + expect(path).not.toContain('query=ada') + }) + + it('reports a failed search instead of an empty list', async () => { + jiraRequestMock.mockRejectedValue(new Error('You do not have permission to browse users.')) + const { searchAssignableUsers } = await import('./jira-user-search') + + const result = await searchAssignableUsers({ projectIdOrKey: 'ENG' }, 'Alex') + + expect(result).toEqual({ + ok: false, + error: 'You do not have permission to browse users.' + }) + }) + + it('reports a disconnected site rather than looking like a directory with no users', async () => { + getClientsMock.mockReturnValue([]) + const { searchAssignableUsers } = await import('./jira-user-search') + + expect(await searchAssignableUsers({ projectIdOrKey: 'ENG' })).toEqual({ + ok: false, + error: 'Not connected to Jira.' + }) + expect(jiraRequestMock).not.toHaveBeenCalled() + }) + + it('refuses an unscoped search', async () => { + const { searchAssignableUsers } = await import('./jira-user-search') + + expect(await searchAssignableUsers({ projectIdOrKey: ' ', issueKey: '' }, 'Alex')).toEqual({ + ok: false, + error: 'A Jira project or issue is required to search users.' + }) + expect(jiraRequestMock).not.toHaveBeenCalled() + }) + + it('clears the token and rethrows on an auth failure', async () => { + const authError = new Error('Unauthorized') + isAuthErrorMock.mockReturnValue(true) + jiraRequestMock.mockRejectedValue(authError) + const { searchAssignableUsers } = await import('./jira-user-search') + + await expect(searchAssignableUsers({ projectIdOrKey: 'ENG' })).rejects.toBe(authError) + expect(clearTokenMock).toHaveBeenCalledWith('site-1') + }) +}) + +describe('listAssignableUsers', () => { + beforeEach(() => { + vi.clearAllMocks() + isAuthErrorMock.mockReturnValue(false) + getClientsMock.mockReturnValue([cloudEntry()]) + jiraRequestMock.mockReset() + }) + + // The `jira.listAssignableUsers` RPC is array-shaped on the wire and older + // paired clients call .map on it, so the failure path must stay an empty array. + it('still returns an array when the search fails', async () => { + jiraRequestMock.mockRejectedValue(new Error('boom')) + const { listAssignableUsers } = await import('./jira-user-search') + + await expect(listAssignableUsers('ENG-1', 'Alex')).resolves.toEqual([]) + }) + + it('returns the mapped users on success', async () => { + jiraRequestMock.mockResolvedValue([{ accountId: '5abc', displayName: 'Alex Rivera' }]) + const { listAssignableUsers } = await import('./jira-user-search') + + await expect(listAssignableUsers('ENG-1')).resolves.toEqual([ + { accountId: '5abc', displayName: 'Alex Rivera', email: undefined, avatarUrl: undefined } + ]) + expect(requestedPath()).toContain('issueKey=ENG-1') + }) +}) diff --git a/src/main/jira/jira-user-search.ts b/src/main/jira/jira-user-search.ts new file mode 100644 index 00000000000..f3c1144268b --- /dev/null +++ b/src/main/jira/jira-user-search.ts @@ -0,0 +1,81 @@ +import type { JiraUser, JiraUserSearchResult } from '../../shared/jira-types' +import { acquire, release } from './request-queue' +import { apiBasePath, jiraRequest } from './authenticated-request' +import { clearToken, getClients, isAuthError } from './client' +import { mapUser } from './jira-issue-mapping' +import type { JiraRecord } from './jira-record-pages' + +function buildAssignableSearchParams( + authType: string | undefined, + scope: { issueKey?: string; projectIdOrKey?: string }, + query?: string +): URLSearchParams { + const params = new URLSearchParams({ maxResults: '50' }) + if (scope.issueKey) { + params.set('issueKey', scope.issueKey) + } else if (scope.projectIdOrKey) { + // Create has no issue yet; Cloud and Server/DC both scope by project here. + params.set('project', scope.projectIdOrKey) + } + if (query?.trim()) { + // Server/DC filters assignable users by `username`; `query` is Cloud-only. + params.set(authType === 'server' ? 'username' : 'query', query.trim()) + } + return params +} + +export async function searchAssignableUsers( + scope: { issueKey?: string | null; projectIdOrKey?: string | null }, + query?: string, + siteId?: string | null +): Promise { + const issueKey = scope.issueKey?.trim() || undefined + const projectIdOrKey = scope.projectIdOrKey?.trim() || undefined + if (!issueKey && !projectIdOrKey) { + return { ok: false, error: 'A Jira project or issue is required to search users.' } + } + const entry = getClients(siteId)[0] + if (!entry) { + return { ok: false, error: 'Not connected to Jira.' } + } + const params = buildAssignableSearchParams( + entry.site.authType, + { issueKey, projectIdOrKey }, + query + ) + await acquire() + try { + const response = await jiraRequest( + entry, + `${apiBasePath(entry.site)}/user/assignable/search?${params.toString()}` + ) + return { + ok: true, + users: response.map(mapUser).filter((user): user is JiraUser => !!user) + } + } catch (error) { + if (isAuthError(error)) { + clearToken(entry.site.id) + throw error + } + console.warn('[jira] searchAssignableUsers failed:', error) + return { + ok: false, + error: error instanceof Error ? error.message : 'Failed to search Jira users.' + } + } finally { + release() + } +} + +// Kept array-shaped: `jira.listAssignableUsers` is an established RPC whose +// existing readers call .map on the result, so a paired older client must keep +// receiving an array. New callers should use searchAssignableUsers. +export async function listAssignableUsers( + key: string, + query?: string, + siteId?: string | null +): Promise { + const result = await searchAssignableUsers({ issueKey: key }, query, siteId) + return result.ok ? result.users : [] +} diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index b3bdc579958..7fbaeb37237 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1085,6 +1085,7 @@ import { getIssueComments as getJiraIssueComments, getProjectStatusOrder as getJiraProjectStatusOrder, listAssignableUsers as listJiraAssignableUsers, + searchAssignableUsers as searchJiraAssignableUsers, listCreateFields as listJiraCreateFields, listIssueTypes as listJiraIssueTypes, listIssues as listJiraIssues, @@ -40840,6 +40841,14 @@ export class OrcaRuntimeService { return listJiraAssignableUsers(key, query, siteId) } + jiraSearchUsers( + scope: { issueKey?: string; projectIdOrKey?: string }, + query?: string, + siteId?: string + ): ReturnType { + return searchJiraAssignableUsers(scope, query, siteId) + } + jiraListTransitions(key: string, siteId?: string): ReturnType { return listJiraTransitions(key, siteId) } diff --git a/src/main/runtime/rpc/methods/jira.test.ts b/src/main/runtime/rpc/methods/jira.test.ts index 6667188f88d..055e7e7096d 100644 --- a/src/main/runtime/rpc/methods/jira.test.ts +++ b/src/main/runtime/rpc/methods/jira.test.ts @@ -217,4 +217,43 @@ describe('jira RPC methods', () => { expect(runtime.jiraListTransitions).toHaveBeenCalledWith('ABC-3', 'site-1') expect(runtime.jiraGetProjectStatusOrder).toHaveBeenCalledWith('ALP', 'site-1') }) + + it('accepts a project-scoped user search, which create needs before an issue exists', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + jiraSearchUsers: vi.fn().mockResolvedValue({ ok: true, users: [] }) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: JIRA_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('jira.searchUsers', { + projectIdOrKey: 'ALP', + query: 'Ada', + siteId: 'site-1' + }) + ) + + expect(response.ok).toBe(true) + expect(runtime.jiraSearchUsers).toHaveBeenCalledWith( + { projectIdOrKey: 'ALP', issueKey: undefined }, + 'Ada', + 'site-1' + ) + }) + + it('accepts an issue-scoped user search on the same method', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + jiraSearchUsers: vi.fn().mockResolvedValue({ ok: true, users: [] }) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: JIRA_METHODS }) + + await dispatcher.dispatch(makeRequest('jira.searchUsers', { issueKey: 'ABC-3' })) + + expect(runtime.jiraSearchUsers).toHaveBeenCalledWith( + { projectIdOrKey: undefined, issueKey: 'ABC-3' }, + undefined, + undefined + ) + }) }) diff --git a/src/main/runtime/rpc/methods/jira.ts b/src/main/runtime/rpc/methods/jira.ts index bd264f5c4d6..39b47fb91ac 100644 --- a/src/main/runtime/rpc/methods/jira.ts +++ b/src/main/runtime/rpc/methods/jira.ts @@ -56,7 +56,10 @@ const CreateIssue = z.object({ issueTypeId: requiredString('Issue type is required'), title: requiredString('Title is required'), description: OptionalPlainString, - customFields: z.record(z.string(), z.unknown()).optional() + customFields: z.record(z.string(), z.unknown()).optional(), + // Optional so an older client that never sends it still decodes; the host then + // declares nothing a user field and rewrites nothing. + userFieldKeys: z.array(z.string()).optional() }) const IssueUpdate = z.object({ @@ -94,6 +97,15 @@ const AssignableUsers = z.object({ siteId: OptionalString }) +// Both scopes optional: create has a project and no issue key, the issue view +// has a key and no project. +const UserSearch = z.object({ + projectIdOrKey: OptionalPlainString, + issueKey: OptionalPlainString, + query: OptionalPlainString, + siteId: OptionalString +}) + const ProjectStatusOrder = z.object({ projectKey: requiredString('Project key is required'), siteId: OptionalString @@ -193,7 +205,8 @@ export const JIRA_METHODS: RpcAnyMethod[] = [ issueTypeId: params.issueTypeId.trim(), title: params.title.trim(), description: params.description?.trim() || undefined, - customFields: params.customFields + customFields: params.customFields, + userFieldKeys: params.userFieldKeys }) }), defineMethod({ @@ -253,6 +266,16 @@ export const JIRA_METHODS: RpcAnyMethod[] = [ handler: async (params, { runtime }) => runtime.jiraListAssignableUsers(params.key.trim(), params.query, params.siteId) }), + defineMethod({ + name: 'jira.searchUsers', + params: UserSearch, + handler: async (params, { runtime }) => + runtime.jiraSearchUsers( + { projectIdOrKey: params.projectIdOrKey, issueKey: params.issueKey }, + params.query, + params.siteId + ) + }), defineMethod({ name: 'jira.listTransitions', params: IssueKey, diff --git a/src/preload/api/jira-api.ts b/src/preload/api/jira-api.ts index 1294850c284..c3783e8b28b 100644 --- a/src/preload/api/jira-api.ts +++ b/src/preload/api/jira-api.ts @@ -13,6 +13,8 @@ import type { JiraSiteSelection, JiraTransition, JiraUser, + JiraUserSearchArgs, + JiraUserSearchResult, JiraViewer } from '../../shared/jira-types' @@ -76,6 +78,7 @@ export type JiraApi = { query?: string siteId?: string }) => Promise + searchUsers: (args: JiraUserSearchArgs) => Promise listTransitions: (args: { key: string; siteId?: string }) => Promise getProjectStatusOrder: (args: { projectKey: string diff --git a/src/preload/index.ts b/src/preload/index.ts index aa599962d92..fe11e19f0e0 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -2078,6 +2078,7 @@ const api = { title: string description?: string customFields?: Record + userFieldKeys?: string[] }): Promise< { ok: true; id: string; key: string; url: string } | { ok: false; error: string } > => ipcRenderer.invoke('jira:createIssue', args), @@ -2120,6 +2121,13 @@ const api = { siteId?: string }): Promise => ipcRenderer.invoke('jira:listAssignableUsers', args), + searchUsers: (args: { + projectIdOrKey?: string + issueKey?: string + query?: string + siteId?: string + }): Promise => ipcRenderer.invoke('jira:searchUsers', args), + listTransitions: (args: { key: string; siteId?: string }): Promise => ipcRenderer.invoke('jira:listTransitions', args), getProjectStatusOrder: (args: { diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 0ed764d16e8..8182515e8b9 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -1351,6 +1351,8 @@ export default function TaskPage(): React.JSX.Element { newJiraIssueTargetType, visibleJiraCreateFields, hasMissingJiraCreateField, + missingJiraCreateFieldNames, + searchJiraCreateUsers, handleNewJiraIssueProjectComboboxOpenChange, handleNewJiraIssueProjectSelect, handleNewJiraIssueProjectTriggerKeyDown @@ -3042,7 +3044,9 @@ export default function TaskPage(): React.JSX.Element { newJiraIssueCustomFieldValues, setNewJiraIssueCustomFieldValues, submitShortcutLabel, - hasMissingJiraCreateField + hasMissingJiraCreateField, + missingJiraCreateFieldNames, + searchJiraCreateUsers } const connectDialogs: TaskPageConnectDialogsProps = { gitlabDialogItem, diff --git a/src/renderer/src/components/task-page-jira-create-fields.test.ts b/src/renderer/src/components/task-page-jira-create-fields.test.ts index 11210e347db..d909e85fe72 100644 --- a/src/renderer/src/components/task-page-jira-create-fields.test.ts +++ b/src/renderer/src/components/task-page-jira-create-fields.test.ts @@ -6,8 +6,11 @@ import { findJiraCreateAllowedValue, getJiraCreateAllowedValueLabel, getJiraCreateOptionPayload, + getJiraUserCreateFieldKeys, + isJiraUserCreateField, isVisibleJiraCreateField } from './task-page-jira-create-fields' +import { isJiraUserFieldValue } from '../../../shared/jira-user-field-value' import type { JiraCreateField } from '../../../shared/jira-types' function field(overrides: Partial = {}): JiraCreateField { @@ -171,3 +174,83 @@ describe('buildJiraCreateCustomFields', () => { expect(buildJiraCreateCustomFields([field({ key: 'a' })], { other: 'x' })).toBeUndefined() }) }) + +describe('isJiraUserCreateField', () => { + const cases: [string, JiraCreateField['schema'], boolean][] = [ + ['a single user field', { type: 'user' }, true], + ['a multi-user field', { type: 'array', items: 'user' }, true], + ['a user picker custom field', { type: 'user', custom: 'com.atlassian:userpicker' }, true], + ['an option field', { type: 'option' }, false], + ['a label array', { type: 'array', items: 'string' }, false], + ['an untyped field', undefined, false] + ] + + for (const [label, schema, expected] of cases) { + it(`returns ${expected} for ${label}`, () => { + expect(isJiraUserCreateField(field({ schema }))).toBe(expected) + }) + } +}) + +describe('buildJiraCreateFieldValue for user fields', () => { + // Jira Cloud rejects a bare string reporter with "Reporter is required." — the + // value has to leave the renderer as a user marker for the host to resolve. + it('wraps a picked account id instead of sending it as text', () => { + const reporter = field({ key: 'reporter', name: 'Reporter', schema: { type: 'user' } }) + expect(buildJiraCreateFieldValue(reporter, '5abc')).toEqual({ accountId: '5abc' }) + }) + + it('wraps every entry of a multi-user field', () => { + const participants = field({ + key: 'customfield_100', + schema: { type: 'array', items: 'user' } + }) + expect(buildJiraCreateFieldValue(participants, '5abc, 5def')).toEqual([ + { accountId: '5abc' }, + { accountId: '5def' } + ]) + }) + + it('drops a blank user field so create never sends an empty reporter', () => { + const reporter = field({ key: 'reporter', schema: { type: 'user' } }) + expect(buildJiraCreateFieldValue(reporter, ' ')).toBeUndefined() + expect( + buildJiraCreateFieldValue(field({ schema: { type: 'array', items: 'user' } }), ' , ') + ).toBeUndefined() + }) + + // The host recognizes the marker with this same predicate before rewriting it + // to Jira's per-deployment user shape. + it('produces a value the host recognizes as a user field', () => { + const reporter = field({ key: 'reporter', schema: { type: 'user' } }) + expect(isJiraUserFieldValue(buildJiraCreateFieldValue(reporter, '5abc'))).toBe(true) + expect(isJiraUserFieldValue(buildJiraCreateFieldValue(field({}), 'plain'))).toBe(false) + }) + + it('carries the reporter through the create payload builder', () => { + const reporter = field({ key: 'reporter', schema: { type: 'user' } }) + expect(buildJiraCreateCustomFields([reporter], { reporter: '5abc' })).toEqual({ + reporter: { accountId: '5abc' } + }) + }) +}) + +// The host rewrites the {accountId} marker only for the keys named here, so this +// list is the whole reason a lookalike value on another field survives untouched. +describe('getJiraUserCreateFieldKeys', () => { + it('names only the fields Jira declares as users, single and array alike', () => { + expect( + getJiraUserCreateFieldKeys([ + field({ key: 'reporter', schema: { type: 'user' } }), + field({ key: 'customfield_watchers', schema: { type: 'array', items: 'user' } }), + field({ key: 'customfield_opt', schema: { type: 'option' } }), + field({ key: 'customfield_text', schema: { type: 'string' } }), + field({ key: 'customfield_untyped' }) + ]) + ).toEqual(['reporter', 'customfield_watchers']) + }) + + it('names nothing when the issue type has no user field', () => { + expect(getJiraUserCreateFieldKeys([field({ key: 'customfield_opt' })])).toEqual([]) + }) +}) diff --git a/src/renderer/src/components/task-page-jira-create-fields.ts b/src/renderer/src/components/task-page-jira-create-fields.ts index 932e64c2bb1..74826b9c454 100644 --- a/src/renderer/src/components/task-page-jira-create-fields.ts +++ b/src/renderer/src/components/task-page-jira-create-fields.ts @@ -1,4 +1,5 @@ import { buildJiraCreateTextAdf } from '@/components/jira-create-adf' +import { buildJiraUserFieldValue } from '../../../shared/jira-user-field-value' import type { JiraCreateField } from '../../../shared/jira-types' const JIRA_CREATE_SYSTEM_FIELD_KEYS = new Set(['project', 'issuetype', 'summary', 'description']) @@ -7,6 +8,18 @@ export function isVisibleJiraCreateField(field: JiraCreateField): boolean { return field.required && !JIRA_CREATE_SYSTEM_FIELD_KEYS.has(field.key) } +// Jira returns user fields (reporter, custom user pickers) with no allowedValues, +// so they need a searched picker rather than the option Select or a text box. +export function isJiraUserCreateField(field: JiraCreateField): boolean { + return field.schema?.type === 'user' || field.schema?.items === 'user' +} + +// The host's {accountId} rewrite is shape-based, so it needs Jira's verdict on +// which keys are user fields rather than inferring it from the value. +export function getJiraUserCreateFieldKeys(fields: readonly JiraCreateField[]): string[] { + return fields.filter(isJiraUserCreateField).map((field) => field.key) +} + export function getJiraCreateAllowedValueLabel( value: NonNullable[number] ): string { @@ -40,6 +53,16 @@ export function buildJiraCreateFieldValue(field: JiraCreateField, draftValue: st if (!trimmed) { return undefined } + if (isJiraUserCreateField(field)) { + if (field.schema?.type === 'array') { + const users = trimmed + .split(',') + .map((part) => buildJiraUserFieldValue(part)) + .filter((user) => user !== undefined) + return users.length > 0 ? users : undefined + } + return buildJiraUserFieldValue(trimmed) + } if (field.schema?.type === 'array') { const parts = trimmed .split(',') @@ -65,6 +88,17 @@ export function buildJiraCreateFieldValue(field: JiraCreateField, draftValue: st return trimmed } +// Names the fields still blank so the dialog can say what is missing instead of +// leaving Create disabled with no explanation. +export function getMissingJiraCreateFieldNames( + fields: readonly JiraCreateField[], + values: Record +): string[] { + return fields + .filter((field) => !(values[field.key] ?? '').trim()) + .map((field) => field.name || field.key) +} + export function buildJiraCreateCustomFields( fields: readonly JiraCreateField[], values: Record diff --git a/src/renderer/src/components/task-page/dialogs/jira-user-field-picker.test.tsx b/src/renderer/src/components/task-page/dialogs/jira-user-field-picker.test.tsx new file mode 100644 index 00000000000..c384786a092 --- /dev/null +++ b/src/renderer/src/components/task-page/dialogs/jira-user-field-picker.test.tsx @@ -0,0 +1,170 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { useState } from 'react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { JiraUserFieldPicker } from './jira-user-field-picker' +import type { JiraUserSearchResult } from '../../../../../shared/jira-types' + +afterEach(cleanup) + +const ALEX = { accountId: '5b10a2844c20165700ede21g', displayName: 'Alex Rivera' } +const BLAIR = { accountId: '5c20b3955d31276811fef32h', displayName: 'Blair Chen' } + +function renderPicker( + searchUsers: (query: string) => Promise, + value = '', + onValueChange = vi.fn() +) { + render( + + ) + return { onValueChange } +} + +const SEARCH_PLACEHOLDER = 'Search users or paste an account ID...' + +async function open(): Promise { + fireEvent.click(screen.getByRole('combobox', { name: 'Reporter' })) + await waitFor(() => expect(screen.getByPlaceholderText(SEARCH_PLACEHOLDER)).toBeTruthy()) +} + +describe('JiraUserFieldPicker', () => { + it('searches on open and binds the picked account id', async () => { + const searchUsers = vi.fn(async () => ({ ok: true as const, users: [ALEX] })) + const { onValueChange } = renderPicker(searchUsers) + + await open() + await waitFor(() => expect(screen.getByText('Alex Rivera')).toBeTruthy()) + expect(searchUsers).toHaveBeenCalledWith('') + + fireEvent.click(screen.getByText('Alex Rivera')) + expect(onValueChange).toHaveBeenCalledWith(ALEX.accountId) + }) + + it('says the directory is empty rather than staying blank', async () => { + renderPicker(vi.fn(async () => ({ ok: true as const, users: [] }))) + + await open() + await waitFor(() => expect(screen.getByText('No matching users.')).toBeTruthy()) + }) + + // A swallowed failure that renders as "no results" is what made a broken Jira + // connection look like an empty directory. + it('shows why the search failed instead of an empty list', async () => { + renderPicker( + vi.fn(async () => ({ + ok: false as const, + error: 'You do not have permission to browse users.' + })) + ) + + await open() + await waitFor(() => + expect( + screen.getByText("Couldn't search Jira users: You do not have permission to browse users.") + ).toBeTruthy() + ) + expect(screen.queryByText('No matching users.')).toBeNull() + }) + + it('never parks on a loading state after the search settles', async () => { + renderPicker(vi.fn(async () => ({ ok: false as const, error: 'Network unreachable' }))) + + await open() + await waitFor(() => expect(screen.getByText(/Network unreachable/)).toBeTruthy()) + expect(screen.queryByText('Searching users...')).toBeNull() + }) + + it('accepts a pasted account id the search never returned', async () => { + const searchUsers = vi.fn(async () => ({ ok: true as const, users: [] })) + const { onValueChange } = renderPicker(searchUsers) + + await open() + fireEvent.change(screen.getByPlaceholderText(SEARCH_PLACEHOLDER), { + target: { value: '5b10a2844c20165700ede21g' } + }) + const useTyped = await screen.findByText( + 'Use account ID "5b10a2844c20165700ede21g"', + {}, + { timeout: 3000 } + ) + + fireEvent.click(useTyped) + expect(onValueChange).toHaveBeenCalledWith('5b10a2844c20165700ede21g') + }) + + it('does not offer the raw option when the search already returned that account', async () => { + renderPicker(vi.fn(async () => ({ ok: true as const, users: [ALEX] }))) + + await open() + fireEvent.change(screen.getByPlaceholderText(SEARCH_PLACEHOLDER), { + target: { value: ALEX.accountId } + }) + await waitFor(() => expect(screen.getByText('Alex Rivera')).toBeTruthy()) + expect(screen.queryByText(`Use account ID "${ALEX.accountId}"`)).toBeNull() + }) + + it('labels the trigger with the picked person, not the raw id', async () => { + const onValueChange = vi.fn() + renderPicker( + vi.fn(async () => ({ ok: true as const, users: [ALEX] })), + '', + onValueChange + ) + + await open() + await waitFor(() => expect(screen.getByText('Alex Rivera')).toBeTruthy()) + fireEvent.click(screen.getByText('Alex Rivera')) + + cleanup() + render( + ({ ok: true as const, users: [ALEX] }))} + /> + ) + // Closed and never searched: the raw id is all it can honestly show. + expect(screen.getByRole('combobox', { name: 'Reporter' }).textContent).toContain(ALEX.accountId) + }) + + it('retains multiple picked users for an array user field', async () => { + const searchUsers = vi.fn(async () => ({ ok: true as const, users: [ALEX, BLAIR] })) + + function Harness(): React.JSX.Element { + const [value, setValue] = useState('') + return ( + <> + + {value} + + ) + } + + render() + fireEvent.click(screen.getByRole('combobox', { name: 'Participants' })) + await waitFor(() => expect(screen.getByText(ALEX.displayName)).toBeTruthy()) + fireEvent.click(screen.getByText(ALEX.displayName)) + + expect(screen.getByPlaceholderText(SEARCH_PLACEHOLDER)).toBeTruthy() + await waitFor(() => expect(screen.getByText(BLAIR.displayName)).toBeTruthy()) + fireEvent.click(screen.getByText(BLAIR.displayName)) + + expect(screen.getByLabelText('picked users').textContent).toBe( + `${ALEX.accountId}, ${BLAIR.accountId}` + ) + }) +}) diff --git a/src/renderer/src/components/task-page/dialogs/jira-user-field-picker.tsx b/src/renderer/src/components/task-page/dialogs/jira-user-field-picker.tsx new file mode 100644 index 00000000000..9a8ff200570 --- /dev/null +++ b/src/renderer/src/components/task-page/dialogs/jira-user-field-picker.tsx @@ -0,0 +1,246 @@ +import React, { useCallback, useEffect, useMemo, useState } from 'react' +import { Check, ChevronDown } from 'lucide-react' + +import { Button } from '@/components/ui/button' +import { + Command, + CommandEmpty, + CommandInput, + CommandItem, + CommandList +} from '@/components/ui/command' +import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' +import { translate } from '@/i18n/i18n' +import { cn } from '@/lib/utils' +import type { JiraUser, JiraUserSearchResult } from '../../../../../shared/jira-types' + +const SEARCH_DEBOUNCE_MS = 250 + +type SearchState = + | { status: 'loading' } + | { status: 'ready'; users: JiraUser[] } + | { status: 'error'; error: string } + +export function JiraUserFieldPicker({ + label, + value, + onValueChange, + searchUsers, + disabled, + multiple = false +}: { + label: string + value: string + onValueChange: (value: string) => void + // Must be referentially stable: it re-triggers the search when it changes. + searchUsers: (query: string) => Promise + disabled?: boolean + multiple?: boolean +}): React.JSX.Element { + const [open, setOpen] = useState(false) + const [query, setQuery] = useState('') + const [search, setSearch] = useState({ status: 'ready', users: [] }) + const [pickedNames, setPickedNames] = useState>({}) + + useEffect(() => { + if (!open) { + return + } + let cancelled = false + setSearch({ status: 'loading' }) + const timer = setTimeout( + () => { + void searchUsers(query).then((result) => { + if (cancelled) { + return + } + setSearch( + result.ok + ? { status: 'ready', users: result.users } + : { status: 'error', error: result.error } + ) + }) + }, + query.trim() ? SEARCH_DEBOUNCE_MS : 0 + ) + return () => { + cancelled = true + clearTimeout(timer) + } + }, [open, query, searchUsers]) + + const users = useMemo(() => (search.status === 'ready' ? search.users : []), [search]) + const selectedAccountIds = useMemo( + () => + multiple + ? value + .split(',') + .map((accountId) => accountId.trim()) + .filter(Boolean) + : value.trim() + ? [value.trim()] + : [], + [multiple, value] + ) + const trimmedQuery = query.trim() + // Jira accepts an accountId the search never surfaced (restricted directory, + // a value pasted from Jira itself), so a typed identifier stays selectable. + const showRawValueOption = + trimmedQuery.length > 0 && + search.status !== 'loading' && + !users.some((user) => user.accountId === trimmedQuery) + + const selectedLabel = useMemo(() => { + if (selectedAccountIds.length === 0) { + return '' + } + return selectedAccountIds + .map( + (accountId) => + users.find((user) => user.accountId === accountId)?.displayName ?? + pickedNames[accountId] ?? + accountId + ) + .join(', ') + }, [pickedNames, selectedAccountIds, users]) + + const handleSelect = useCallback( + (accountId: string, displayName?: string) => { + const nextAccountIds = multiple + ? selectedAccountIds.includes(accountId) + ? selectedAccountIds.filter((selected) => selected !== accountId) + : [...selectedAccountIds, accountId] + : [accountId] + onValueChange(nextAccountIds.join(', ')) + if (displayName) { + setPickedNames((prev) => ({ ...prev, [accountId]: displayName })) + } + if (!multiple) { + setOpen(false) + setQuery('') + } + }, + [multiple, onValueChange, selectedAccountIds] + ) + + return ( + { + setOpen(next) + if (!next) { + setQuery('') + } + }} + > + + + + + + + + {search.status === 'loading' ? ( +
+ {translate( + 'auto.components.task.page.dialogs.jira.user.field.picker.searching', + 'Searching users...' + )} +
+ ) : null} + {search.status === 'error' ? ( +
+ {translate( + 'auto.components.task.page.dialogs.jira.user.field.picker.searchFailed', + "Couldn't search Jira users: {{value0}}", + { value0: search.error } + )} +
+ ) : null} + {search.status === 'ready' ? ( + + {translate( + 'auto.components.task.page.dialogs.jira.user.field.picker.noUsers', + 'No matching users.' + )} + + ) : null} + {users.map((user) => ( + handleSelect(user.accountId, user.displayName)} + className="items-center gap-2 px-3 py-2 text-xs" + > + + {user.displayName} + {user.email ? ( + + {user.email} + + ) : null} + + ))} + {showRawValueOption ? ( + handleSelect(trimmedQuery)} + className="items-center gap-2 px-3 py-2 text-xs" + > + + + {translate( + 'auto.components.task.page.dialogs.jira.user.field.picker.useTypedId', + 'Use account ID "{{value0}}"', + { value0: trimmedQuery } + )} + + + ) : null} + + + + + ) +} diff --git a/src/renderer/src/components/task-page/dialogs/new-jira-issue-custom-fields.test.tsx b/src/renderer/src/components/task-page/dialogs/new-jira-issue-custom-fields.test.tsx new file mode 100644 index 00000000000..a09c315aae5 --- /dev/null +++ b/src/renderer/src/components/task-page/dialogs/new-jira-issue-custom-fields.test.tsx @@ -0,0 +1,113 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { useState } from 'react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { NewJiraIssueCustomFields } from './new-jira-issue-custom-fields' +import type { JiraCreateField } from '../../../../../shared/jira-types' + +afterEach(cleanup) + +const REPORTER: JiraCreateField = { + key: 'reporter', + name: 'Reporter', + required: true, + schema: { type: 'user' } +} + +const NOTES: JiraCreateField = { + key: 'customfield_9', + name: 'Notes', + required: true, + schema: { type: 'string' } +} + +const PARTICIPANTS: JiraCreateField = { + key: 'customfield_10', + name: 'Participants', + required: true, + schema: { type: 'array', items: 'user' } +} + +function renderFields( + fields: JiraCreateField[], + searchUsers = vi.fn(async () => ({ ok: true as const, users: [] })) +) { + const setValues = vi.fn() + render( + + ) + return { setValues, searchUsers } +} + +describe('NewJiraIssueCustomFields', () => { + // Jira returns `reporter` with schema.type "user" and no allowedValues, which + // used to fall through to a free-text Input that could never resolve a user. + it('renders a searchable picker for a user field, not a text box', async () => { + const { searchUsers } = renderFields([REPORTER]) + + const trigger = screen.getByRole('combobox', { name: 'Reporter' }) + expect(screen.queryByRole('textbox', { name: 'Reporter' })).toBeNull() + + fireEvent.click(trigger) + await waitFor(() => expect(searchUsers).toHaveBeenCalledWith('')) + }) + + it('leaves non-user fields as their existing text input', () => { + renderFields([NOTES]) + + expect(screen.getByRole('textbox', { name: 'Notes' })).toBeTruthy() + }) + + it('keeps every selection for a multi-user field', async () => { + const alex = { accountId: 'account-1', displayName: 'Alex Rivera' } + const blair = { accountId: 'account-2', displayName: 'Blair Chen' } + const searchUsers = vi.fn(async () => ({ ok: true as const, users: [alex, blair] })) + + function Harness(): React.JSX.Element { + const [values, setValues] = useState>({}) + return ( + <> + + {values[PARTICIPANTS.key]} + + ) + } + + render() + fireEvent.click(screen.getByRole('combobox', { name: 'Participants' })) + await waitFor(() => expect(screen.getByText(alex.displayName)).toBeTruthy()) + fireEvent.click(screen.getByText(alex.displayName)) + expect(screen.getByPlaceholderText('Search users or paste an account ID...')).toBeTruthy() + await waitFor(() => expect(screen.getByText(blair.displayName)).toBeTruthy()) + fireEvent.click(screen.getByText(blair.displayName)) + + expect(screen.getByLabelText('participant ids').textContent).toBe('account-1, account-2') + }) + + it('renders nothing when there are no required fields', () => { + const { container } = render( + ({ ok: true as const, users: [] }))} + /> + ) + + expect(container.firstChild).toBeNull() + }) +}) diff --git a/src/renderer/src/components/task-page/dialogs/new-jira-issue-custom-fields.tsx b/src/renderer/src/components/task-page/dialogs/new-jira-issue-custom-fields.tsx index 5607a460485..24438456245 100644 --- a/src/renderer/src/components/task-page/dialogs/new-jira-issue-custom-fields.tsx +++ b/src/renderer/src/components/task-page/dialogs/new-jira-issue-custom-fields.tsx @@ -8,20 +8,26 @@ import { SelectTrigger, SelectValue } from '@/components/ui/select' -import { getJiraCreateAllowedValueLabel } from '@/components/task-page-jira-create-fields' +import { + getJiraCreateAllowedValueLabel, + isJiraUserCreateField +} from '@/components/task-page-jira-create-fields' +import { JiraUserFieldPicker } from './jira-user-field-picker' import { translate } from '@/i18n/i18n' -import type { JiraCreateField } from '../../../../../shared/jira-types' +import type { JiraCreateField, JiraUserSearchResult } from '../../../../../shared/jira-types' export function NewJiraIssueCustomFields({ visibleJiraCreateFields, newJiraIssueCustomFieldValues, setNewJiraIssueCustomFieldValues, - newJiraIssueSubmitting + newJiraIssueSubmitting, + searchJiraCreateUsers }: { visibleJiraCreateFields: JiraCreateField[] newJiraIssueCustomFieldValues: Record setNewJiraIssueCustomFieldValues: React.Dispatch>> newJiraIssueSubmitting: boolean + searchJiraCreateUsers: (query: string) => Promise }): React.JSX.Element | null { if (visibleJiraCreateFields.length === 0) { return null @@ -33,7 +39,21 @@ export function NewJiraIssueCustomFields({ return (
- {field.allowedValues?.length && field.schema?.type !== 'array' ? ( + {isJiraUserCreateField(field) ? ( + + setNewJiraIssueCustomFieldValues((prev) => ({ + ...prev, + [field.key]: value + })) + } + searchUsers={searchJiraCreateUsers} + disabled={newJiraIssueSubmitting} + multiple={field.schema?.type === 'array'} + /> + ) : field.allowedValues?.length && field.schema?.type !== 'array' ? (