fix(validator): allow issue validator to accept external issues (#127)

* fix(validator/issue): allow external repository to be referenced * docs(validator/issue): add disclaimer for issue access * fix(validator/issue): replace freeform char with alphanum * chore: dummy commit to trigger CI
Namchee · Jan 14, 2025 · a6f50a1 · a6f50a1
1 parent 061fefa
commit a6f50a1
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 45 deletions.
diff --git a/README.md b/README.md
@@ -1,7 +1,7 @@
 # Conventional PR
 
-[![Go Version Badge](https://img.shields.io/github/go-mod/go-version/namchee/conventional-pr)](https://github.com/Namchee/conventional-pr) [![Go Report Card](https://goreportcard.com/badge/github.com/Namchee/conventional-pr)](https://goreportcard.com/report/github.com/Namchee/conventional-pr) [![codecov](https://codecov.io/gh/Namchee/conventional-pr/branch/master/graph/badge.svg)](https://codecov.io/gh/Namchee/conventional-pr) <!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->
-[![All Contributors](https://img.shields.io/badge/all_contributors-8-orange.svg?style=flat-square)](#contributors-)
+[![Go Version Badge](https://img.shields.io/github/go-mod/go-version/namchee/conventional-pr)](https://github.com/Namchee/conventional-pr) [![Go Report Card](https://goreportcard.com/badge/github.com/Namchee/conventional-pr)](https://goreportcard.com/report/github.com/Namchee/conventional-pr) [![codecov](https://codecov.io/gh/Namchee/conventional-pr/branch/master/graph/badge.svg)](https://codecov.io/gh/Namchee/conventional-pr) <!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->
+[![All Contributors](https://img.shields.io/badge/all_contributors-8-orange.svg?style=flat-square)](#contributors-)
 <!-- ALL-CONTRIBUTORS-BADGE:END --> 
 
 Conventional PR is a GitHub Action that validates all pull requests sent to a GitHub-hosted repository.
@@ -213,7 +213,10 @@ Filling the input with an empty string will disabled this validator.
   </tr>
 </table>
 
-This validator checks if the pull request is linked one or more issues.
+This validator checks if the pull request is linked one or more issues in the same or different repository either by [directly linking issue through the sidebar](https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#manually-linking-a-pull-request-or-branch-to-an-issue-using-the-issue-sidebar) or by using [magic closing keywords](https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword).
+
+> [!NOTE]
+> Ensure that the access token you provide can access issues on the target repository
 
 ### Pull request does not introduce too many changes
 
@@ -300,29 +303,29 @@ Do note that `pull_request_target` allows unsafe code to be executed from the he
 ## Contributors ✨
 
 Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/docs/en/emoji-key)):
-<!-- ALL-CONTRIBUTORS-LIST:START - Do not remove or modify this section -->
-<!-- prettier-ignore-start -->
-<!-- markdownlint-disable -->
-<table>
-  <tbody>
-    <tr>
-      <td align="center" valign="top" width="14.28%"><a href="https://github.com/Namchee"><img src="https://avatars.githubusercontent.com/u/32661241?v=4?s=100" width="100px;" alt="Cristopher"/><br /><sub><b>Cristopher</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/commits?author=Namchee" title="Code">💻</a> <a href="https://github.com/Namchee/conventional-pr/issues?q=author%3ANamchee" title="Bug reports">🐛</a> <a href="https://github.com/Namchee/conventional-pr/commits?author=Namchee" title="Documentation">📖</a> <a href="#example-Namchee" title="Examples">💡</a> <a href="#ideas-Namchee" title="Ideas, Planning, & Feedback">🤔</a></td>
-      <td align="center" valign="top" width="14.28%"><a href="https://github.com/smutel"><img src="https://avatars.githubusercontent.com/u/12967891?v=4?s=100" width="100px;" alt="Samuel Mutel"/><br /><sub><b>Samuel Mutel</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/issues?q=author%3Asmutel" title="Bug reports">🐛</a></td>
-      <td align="center" valign="top" width="14.28%"><a href="https://github.com/BinToss"><img src="https://avatars.githubusercontent.com/u/7243190?v=4?s=100" width="100px;" alt="Noah Sherwin"/><br /><sub><b>Noah Sherwin</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/issues?q=author%3ABinToss" title="Bug reports">🐛</a> <a href="#ideas-BinToss" title="Ideas, Planning, & Feedback">🤔</a></td>
-      <td align="center" valign="top" width="14.28%"><a href="https://github.com/avorima"><img src="https://avatars.githubusercontent.com/u/15158349?v=4?s=100" width="100px;" alt="Mario Valderrama"/><br /><sub><b>Mario Valderrama</b></sub></a><br /><a href="#ideas-avorima" title="Ideas, Planning, & Feedback">🤔</a></td>
-      <td align="center" valign="top" width="14.28%"><a href="https://th3s4mur41.me"><img src="https://avatars.githubusercontent.com/u/5543595?v=4?s=100" width="100px;" alt="Michaël Vanderheyden"/><br /><sub><b>Michaël Vanderheyden</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/issues?q=author%3ATh3S4mur41" title="Bug reports">🐛</a></td>
-      <td align="center" valign="top" width="14.28%"><a href="https://github.com/berkon"><img src="https://avatars.githubusercontent.com/u/28842661?v=4?s=100" width="100px;" alt="Bernd Konnerth"/><br /><sub><b>Bernd Konnerth</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/issues?q=author%3Aberkon" title="Bug reports">🐛</a></td>
-      <td align="center" valign="top" width="14.28%"><a href="https://github.com/TheGoderGuy"><img src="https://avatars.githubusercontent.com/u/26481335?v=4?s=100" width="100px;" alt="TheGoderGuy"/><br /><sub><b>TheGoderGuy</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/commits?author=TheGoderGuy" title="Code">💻</a></td>
-    </tr>
-    <tr>
-      <td align="center" valign="top" width="14.28%"><a href="https://github.com/Lapotor"><img src="https://avatars.githubusercontent.com/u/17144397?v=4?s=100" width="100px;" alt="Lapotor"/><br /><sub><b>Lapotor</b></sub></a><br /><a href="#ideas-Lapotor" title="Ideas, Planning, & Feedback">🤔</a></td>
-    </tr>
-  </tbody>
-</table>
-
-<!-- markdownlint-restore -->
-<!-- prettier-ignore-end -->
-
+<!-- ALL-CONTRIBUTORS-LIST:START - Do not remove or modify this section -->
+<!-- prettier-ignore-start -->
+<!-- markdownlint-disable -->
+<table>
+  <tbody>
+    <tr>
+      <td align="center" valign="top" width="14.28%"><a href="https://github.com/Namchee"><img src="https://avatars.githubusercontent.com/u/32661241?v=4?s=100" width="100px;" alt="Cristopher"/><br /><sub><b>Cristopher</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/commits?author=Namchee" title="Code">💻</a> <a href="https://github.com/Namchee/conventional-pr/issues?q=author%3ANamchee" title="Bug reports">🐛</a> <a href="https://github.com/Namchee/conventional-pr/commits?author=Namchee" title="Documentation">📖</a> <a href="#example-Namchee" title="Examples">💡</a> <a href="#ideas-Namchee" title="Ideas, Planning, & Feedback">🤔</a></td>
+      <td align="center" valign="top" width="14.28%"><a href="https://github.com/smutel"><img src="https://avatars.githubusercontent.com/u/12967891?v=4?s=100" width="100px;" alt="Samuel Mutel"/><br /><sub><b>Samuel Mutel</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/issues?q=author%3Asmutel" title="Bug reports">🐛</a></td>
+      <td align="center" valign="top" width="14.28%"><a href="https://github.com/BinToss"><img src="https://avatars.githubusercontent.com/u/7243190?v=4?s=100" width="100px;" alt="Noah Sherwin"/><br /><sub><b>Noah Sherwin</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/issues?q=author%3ABinToss" title="Bug reports">🐛</a> <a href="#ideas-BinToss" title="Ideas, Planning, & Feedback">🤔</a></td>
+      <td align="center" valign="top" width="14.28%"><a href="https://github.com/avorima"><img src="https://avatars.githubusercontent.com/u/15158349?v=4?s=100" width="100px;" alt="Mario Valderrama"/><br /><sub><b>Mario Valderrama</b></sub></a><br /><a href="#ideas-avorima" title="Ideas, Planning, & Feedback">🤔</a></td>
+      <td align="center" valign="top" width="14.28%"><a href="https://th3s4mur41.me"><img src="https://avatars.githubusercontent.com/u/5543595?v=4?s=100" width="100px;" alt="Michaël Vanderheyden"/><br /><sub><b>Michaël Vanderheyden</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/issues?q=author%3ATh3S4mur41" title="Bug reports">🐛</a></td>
+      <td align="center" valign="top" width="14.28%"><a href="https://github.com/berkon"><img src="https://avatars.githubusercontent.com/u/28842661?v=4?s=100" width="100px;" alt="Bernd Konnerth"/><br /><sub><b>Bernd Konnerth</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/issues?q=author%3Aberkon" title="Bug reports">🐛</a></td>
+      <td align="center" valign="top" width="14.28%"><a href="https://github.com/TheGoderGuy"><img src="https://avatars.githubusercontent.com/u/26481335?v=4?s=100" width="100px;" alt="TheGoderGuy"/><br /><sub><b>TheGoderGuy</b></sub></a><br /><a href="https://github.com/Namchee/conventional-pr/commits?author=TheGoderGuy" title="Code">💻</a></td>
+    </tr>
+    <tr>
+      <td align="center" valign="top" width="14.28%"><a href="https://github.com/Lapotor"><img src="https://avatars.githubusercontent.com/u/17144397?v=4?s=100" width="100px;" alt="Lapotor"/><br /><sub><b>Lapotor</b></sub></a><br /><a href="#ideas-Lapotor" title="Ideas, Planning, & Feedback">🤔</a></td>
+    </tr>
+  </tbody>
+</table>
+
+<!-- markdownlint-restore -->
+<!-- prettier-ignore-end -->
+
 <!-- ALL-CONTRIBUTORS-LIST:END -->
 
 This project follows the [all-contributors](https://allcontributors.org) specification.

diff --git a/internal/mocks/client_mock.go b/internal/mocks/client_mock.go
@@ -158,23 +158,27 @@ func (m *githubClientMock) Close(
 
 func (m *githubClientMock) GetIssue(
 	_ context.Context,
-	_ *entity.Meta,
+	meta *entity.Meta,
 	issueNumber int,
 ) (*entity.IssueReference, error) {
-	switch issueNumber {
-	case 2:
+	if meta.Owner == "vitejs" && meta.Name == "vite" {
+		return &entity.IssueReference{
+			Number: issueNumber,
+			Meta: *meta,
+		}, nil
+	}
+
+	if issueNumber == 3 {
 		return &entity.IssueReference{
-			Number: 2,
+			Number: issueNumber,
 			Meta: entity.Meta{
 				Owner: "Namchee",
 				Name:  "conventional-pr",
 			},
 		}, nil
-	case 3:
-		return &entity.IssueReference{}, nil
-	default:
-		return nil, errors.New("mock error")
 	}
+
+	return nil, errors.New("mock error")
 }
 
 func (m *githubClientMock) GetIssueReferences(
@@ -194,6 +198,8 @@ func (m *githubClientMock) GetIssueReferences(
 		}, nil
 	case 2:
 		return []*entity.IssueReference{}, nil
+	case 3:
+		return []*entity.IssueReference{}, nil
 	default:
 		return nil, errors.New("mock error")
 	}

diff --git a/internal/validator/issue.go b/internal/validator/issue.go
@@ -4,14 +4,16 @@ import (
 	"context"
 	"regexp"
 	"strconv"
+	"strings"
 
 	"github.com/Namchee/conventional-pr/internal"
 	"github.com/Namchee/conventional-pr/internal/constants"
 	"github.com/Namchee/conventional-pr/internal/entity"
 )
 
 var (
-	keywordPattern = regexp.MustCompile(`(?mi)\b(close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved) #(\d+)\b`)
+	// TODO: Investigate allowed characters for orgs and repositories
+	keywordPattern = regexp.MustCompile(`(?mi)\b(close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved)\s+([a-zA-Z0-9\-]+/[a-zA-Z0-9\-\._]+)?#(\d+)\b`)
 )
 
 type issueValidator struct {
@@ -87,17 +89,27 @@ func (v *issueValidator) IsValid(
 }
 
 func (v *issueValidator) hasIssueMagicString(
-	ctx context.Context,
+	_ context.Context,
 	pullRequest *entity.PullRequest,
 ) bool {
 	keywords := keywordPattern.FindAllStringSubmatch(pullRequest.Body, -1)
 
-	for _, number := range keywords {
-		num, _ := strconv.Atoi(number[2])
+	for _, keyword := range keywords {
+		org := &pullRequest.Repository
+		num, _ := strconv.Atoi(keyword[3]) 
+
+		if len(keyword[2]) > 0 {
+			tokens := strings.Split(keyword[2], "/")
+
+			org = &entity.Meta{
+				Name: tokens[1],
+				Owner: tokens[0],
+			}
+		}
 
 		issue, _ := v.client.GetIssue(
 			context.Background(),
-			&pullRequest.Repository,
+			org,
 			num,
 		)
 

diff --git a/internal/validator/issue_test.go b/internal/validator/issue_test.go
@@ -71,30 +71,48 @@ func TestIssueValidator_IsValid(t *testing.T) {
 			},
 		},
 		{
-			name: "should reject if reference is not on the same repository",
+			name: "should pass if issue is referenced as magic string in the same repository",
 			args: args{
-				prNumber: 1,
+				prNumber: 2,
 				meta: &entity.Meta{
 					Name:  "conventional-pr",
 					Owner: "namcheee",
 				},
+				body:   "Closes #3",
 				config: true,
 			},
 			want: &entity.ValidationResult{
 				Name:   constants.IssueValidatorName,
 				Active: true,
-				Result: constants.ErrNoIssue,
+				Result: nil,
 			},
 		},
 		{
-			name: "should pass if issue is referenced as magic string",
+			name: "should pass if issue is referenced as magic string in different repository",
 			args: args{
-				prNumber: 2,
+				prNumber: 3,
 				meta: &entity.Meta{
 					Name:  "conventional-pr",
 					Owner: "namcheee",
 				},
-				body:   "Closes #3",
+				body:   "Closes    vitejs/vite#1783",
+				config: true,
+			},
+			want: &entity.ValidationResult{
+				Name:   constants.IssueValidatorName,
+				Active: true,
+				Result: nil,
+			},
+		},
+		{
+			name: "should pass if provided by multiple references",
+			args: args{
+				prNumber: 3,
+				meta: &entity.Meta{
+					Name:  "conventional-pr",
+					Owner: "namcheee",
+				},
+				body:   "Closed #3. Fixes vitejs/vite#1783",
 				config: true,
 			},
 			want: &entity.ValidationResult{
@@ -103,6 +121,23 @@ func TestIssueValidator_IsValid(t *testing.T) {
 				Result: nil,
 			},
 		},
+		{
+			name: "should reject if issue is not accessible",
+			args: args{
+				prNumber: 2,
+				meta: &entity.Meta{
+					Name:  "conventional-pr",
+					Owner: "namcheee",
+				},
+				body:   "Closes #4",
+				config: true,
+			},
+			want: &entity.ValidationResult{
+				Name:   constants.IssueValidatorName,
+				Active: true,
+				Result: constants.ErrNoIssue,
+			},
+		},
 		{
 			name: "should pass if data fetching failed",
 			args: args{