diff --git a/src/__tests__/unit/services/FieldValueService.test.ts b/src/__tests__/unit/services/FieldValueService.test.ts index f5f1188..90749a0 100644 --- a/src/__tests__/unit/services/FieldValueService.test.ts +++ b/src/__tests__/unit/services/FieldValueService.test.ts @@ -30,7 +30,7 @@ describe("FieldValueService", () => { it("should set a TEXT field value", async () => { mockGraphql .mockResolvedValueOnce({ - node: { field: { id: fieldId, name: "Description", dataType: "TEXT" } }, + node: { id: fieldId, name: "Description", dataType: "TEXT" }, }) .mockResolvedValueOnce({ updateProjectV2ItemFieldValue: { projectV2Item: { id: itemId } }, @@ -48,7 +48,7 @@ describe("FieldValueService", () => { it("should set a NUMBER field value", async () => { mockGraphql .mockResolvedValueOnce({ - node: { field: { id: fieldId, name: "Points", dataType: "NUMBER" } }, + node: { id: fieldId, name: "Points", dataType: "NUMBER" }, }) .mockResolvedValueOnce({}); @@ -62,7 +62,7 @@ describe("FieldValueService", () => { it("should set a DATE field value", async () => { mockGraphql .mockResolvedValueOnce({ - node: { field: { id: fieldId, name: "Due", dataType: "DATE" } }, + node: { id: fieldId, name: "Due", dataType: "DATE" }, }) .mockResolvedValueOnce({}); @@ -82,15 +82,13 @@ describe("FieldValueService", () => { mockGraphql .mockResolvedValueOnce({ node: { - field: { - id: fieldId, - name: "Status", - dataType: "SINGLE_SELECT", - options: [ - { id: "opt-1", name: "Todo" }, - { id: "opt-2", name: "Done" }, - ], - }, + id: fieldId, + name: "Status", + dataType: "SINGLE_SELECT", + options: [ + { id: "opt-1", name: "Todo" }, + { id: "opt-2", name: "Done" }, + ], }, }) .mockResolvedValueOnce({}); @@ -105,7 +103,7 @@ describe("FieldValueService", () => { it("should set an ITERATION field value", async () => { mockGraphql .mockResolvedValueOnce({ - node: { field: { id: fieldId, name: "Sprint", dataType: "ITERATION" } }, + node: { id: fieldId, name: "Sprint", dataType: "ITERATION" }, }) .mockResolvedValueOnce({}); @@ -121,17 +119,43 @@ describe("FieldValueService", () => { expect(mutationVars.value).toBe("iter-1"); }); + // ProjectV2.field() only takes `name`; querying it by id is rejected by GitHub, + // which made every set_field_value call fail (e.g. moving a board item between columns). + it("should look up the field by its node ID, not through ProjectV2.field(id:)", async () => { + mockGraphql + .mockResolvedValueOnce({ node: { id: fieldId, name: "Description", dataType: "TEXT" } }) + .mockResolvedValueOnce({}); + + await service.setFieldValue({ projectId, itemId, fieldId, value: "hello" }); + + const [lookupQuery, lookupVars] = mockGraphql.mock.calls[0]; + expect(lookupQuery).toContain("node(id: $fieldId)"); + expect(lookupQuery).not.toContain("field(id:"); + expect(lookupVars).toEqual({ fieldId }); + }); + it("should throw when field is not found", async () => { - mockGraphql.mockResolvedValueOnce({ node: { field: null } }); + mockGraphql.mockResolvedValueOnce({ node: null }); await expect( service.setFieldValue({ projectId, itemId, fieldId, value: "x" }) ).rejects.toThrow(DomainError); + expect(mockGraphql).toHaveBeenCalledTimes(1); + }); + + it("should throw when the ID belongs to a node that is not a project field", async () => { + // No fragment matches, so GitHub returns an empty object for the node. + mockGraphql.mockResolvedValueOnce({ node: {} }); + + await expect( + service.setFieldValue({ projectId, itemId, fieldId: "PVTI_not_a_field", value: "x" }) + ).rejects.toThrow(DomainError); + expect(mockGraphql).toHaveBeenCalledTimes(1); }); it("should throw mapped error for unsupported field type", async () => { mockGraphql.mockResolvedValueOnce({ - node: { field: { id: fieldId, name: "Custom", dataType: "UNSUPPORTED" } }, + node: { id: fieldId, name: "Custom", dataType: "UNSUPPORTED" }, }); await expect( diff --git a/src/services/FieldValueService.ts b/src/services/FieldValueService.ts index a64a489..48b2585 100644 --- a/src/services/FieldValueService.ts +++ b/src/services/FieldValueService.ts @@ -102,41 +102,36 @@ export class FieldValueService { value: unknown; }): Promise<{ success: boolean; message: string }> { return safeCall(async () => { + // ProjectV2.field() only accepts `name`, so the field is looked up by its node ID. const fieldQuery = ` - query($projectId: ID!, $fieldId: ID!) { - node(id: $projectId) { - ... on ProjectV2 { - field(id: $fieldId) { - ... on ProjectV2Field { id name dataType } - ... on ProjectV2IterationField { id name dataType } - ... on ProjectV2SingleSelectField { id name dataType options { id name } } - } - } + query($fieldId: ID!) { + node(id: $fieldId) { + ... on ProjectV2Field { id name dataType } + ... on ProjectV2IterationField { id name dataType } + ... on ProjectV2SingleSelectField { id name dataType options { id name } } } } `; interface FieldQueryResponse { node: { - field: { - id: string; - name: string; - dataType: string; - options?: Array<{ id: string; name: string }>; - }; - }; + id?: string; + name: string; + dataType: string; + options?: Array<{ id: string; name: string }>; + } | null; } const fieldResponse = await this.factory.graphql(fieldQuery, { - projectId: data.projectId, fieldId: data.fieldId, }); - if (!fieldResponse.node?.field) { + // A node ID that is not a project field matches no fragment and comes back as {}. + if (!fieldResponse.node?.id) { throw new ResourceNotFoundError(ResourceType.FIELD, data.fieldId); } - const field = fieldResponse.node.field; + const field = fieldResponse.node; const strategy = makeStrategy(field.dataType, field.options); const mutation = strategy.buildMutation(); const variables = strategy.buildVariables(