allstar icon indicating copy to clipboard operation
allstar copied to clipboard

Refactor: Use context to pass action URLs instead of redundant pagination searches

Open maigl opened this issue 1 month ago • 0 comments

Problem

We're tracking scanned repositories and their results in a database, but the implementation has issues:

  1. Multiple pagination searches - After creating/updating issues or PRs, we search through all issues/PRs again to find the URL
  2. Test failures - Tests crash with nil pointer errors
  3. Verbose error handling - Database save functions require verbose error handling at every call site

Solution

Use Go's context pattern to communicate action results (issue/PR URLs) from the action functions back to the caller.

Implementation Progress

✅ Completed

  1. Created context helper - pkg/policies/action/actionresult.go

    • WithActionResult(ctx) - wraps context to track results
    • SetActionURL(ctx, url) - stores URL in context
    • GetActionResult(ctx) - retrieves stored URL
  2. Updated issue package - pkg/issue/issue.go - calls action.SetActionURL() after creating/updating issues

  3. Updated pullrequest package - pkg/pullrequest/pullrequest.go - calls action.SetActionURL() after creating/updating PRs

  4. Updated enforce.go - wraps context, retrieves URL from context, deleted getIssueURL() and getPRURL() functions

  5. Enhanced database logging - both save functions now log errors internally

🔄 In Progress

Simplify database error handling - Remove verbose error handling at 6+ call sites in pkg/enforce/enforce.go

Lines to fix: 162, 222, 388, 419, 468, 505

⏳ Remaining

Update tests - Fix test mocks in pkg/enforce/enforce_test.go

Files Modified

  • pkg/policies/action/actionresult.go - new file
  • pkg/issue/issue.go
  • pkg/pullrequest/pullrequest.go
  • pkg/enforce/enforce.go - partial
  • 🔄 pkg/enforce/enforce.go - error handling cleanup needed
  • pkg/enforce/enforce_test.go - mocks need updating

maigl avatar Nov 26 '25 15:11 maigl