Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions actions/setup/js/update_discussion.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

const { isDiscussionContext, getDiscussionNumber } = require("./update_context_helpers.cjs");
const { createUpdateHandlerFactory } = require("./update_handler_factory.cjs");
const { createUpdateHandlerFactory, createStandardFormatResult } = require("./update_handler_factory.cjs");
const { sanitizeTitle } = require("./sanitize_title.cjs");

/**
Expand Down Expand Up @@ -142,18 +142,13 @@ function buildDiscussionUpdateData(item, config) {

/**
* Format success result for discussion update
* @param {number} discussionNumber - Discussion number
* @param {Object} updatedDiscussion - Updated discussion object
* @returns {Object} Formatted success result
* Uses the standard format helper for consistency across update handlers
*/
function formatDiscussionSuccessResult(discussionNumber, updatedDiscussion) {
return {
success: true,
number: discussionNumber,
url: updatedDiscussion.url,
title: updatedDiscussion.title,
};
}
const formatDiscussionSuccessResult = createStandardFormatResult({
numberField: "number",
urlField: "url",
urlSource: "url",
});

/**
* Main handler factory for update_discussion
Expand Down
62 changes: 61 additions & 1 deletion actions/setup/js/update_handler_factory.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,62 @@ const { resolveTarget } = require("./safe_output_helpers.cjs");
* @property {Object} [additionalConfig] - Additional configuration options specific to the handler
*/

/**
* Creates a standard resolve number function for issue/PR handlers that use resolveTarget helper
* This factory eliminates duplication between update_issue and update_pull_request
*
* @param {Object} config - Configuration for the resolve function
* @param {string} config.itemType - Type of item (e.g., "update_issue", "update_pull_request")
* @param {string} config.itemNumberField - Field name in item object (e.g., "issue_number", "pull_request_number")
* @param {boolean} config.supportsPR - Whether this handler supports PR context
* @param {boolean} config.supportsIssue - Whether this handler supports issue context
* @returns {Function} Resolve number function
*/
function createStandardResolveNumber(config) {
const { itemType, itemNumberField, supportsPR, supportsIssue } = config;

return function resolveNumber(item, updateTarget, context) {
const targetResult = resolveTarget({
targetConfig: updateTarget,
item: { ...item, item_number: item[itemNumberField] },
context: context,
itemType: itemType,
supportsPR: supportsPR,
supportsIssue: supportsIssue,
});

if (!targetResult.success) {
return { success: false, error: targetResult.error };
}

return { success: true, number: targetResult.number };
};
}

/**
* Creates a standard format success result function
* This factory eliminates duplication across all update handlers
*
* @param {Object} fieldMapping - Mapping of result fields
* @param {string} fieldMapping.numberField - Field name for number (e.g., "number", "pull_request_number")
* @param {string} fieldMapping.urlField - Field name for URL (e.g., "url", "pull_request_url")
* @param {string} fieldMapping.urlSource - Source field in updated item (e.g., "html_url", "url")
* @returns {Function} Format success result function
*/
function createStandardFormatResult(fieldMapping) {
const { numberField, urlField, urlSource } = fieldMapping;

return function formatSuccessResult(itemNumber, updatedItem) {
const result = {
success: true,
[numberField]: itemNumber,
[urlField]: updatedItem[urlSource],
title: updatedItem.title,
};
return result;
};
}

/**
* Creates a handler factory function with common update logic
* This factory encapsulates the shared control flow:
Expand Down Expand Up @@ -169,4 +225,8 @@ function createUpdateHandlerFactory(handlerConfig) {
};
}

module.exports = { createUpdateHandlerFactory };
module.exports = {
createUpdateHandlerFactory,
createStandardResolveNumber,
createStandardFormatResult,
};
134 changes: 134 additions & 0 deletions actions/setup/js/update_handler_factory.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -350,4 +350,138 @@ describe("update_handler_factory.cjs", () => {
expect(result4.error).toContain("Max count of 3 reached");
});
});

describe("createStandardResolveNumber", () => {
it("should create a resolve function that uses resolveTarget helper", async () => {
const resolveNumber = factoryModule.createStandardResolveNumber({
itemType: "update_issue",
itemNumberField: "issue_number",
supportsPR: false,
supportsIssue: true,
});

const item = { issue_number: 42 };
const updateTarget = "triggering";
const context = mockContext;

const result = resolveNumber(item, updateTarget, context);

expect(result.success).toBe(true);
expect(result.number).toBe(42);
});

it("should handle different item number fields", async () => {
const resolveNumber = factoryModule.createStandardResolveNumber({
itemType: "update_pull_request",
itemNumberField: "pull_request_number",
supportsPR: false,
supportsIssue: false,
});

const item = { pull_request_number: 123 };
const updateTarget = "triggering";
// Create a context with PR payload instead of issue
const prContext = {
...mockContext,
eventName: "pull_request",
payload: {
pull_request: {
number: 123,
},
},
};

const result = resolveNumber(item, updateTarget, prContext);

expect(result.success).toBe(true);
expect(result.number).toBe(123);
});

it("should return error when resolveTarget fails", async () => {
const resolveNumber = factoryModule.createStandardResolveNumber({
itemType: "update_issue",
itemNumberField: "issue_number",
supportsPR: false,
supportsIssue: true,
});

// No issue number in item or context
const item = {};
const updateTarget = "triggering";
const context = { ...mockContext, payload: {} };

const result = resolveNumber(item, updateTarget, context);

expect(result.success).toBe(false);
expect(result.error).toBeDefined();
});
});

describe("createStandardFormatResult", () => {
it("should format result with standard fields (issue pattern)", async () => {
const formatResult = factoryModule.createStandardFormatResult({
numberField: "number",
urlField: "url",
urlSource: "html_url",
});

const updatedItem = {
html_url: "https://github.com/owner/repo/issues/42",
title: "Test Issue",
};

const result = formatResult(42, updatedItem);

expect(result).toEqual({
success: true,
number: 42,
url: "https://github.com/owner/repo/issues/42",
title: "Test Issue",
});
});

it("should format result with PR-specific fields", async () => {
const formatResult = factoryModule.createStandardFormatResult({
numberField: "pull_request_number",
urlField: "pull_request_url",
urlSource: "html_url",
});

const updatedItem = {
html_url: "https://github.com/owner/repo/pull/123",
title: "Test PR",
};

const result = formatResult(123, updatedItem);

expect(result).toEqual({
success: true,
pull_request_number: 123,
pull_request_url: "https://github.com/owner/repo/pull/123",
title: "Test PR",
});
});

it("should format result with discussion-specific fields", async () => {
const formatResult = factoryModule.createStandardFormatResult({
numberField: "number",
urlField: "url",
urlSource: "url",
});

const updatedItem = {
url: "https://github.com/owner/repo/discussions/456",
title: "Test Discussion",
};

const result = formatResult(456, updatedItem);

expect(result).toEqual({
success: true,
number: 456,
url: "https://github.com/owner/repo/discussions/456",
title: "Test Discussion",
});
});
});
});
46 changes: 14 additions & 32 deletions actions/setup/js/update_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
const HANDLER_TYPE = "update_issue";

const { resolveTarget } = require("./safe_output_helpers.cjs");
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolveTarget import is no longer used in this file since the createStandardResolveNumber factory now handles the target resolution internally. This unused import should be removed.

Suggested change
const { resolveTarget } = require("./safe_output_helpers.cjs");

Copilot uses AI. Check for mistakes.
const { createUpdateHandlerFactory } = require("./update_handler_factory.cjs");
const { createUpdateHandlerFactory, createStandardResolveNumber, createStandardFormatResult } = require("./update_handler_factory.cjs");
const { updateBody } = require("./update_pr_description_helpers.cjs");
const { loadTemporaryProjectMap, replaceTemporaryProjectReferences } = require("./temporary_id.cjs");
const { sanitizeTitle } = require("./sanitize_title.cjs");
Expand Down Expand Up @@ -92,27 +92,14 @@ async function executeIssueUpdate(github, context, issueNumber, updateData) {

/**
* Resolve issue number from message and configuration
* @param {Object} item - The message item
* @param {string} updateTarget - Target configuration
* @param {Object} context - GitHub Actions context
* @returns {{success: true, number: number} | {success: false, error: string}} Resolution result
* Uses the standard resolve helper for consistency with update_pull_request
*/
function resolveIssueNumber(item, updateTarget, context) {
const targetResult = resolveTarget({
targetConfig: updateTarget,
item: { ...item, item_number: item.issue_number },
context: context,
itemType: "update_issue",
supportsPR: false, // Not used when supportsIssue is true
supportsIssue: true, // update_issue only supports issues, not PRs
});

if (!targetResult.success) {
return { success: false, error: targetResult.error };
}

return { success: true, number: targetResult.number };
}
const resolveIssueNumber = createStandardResolveNumber({
itemType: "update_issue",
itemNumberField: "issue_number",
supportsPR: false, // Not used when supportsIssue is true
supportsIssue: true, // update_issue only supports issues, not PRs
});

/**
* Build update data from message
Expand Down Expand Up @@ -176,18 +163,13 @@ function buildIssueUpdateData(item, config) {

/**
* Format success result for issue update
* @param {number} issueNumber - Issue number
* @param {Object} updatedIssue - Updated issue object
* @returns {Object} Formatted success result
* Uses the standard format helper for consistency across update handlers
*/
function formatIssueSuccessResult(issueNumber, updatedIssue) {
return {
success: true,
number: issueNumber,
url: updatedIssue.html_url,
title: updatedIssue.title,
};
}
const formatIssueSuccessResult = createStandardFormatResult({
numberField: "number",
urlField: "url",
urlSource: "html_url",
});

/**
* Main handler factory for update_issue
Expand Down
46 changes: 14 additions & 32 deletions actions/setup/js/update_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const HANDLER_TYPE = "update_pull_request";

const { updateBody } = require("./update_pr_description_helpers.cjs");
const { resolveTarget } = require("./safe_output_helpers.cjs");
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolveTarget import is no longer used in this file since the createStandardResolveNumber factory now handles the target resolution internally. This unused import should be removed.

Suggested change
const { resolveTarget } = require("./safe_output_helpers.cjs");

Copilot uses AI. Check for mistakes.
const { createUpdateHandlerFactory } = require("./update_handler_factory.cjs");
const { createUpdateHandlerFactory, createStandardResolveNumber, createStandardFormatResult } = require("./update_handler_factory.cjs");
const { sanitizeTitle } = require("./sanitize_title.cjs");

/**
Expand Down Expand Up @@ -69,27 +69,14 @@ async function executePRUpdate(github, context, prNumber, updateData) {

/**
* Resolve PR number from message and configuration
* @param {Object} item - The message item
* @param {string} updateTarget - Target configuration
* @param {Object} context - GitHub Actions context
* @returns {{success: true, number: number} | {success: false, error: string}} Resolution result
* Uses the standard resolve helper for consistency with update_issue
*/
function resolvePRNumber(item, updateTarget, context) {
const targetResult = resolveTarget({
targetConfig: updateTarget,
item: { ...item, item_number: item.pull_request_number },
context: context,
itemType: "update_pull_request",
supportsPR: false, // update_pull_request only supports PRs, not issues
supportsIssue: false,
});

if (!targetResult.success) {
return { success: false, error: targetResult.error };
}

return { success: true, number: targetResult.number };
}
const resolvePRNumber = createStandardResolveNumber({
itemType: "update_pull_request",
itemNumberField: "pull_request_number",
supportsPR: false, // update_pull_request only supports PRs, not issues
supportsIssue: false,
});

/**
* Build update data from message
Expand Down Expand Up @@ -147,18 +134,13 @@ function buildPRUpdateData(item, config) {

/**
* Format success result for PR update
* @param {number} prNumber - PR number
* @param {Object} updatedPR - Updated PR object
* @returns {Object} Formatted success result
* Uses the standard format helper for consistency across update handlers
*/
function formatPRSuccessResult(prNumber, updatedPR) {
return {
success: true,
pull_request_number: prNumber,
pull_request_url: updatedPR.html_url,
title: updatedPR.title,
};
}
const formatPRSuccessResult = createStandardFormatResult({
numberField: "pull_request_number",
urlField: "pull_request_url",
urlSource: "html_url",
});

/**
* Main handler factory for update_pull_request
Expand Down
Loading