Extract duplicate logic to safe output helper functions #15237
Conversation
- Add resolveIssueNumber() helper in safe_output_helpers.cjs - Add extractAssignees() helper in safe_output_helpers.cjs - Update assign_to_user.cjs to use new helper functions - Update unassign_from_user.cjs to use new helper functions - Reduces 108 lines of duplicate code to 52 lines of reusable helpers - Net reduction: 56 lines (58 lines removed, 2 lines added for imports) - All tests passing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request extracts duplicate code from assign_to_user.cjs and unassign_from_user.cjs into reusable helper functions in safe_output_helpers.cjs. Both handlers contained identical logic for determining issue numbers from messages or context (23 lines) and extracting assignees supporting both singular and plural forms (8 lines).
Changes:
- Added
resolveIssueNumber()helper to centralize issue number resolution logic with validation - Added
extractAssignees()helper to handle bothassignee(singular) andassignees(plural) field formats - Refactored both
assign_to_user.cjsandunassign_from_user.cjsto use the shared helpers, reducing duplication by 82 lines
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
actions/setup/js/safe_output_helpers.cjs |
Added two new exported helper functions: resolveIssueNumber() for issue number resolution and extractAssignees() for extracting assignee arrays from messages |
actions/setup/js/assign_to_user.cjs |
Replaced 31 lines of inline logic with calls to the new shared helpers, maintaining identical behavior |
actions/setup/js/unassign_from_user.cjs |
Replaced 31 lines of inline logic with calls to the new shared helpers, maintaining identical behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function resolveIssueNumber(message) { | ||
| // Determine issue number | ||
| let issueNumber; | ||
| if (message.issue_number !== undefined) { | ||
| issueNumber = parseInt(String(message.issue_number), 10); | ||
| if (isNaN(issueNumber)) { | ||
| return { | ||
| success: false, | ||
| error: `Invalid issue_number: ${message.issue_number}`, | ||
| }; | ||
| } | ||
| } else { | ||
| // Use context issue if available | ||
| const contextIssue = context.payload?.issue?.number; | ||
| if (!contextIssue) { | ||
| return { | ||
| success: false, | ||
| error: "No issue number available", | ||
| }; | ||
| } | ||
| issueNumber = contextIssue; | ||
| } | ||
|
|
||
| return { | ||
| success: true, | ||
| issueNumber: issueNumber, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Extract assignees from message supporting both singular and plural forms | ||
| * @param {Object} message - Message object that may contain assignee or assignees | ||
| * @returns {string[]} Array of assignee usernames | ||
| */ | ||
| function extractAssignees(message) { | ||
| // Support both singular "assignee" and plural "assignees" for flexibility | ||
| let requestedAssignees = []; | ||
| if (message.assignees && Array.isArray(message.assignees)) { | ||
| requestedAssignees = message.assignees; | ||
| } else if (message.assignee) { | ||
| requestedAssignees = [message.assignee]; | ||
| } | ||
| return requestedAssignees; | ||
| } |
There was a problem hiding this comment.
The newly added helper functions resolveIssueNumber and extractAssignees lack test coverage. The existing test file safe_output_helpers.test.cjs has comprehensive tests for other helper functions like parseAllowedItems, parseMaxCount, resolveTarget, and loadCustomSafeOutputJobTypes. These new functions should have similar test coverage to ensure they handle edge cases correctly (e.g., invalid issue numbers, missing context, various assignee field formats).
Both
assign_to_user.cjsandunassign_from_user.cjscontained identical 23-line blocks for issue number resolution and 8-line blocks for assignee extraction.Changes
New helper functions in
safe_output_helpers.cjs:resolveIssueNumber(message)- Resolves issue number from message field or context payloadextractAssignees(message)- Extracts assignees array, supporting bothassignee(singular) andassignees(plural) formsRefactored handlers:
assign_to_user.cjs- Replaced inline logic with helper calls (41 lines → 13 lines)unassign_from_user.cjs- Replaced inline logic with helper calls (41 lines → 13 lines)Example
Net: 82 lines of duplicate code consolidated into 52 lines of reusable helpers.
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/repos/github/gh-aw/actions/artifacts/5489103278/zip/usr/bin/curl curl -L -H Accept: application/vnd.github+json -H X-GitHub-Api-Version: 2022-11-28 REDACTED -o agent-artifacts.zip(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
This section details on the original issue you should resolve
<issue_title>[code-simplifier] Extract duplicate logic to safe output helper functions</issue_title>
<issue_description>## Code Simplification - 2026-02-12
This PR simplifies recently modified code to improve clarity, consistency, and maintainability while preserving all functionality.
Files Simplified
actions/setup/js/assign_to_user.cjs- Reduced 41 lines by extracting duplicate issue number resolution and assignee extraction logicactions/setup/js/unassign_from_user.cjs- Reduced 67 lines by using shared helper functions for common operationsactions/setup/js/safe_output_helpers.cjs- Added 45 lines of reusable helper functionsImprovements Made
Reduced Code Duplication
assign_to_user.cjsandunassign_from_user.cjscontained identical 23-line blocks for determining issue numbers from messages or contextEnhanced Maintainability
resolveIssueNumber()helperextractAssignees()helperPreserved Functionality
Applied Project Standards
*_helpers.cjs)Changes Based On
Recent changes from:
Testing
node -c)make fmt-cjs)make lint-cjs)Code Metrics
Helper Functions Added
Review Focus
Please verify:
assign_to_userandunassign_from_userhandlers work correctlysafe_output_helpers.cjsAutomated by Code Simplifier Agent - analyzing code from the last 24 hours
To apply the patch locally:
Show patch preview (250 of 250 lines)