Add subject number and repository URL to notifications

gitea/notifications.go

@@ -2,6 +2,7 @@ package gitea
 
 import (
 	"context"
+	"net/url"
 	"strconv"
 	"strings"
 
@@ -32,6 +33,27 @@ func convertGiteaSubjectType(t string) forge.NotificationSubjectType {
 	}
 }
 
+// giteaSubjectNumber returns the issue or pull request number at the end of a
+// subject URL such as .../issues/12 or .../pulls/12, or 0 if there is none.
+func giteaSubjectNumber(subjectURL string) int {
+	u, err := url.Parse(subjectURL)
+	if err != nil {
+		return 0
+	}
+	parts := strings.Split(strings.Trim(u.Path, "/"), "/")
+	if len(parts) < 2 {
+		return 0
+	}
+	if kind := parts[len(parts)-2]; kind != "issues" && kind != "pulls" {
+		return 0
+	}
+	number, err := strconv.Atoi(parts[len(parts)-1])
+	if err != nil || number <= 0 {
+		return 0
+	}
+	return number
+}
+
 func convertGiteaNotification(n *gitea.NotificationThread) forge.Notification {
 	result := forge.Notification{
 		ID:     strconv.FormatInt(n.ID, 10),
@@ -42,10 +64,18 @@ func convertGiteaNotification(n *gitea.NotificationThread) forge.Notification {
 		result.Title = n.Subject.Title
 		result.SubjectType = convertGiteaSubjectType(string(n.Subject.Type))
 		result.URL = n.Subject.HTMLURL
+		switch result.SubjectType {
+		case forge.NotificationSubjectIssue, forge.NotificationSubjectPullRequest:
+			result.SubjectNumber = giteaSubjectNumber(n.Subject.HTMLURL)
+			if result.SubjectNumber == 0 {
+				result.SubjectNumber = giteaSubjectNumber(n.Subject.URL)
+			}
+		}
 	}
 
 	if n.Repository != nil {
 		result.Repo = n.Repository.FullName
+		result.RepoURL = n.Repository.HTMLURL
 	}
 
 	if !n.UpdatedAt.IsZero() {

gitea/notifications_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http/httptest"
 	"testing"
 
+	"code.gitea.io/sdk/gitea"
 	forge "github.com/git-pkgs/forge"
 )
 
@@ -26,6 +27,7 @@ func TestGiteaListNotifications(t *testing.T) {
 				"repository": map[string]any{
 					"full_name": "testorg/testrepo",
 					"name":      "testrepo",
+					"html_url":  "https://codeberg.org/testorg/testrepo",
 					"owner":     map[string]any{"login": "testorg"},
 				},
 				"updated_at": "2024-06-01T10:00:00Z",
@@ -50,6 +52,36 @@ func TestGiteaListNotifications(t *testing.T) {
 	assertEqual(t, "Title", "Bug fix", notifications[0].Title)
 	assertEqual(t, "SubjectType", "issue", string(notifications[0].SubjectType))
 	assertEqual(t, "Repo", "testorg/testrepo", notifications[0].Repo)
+	assertEqualInt(t, "SubjectNumber", 1, notifications[0].SubjectNumber)
+	assertEqual(t, "URL", "https://codeberg.org/testorg/testrepo/issues/1", notifications[0].URL)
+	assertEqual(t, "RepoURL", "https://codeberg.org/testorg/testrepo", notifications[0].RepoURL)
+}
+
+func TestConvertGiteaNotificationSubject(t *testing.T) {
+	tests := []struct {
+		name       string
+		subject    *gitea.NotificationSubject
+		wantNumber int
+	}{
+		{"pull request", &gitea.NotificationSubject{Type: gitea.NotifySubjectPull,
+			HTMLURL: "https://codeberg.org/o/r/pulls/42"}, 42},
+		{"falls back to API URL", &gitea.NotificationSubject{Type: gitea.NotifySubjectIssue,
+			URL: "https://codeberg.org/api/v1/repos/o/r/issues/7"}, 7},
+		{"commit has no number", &gitea.NotificationSubject{Type: gitea.NotifySubjectCommit,
+			HTMLURL: "https://codeberg.org/o/r/commit/abc123"}, 0},
+		{"repository has no number", &gitea.NotificationSubject{Type: gitea.NotifySubjectRepository,
+			HTMLURL: "https://codeberg.org/o/r"}, 0},
+		{"non-numeric", &gitea.NotificationSubject{Type: gitea.NotifySubjectIssue,
+			HTMLURL: "https://codeberg.org/o/r/issues/new"}, 0},
+		{"no URLs", &gitea.NotificationSubject{Type: gitea.NotifySubjectIssue}, 0},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			n := convertGiteaNotification(&gitea.NotificationThread{Subject: tt.subject})
+			assertEqualInt(t, "SubjectNumber", tt.wantNumber, n.SubjectNumber)
+			assertEqual(t, "URL", tt.subject.HTMLURL, n.URL)
+		})
+	}
 }
 
 func TestGiteaListNotificationsForRepo(t *testing.T) {

github/notifications.go

@@ -3,6 +3,8 @@ package github
 import (
 	"context"
 	"net/http"
+	"net/url"
+	"strconv"
 	"strings"
 
 	forge "github.com/git-pkgs/forge"
@@ -36,6 +38,47 @@ func convertGitHubSubjectType(t string) forge.NotificationSubjectType {
 	}
 }
 
+// gitHubSubjectLink derives a subject's web URL and number from the API URL
+// GitHub reports for it, such as .../repos/octocat/hello/pulls/42, using the
+// repository's web URL as the base so GitHub Enterprise works too. Subjects
+// without a web page that can be built from the API URL, such as releases
+// (addressed by ID rather than tag), return an empty URL.
+func gitHubSubjectLink(repoHTMLURL, apiURL string) (string, int) {
+	if repoHTMLURL == "" || apiURL == "" {
+		return "", 0
+	}
+	u, err := url.Parse(apiURL)
+	if err != nil {
+		return "", 0
+	}
+	parts := strings.Split(strings.Trim(u.Path, "/"), "/")
+	// Expect .../repos/{owner}/{repo}/{kind}/{id}.
+	if len(parts) < 5 || parts[len(parts)-5] != "repos" {
+		return "", 0
+	}
+	kind, id := parts[len(parts)-2], parts[len(parts)-1]
+	base := strings.TrimSuffix(repoHTMLURL, "/")
+
+	var page string
+	switch kind {
+	case "issues":
+		page = "issues"
+	case "pulls":
+		page = "pull"
+	case "discussions":
+		page = "discussions"
+	case "commits":
+		return base + "/commit/" + id, 0
+	default:
+		return "", 0
+	}
+	number, err := strconv.Atoi(id)
+	if err != nil || number <= 0 {
+		return "", 0
+	}
+	return base + "/" + page + "/" + id, number
+}
+
 func convertGitHubNotification(n *github.Notification) forge.Notification {
 	result := forge.Notification{
 		ID:     n.GetID(),
@@ -45,12 +88,13 @@ func convertGitHubNotification(n *github.Notification) forge.Notification {
 
 	if r := n.GetRepository(); r != nil {
 		result.Repo = r.GetFullName()
-		result.URL = r.GetHTMLURL()
+		result.RepoURL = r.GetHTMLURL()
 	}
 
 	if s := n.GetSubject(); s != nil {
 		result.Title = s.GetTitle()
 		result.SubjectType = convertGitHubSubjectType(s.GetType())
+		result.URL, result.SubjectNumber = gitHubSubjectLink(result.RepoURL, s.GetURL())
 	}
 
 	if t := n.GetUpdatedAt(); !t.IsZero() {

github/notifications_test.go

@@ -28,6 +28,7 @@ func TestGitHubListNotifications(t *testing.T) {
 				Subject: &github.NotificationSubject{
 					Title: ptr("Fix bug"),
 					Type:  ptr("Issue"),
+					URL:   ptr("https://api.github.com/repos/octocat/hello-world/issues/123"),
 				},
 				Repository: &github.Repository{
 					FullName: ptr("octocat/hello-world"),
@@ -55,7 +56,41 @@ func TestGitHubListNotifications(t *testing.T) {
 	assertEqual(t, "Reason", "mention", notifications[0].Reason)
 	assertEqual(t, "Title", "Fix bug", notifications[0].Title)
 	assertEqual(t, "SubjectType", "issue", string(notifications[0].SubjectType))
+	assertEqualInt(t, "SubjectNumber", 123, notifications[0].SubjectNumber)
+	assertEqual(t, "URL", "https://github.com/octocat/hello-world/issues/123", notifications[0].URL)
 	assertEqual(t, "Repo", "octocat/hello-world", notifications[0].Repo)
+	assertEqual(t, "RepoURL", "https://github.com/octocat/hello-world", notifications[0].RepoURL)
+}
+
+func TestGitHubSubjectLink(t *testing.T) {
+	const repo = "https://github.com/octocat/hello"
+	tests := []struct {
+		name       string
+		repoURL    string
+		apiURL     string
+		wantURL    string
+		wantNumber int
+	}{
+		{"issue", repo, "https://api.github.com/repos/octocat/hello/issues/12", repo + "/issues/12", 12},
+		{"pull request", repo, "https://api.github.com/repos/octocat/hello/pulls/42", repo + "/pull/42", 42},
+		{"discussion", repo, "https://api.github.com/repos/octocat/hello/discussions/7", repo + "/discussions/7", 7},
+		{"commit", repo, "https://api.github.com/repos/octocat/hello/commits/abc123", repo + "/commit/abc123", 0},
+		{"release by ID", repo, "https://api.github.com/repos/octocat/hello/releases/987654", "", 0},
+		{"enterprise", "https://ghe.example.com/octocat/hello", "https://ghe.example.com/api/v3/repos/octocat/hello/pulls/5",
+			"https://ghe.example.com/octocat/hello/pull/5", 5},
+		{"trailing slash on repo URL", repo + "/", "https://api.github.com/repos/octocat/hello/issues/3", repo + "/issues/3", 3},
+		{"no subject URL", repo, "", "", 0},
+		{"no repository URL", "", "https://api.github.com/repos/octocat/hello/issues/12", "", 0},
+		{"non-numeric number", repo, "https://api.github.com/repos/octocat/hello/issues/abc", "", 0},
+		{"not a repository path", repo, "https://api.github.com/notifications/threads/1", "", 0},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotURL, gotNumber := gitHubSubjectLink(tt.repoURL, tt.apiURL)
+			assertEqual(t, "URL", tt.wantURL, gotURL)
+			assertEqualInt(t, "Number", tt.wantNumber, gotNumber)
+		})
+	}
 }
 
 func TestGitHubListNotificationsUnread(t *testing.T) {

gitlab/notifications.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 
 	forge "github.com/git-pkgs/forge"
 	gitlab "gitlab.com/gitlab-org/api/client-go"
@@ -45,10 +46,19 @@ func convertGitLabTodo(t *gitlab.Todo) forge.Notification {
 
 	if t.Target != nil {
 		result.URL = t.TargetURL
+		switch result.SubjectType {
+		case forge.NotificationSubjectIssue, forge.NotificationSubjectPullRequest:
+			result.SubjectNumber = int(t.Target.IID)
+		}
 	}
 
 	if t.Project != nil {
 		result.Repo = t.Project.PathWithNamespace
+		// Todos carry no project web URL, but GitLab target pages live under the
+		// project at <project>/-/issues/5, <project>/-/merge_requests/3 and so on.
+		if repoURL, _, ok := strings.Cut(t.TargetURL, "/-/"); ok {
+			result.RepoURL = repoURL
+		}
 		if result.Title == "" {
 			result.Title = t.Project.Name
 		}

gitlab/notifications_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	forge "github.com/git-pkgs/forge"
+	gitlab "gitlab.com/gitlab-org/api/client-go"
 )
 
 func TestGitLabListNotifications(t *testing.T) {
@@ -22,7 +23,7 @@ func TestGitLabListNotifications(t *testing.T) {
 				"action_name": "mentioned",
 				"target_type": "Issue",
 				"target_url":  "https://gitlab.com/mygroup/myrepo/-/issues/5",
-				"target":      map[string]any{"id": 5},
+				"target":      map[string]any{"id": 501, "iid": 5},
 				"state":       "pending",
 				"project": map[string]any{
 					"id":                  10,
@@ -52,6 +53,64 @@ func TestGitLabListNotifications(t *testing.T) {
 	assertEqual(t, "SubjectType", "issue", string(notifications[0].SubjectType))
 	assertEqual(t, "Repo", "mygroup/myrepo", notifications[0].Repo)
 	assertEqualBool(t, "Unread", true, notifications[0].Unread)
+	assertEqualInt(t, "SubjectNumber", 5, notifications[0].SubjectNumber)
+	assertEqual(t, "URL", "https://gitlab.com/mygroup/myrepo/-/issues/5", notifications[0].URL)
+	assertEqual(t, "RepoURL", "https://gitlab.com/mygroup/myrepo", notifications[0].RepoURL)
+}
+
+func TestConvertGitLabTodoSubject(t *testing.T) {
+	tests := []struct {
+		name        string
+		todo        *gitlab.Todo
+		wantNumber  int
+		wantURL     string
+		wantRepoURL string
+	}{
+		{
+			name: "merge request",
+			todo: &gitlab.Todo{
+				TargetType: "MergeRequest",
+				Target:     &gitlab.TodoTarget{IID: 42},
+				TargetURL:  "https://gitlab.com/group/sub/repo/-/merge_requests/42",
+				Project:    &gitlab.BasicProject{PathWithNamespace: "group/sub/repo"},
+			},
+			wantNumber:  42,
+			wantURL:     "https://gitlab.com/group/sub/repo/-/merge_requests/42",
+			wantRepoURL: "https://gitlab.com/group/sub/repo",
+		},
+		{
+			name: "commit has no number",
+			todo: &gitlab.Todo{
+				TargetType: "Commit",
+				Target:     &gitlab.TodoTarget{IID: 0},
+				TargetURL:  "https://gitlab.com/group/repo/-/commit/abc123",
+				Project:    &gitlab.BasicProject{PathWithNamespace: "group/repo"},
+			},
+			wantURL:     "https://gitlab.com/group/repo/-/commit/abc123",
+			wantRepoURL: "https://gitlab.com/group/repo",
+		},
+		{
+			name: "epic keeps no number",
+			todo: &gitlab.Todo{
+				TargetType: "Epic",
+				Target:     &gitlab.TodoTarget{IID: 3},
+				TargetURL:  "https://gitlab.com/groups/group/-/epics/3",
+			},
+			wantURL: "https://gitlab.com/groups/group/-/epics/3",
+		},
+		{
+			name: "no target",
+			todo: &gitlab.Todo{TargetType: "Issue"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			n := convertGitLabTodo(tt.todo)
+			assertEqualInt(t, "SubjectNumber", tt.wantNumber, n.SubjectNumber)
+			assertEqual(t, "URL", tt.wantURL, n.URL)
+			assertEqual(t, "RepoURL", tt.wantRepoURL, n.RepoURL)
+		})
+	}
 }
 
 func TestGitLabListNotificationsUnread(t *testing.T) {

types.go

@@ -573,15 +573,21 @@ const (
 )
 
 // Notification holds normalized metadata about a notification thread.
+//
+// URL is the web page of the subject (the issue, pull request, commit and so
+// on) and RepoURL is the web page of its repository. Either is empty when the
+// forge does not report it.
 type Notification struct {
-	ID          string                  `json:"id"`
-	Title       string                  `json:"title"`
-	SubjectType NotificationSubjectType `json:"subject_type"`
-	Repo        string                  `json:"repo"`
-	Unread      bool                    `json:"unread"`
-	Reason      string                  `json:"reason,omitempty"`
-	URL         string                  `json:"url,omitempty"`
-	UpdatedAt   time.Time               `json:"updated_at"`
+	ID            string                  `json:"id"`
+	Title         string                  `json:"title"`
+	SubjectType   NotificationSubjectType `json:"subject_type"`
+	SubjectNumber int                     `json:"subject_number,omitempty"` // issue, pull request or discussion number; 0 for other subjects
+	Repo          string                  `json:"repo"`
+	RepoURL       string                  `json:"repo_url,omitempty"`
+	Unread        bool                    `json:"unread"`
+	Reason        string                  `json:"reason,omitempty"`
+	URL           string                  `json:"url,omitempty"`
+	UpdatedAt     time.Time               `json:"updated_at"`
 }
 
 // ListNotificationOpts holds options for listing notifications.