diff --git a/actions/setup/js/update_entity_helpers.cjs b/actions/setup/js/update_entity_helpers.cjs new file mode 100644 index 00000000000..61a353fb49d --- /dev/null +++ b/actions/setup/js/update_entity_helpers.cjs @@ -0,0 +1,54 @@ +// @ts-check +/// + +const { sanitizeTitle } = require("./sanitize_title.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); + +/** + * Build shared update payload fields for issue/PR update handlers. + * + * @param {Object} item + * @param {Object} config + * @param {Object} options + * @param {boolean} [options.allowTitle=true] + * @param {string} [options.defaultOperation] - Required when item.body may be present; used as fallback operation if item.operation and configDefaultOperation are both absent. + * @param {string | undefined} [options.configDefaultOperation] + * @param {boolean} [options.includeBodyInApiData=false] + * @param {(() => void) | undefined} [options.onBodyDisallowed] + * @returns {{updateData: Object, hasCommonUpdates: boolean}} + */ +function buildCommonEntityUpdateData(item, config, options = {}) { + const { allowTitle = true, defaultOperation, configDefaultOperation, includeBodyInApiData = false, onBodyDisallowed } = options; + + const updateData = {}; + let hasCommonUpdates = false; + + if (allowTitle && item.title !== undefined) { + updateData.title = sanitizeTitle(item.title); + hasCommonUpdates = true; + } + + const canUpdateBody = config.allow_body !== false; + if (item.body !== undefined && canUpdateBody) { + const resolvedOperation = item.operation || configDefaultOperation || defaultOperation; + if (!resolvedOperation) { + throw new Error("buildCommonEntityUpdateData: defaultOperation is required when body may be present"); + } + updateData._operation = resolvedOperation; + updateData._rawBody = item.body; + if (includeBodyInApiData) { + updateData.body = item.body; + } + hasCommonUpdates = true; + } else if (item.body !== undefined && !canUpdateBody && typeof onBodyDisallowed === "function") { + onBodyDisallowed(); + } + + // Always populate _includeFooter: downstream executeUpdate reads it regardless of + // whether title/body changed, matching pre-refactor behavior in both callers. + updateData._includeFooter = parseBoolTemplatable(config.footer, true); + + return { updateData, hasCommonUpdates }; +} + +module.exports = { buildCommonEntityUpdateData }; diff --git a/actions/setup/js/update_entity_helpers.test.cjs b/actions/setup/js/update_entity_helpers.test.cjs new file mode 100644 index 00000000000..28fe4538d38 --- /dev/null +++ b/actions/setup/js/update_entity_helpers.test.cjs @@ -0,0 +1,83 @@ +import { describe, it, expect, vi } from "vitest"; +const { buildCommonEntityUpdateData } = require("./update_entity_helpers.cjs"); + +describe("update_entity_helpers.cjs - buildCommonEntityUpdateData", () => { + it("returns hasCommonUpdates true and populates title, body fields, and footer when title and body are provided", () => { + const result = buildCommonEntityUpdateData( + { title: "New title", body: "Body text" }, + {}, + { + defaultOperation: "append", + } + ); + + expect(result.updateData.title).toBe("New title"); + expect(result.updateData._operation).toBe("append"); + expect(result.updateData._rawBody).toBe("Body text"); + expect(result.updateData._includeFooter).toBe(true); + expect(result.hasCommonUpdates).toBe(true); + }); + + it("prefers configDefaultOperation over defaultOperation for body operation", () => { + const result = buildCommonEntityUpdateData( + { body: "Body text" }, + {}, + { + defaultOperation: "append", + configDefaultOperation: "replace", + } + ); + + expect(result.updateData._operation).toBe("replace"); + }); + + it("includes body in api data when includeBodyInApiData is true", () => { + const result = buildCommonEntityUpdateData( + { body: "Body text" }, + {}, + { + defaultOperation: "append", + includeBodyInApiData: true, + } + ); + + expect(result.updateData.body).toBe("Body text"); + }); + + it("item.operation takes precedence over configDefaultOperation and defaultOperation", () => { + const result = buildCommonEntityUpdateData( + { body: "Body text", operation: "prepend" }, + {}, + { + defaultOperation: "append", + configDefaultOperation: "replace", + } + ); + + expect(result.updateData._operation).toBe("prepend"); + }); + + it("skips title when allowTitle is false and does not set hasCommonUpdates", () => { + const result = buildCommonEntityUpdateData({ title: "Should be ignored" }, {}, { allowTitle: false, defaultOperation: "append" }); + + expect(result.updateData.title).toBeUndefined(); + expect(result.hasCommonUpdates).toBe(false); + }); + + it("invokes onBodyDisallowed when body updates are blocked", () => { + const onBodyDisallowed = vi.fn(); + + const result = buildCommonEntityUpdateData( + { body: "Body text" }, + { allow_body: false }, + { + defaultOperation: "append", + onBodyDisallowed, + } + ); + + expect(onBodyDisallowed).toHaveBeenCalledTimes(1); + expect(result.updateData._rawBody).toBeUndefined(); + expect(result.hasCommonUpdates).toBe(false); + }); +}); diff --git a/actions/setup/js/update_issue.cjs b/actions/setup/js/update_issue.cjs index 5abe7f11317..a101b367b2c 100644 --- a/actions/setup/js/update_issue.cjs +++ b/actions/setup/js/update_issue.cjs @@ -11,11 +11,10 @@ const HANDLER_TYPE = "update_issue"; const { resolveTarget, checkRequiredFilter } = require("./safe_output_helpers.cjs"); const { createUpdateHandlerFactory, createStandardResolveNumber, createStandardFormatResult } = require("./update_handler_factory.cjs"); const { updateBody } = require("./update_pr_description_helpers.cjs"); +const { buildCommonEntityUpdateData } = require("./update_entity_helpers.cjs"); const { loadTemporaryProjectMap, replaceTemporaryProjectReferences } = require("./temporary_id.cjs"); -const { sanitizeTitle } = require("./sanitize_title.cjs"); const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const { ERR_VALIDATION } = require("./error_codes.cjs"); -const { parseBoolTemplatable } = require("./templatable.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { generateHistoryUrl } = require("./generate_history_link.cjs"); const { fetchIssueState, mergeIssueState } = require("./safe_output_execution_metadata.cjs"); @@ -131,23 +130,15 @@ const resolveIssueNumber = createStandardResolveNumber({ * @returns {{success: true, data: Object} | {success: false, error: string}} Update data result */ function buildIssueUpdateData(item, config) { - const updateData = {}; + // hasCommonUpdates is not needed here: the issue handler always continues to check + // entity-specific fields (state, labels, assignees, milestone, title prefix). + const { updateData } = buildCommonEntityUpdateData(item, config, { + defaultOperation: "append", + onBodyDisallowed: () => { + core.warning("Body update not allowed by safe-outputs configuration"); + }, + }); - if (item.title !== undefined) { - // Sanitize title for Unicode security - updateData.title = sanitizeTitle(item.title); - } - // Check if body updates are allowed (defaults to true if not specified) - const canUpdateBody = config.allow_body !== false; - if (item.body !== undefined && canUpdateBody) { - // Store operation information for consistent footer/append behavior. - // Default to "append" so we preserve the original issue text. - updateData._operation = item.operation || "append"; - updateData._rawBody = item.body; - } else if (item.body !== undefined && !canUpdateBody) { - // Body update attempted but not allowed by configuration - core.warning("Body update not allowed by safe-outputs configuration"); - } // The safe-outputs schema uses "status" (open/closed), while the GitHub API uses "state". // Accept both for compatibility. if (item.state !== undefined) { @@ -178,9 +169,6 @@ function buildIssueUpdateData(item, config) { return { success: false, error: assigneesLimitResult.error }; } - // Pass footer config to executeUpdate (default to true) - updateData._includeFooter = parseBoolTemplatable(config.footer, true); - // Store title prefix for validation in executeIssueUpdate if (config.title_prefix) { updateData._titlePrefix = config.title_prefix; diff --git a/actions/setup/js/update_pull_request.cjs b/actions/setup/js/update_pull_request.cjs index 7917ef8738f..f23472501b3 100644 --- a/actions/setup/js/update_pull_request.cjs +++ b/actions/setup/js/update_pull_request.cjs @@ -11,8 +11,7 @@ const HANDLER_TYPE = "update_pull_request"; const { updateBody } = require("./update_pr_description_helpers.cjs"); const { resolveTarget, checkRequiredFilter } = require("./safe_output_helpers.cjs"); const { createUpdateHandlerFactory, createStandardResolveNumber, createStandardFormatResult } = require("./update_handler_factory.cjs"); -const { sanitizeTitle } = require("./sanitize_title.cjs"); -const { parseBoolTemplatable } = require("./templatable.cjs"); +const { buildCommonEntityUpdateData } = require("./update_entity_helpers.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { generateHistoryUrl } = require("./generate_history_link.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); @@ -187,26 +186,13 @@ const resolvePRNumber = createStandardResolveNumber({ */ function buildPRUpdateData(item, config) { const canUpdateTitle = config.allow_title !== false; // Default true - const canUpdateBody = config.allow_body !== false; // Default true - - const updateData = {}; - let hasUpdates = false; - - if (canUpdateTitle && item.title !== undefined) { - // Sanitize title for Unicode security (no prefix handling needed for updates) - updateData.title = sanitizeTitle(item.title); - hasUpdates = true; - } - - if (canUpdateBody && item.body !== undefined) { - // Store operation information - // Use operation from item, or fall back to config default, or use "replace" as final default - const operation = item.operation || config.default_operation || "replace"; - updateData._operation = operation; - updateData._rawBody = item.body; - updateData.body = item.body; - hasUpdates = true; - } + const { updateData, hasCommonUpdates } = buildCommonEntityUpdateData(item, config, { + allowTitle: canUpdateTitle, + defaultOperation: "replace", + configDefaultOperation: config.default_operation, + includeBodyInApiData: true, + }); + let hasUpdates = hasCommonUpdates; // Other fields (always allowed) if (item.state !== undefined) { @@ -236,9 +222,6 @@ function buildPRUpdateData(item, config) { }; } - // Pass footer config to executeUpdate (default to true) - updateData._includeFooter = parseBoolTemplatable(config.footer, true); - return { success: true, data: updateData }; }