From 7b3b374d38b83c5e20357236d1180ce0fa912969 Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Thu, 22 Oct 2026 10:06:12 +0530 Subject: [PATCH 1/2] Add subject number and repository URL to notifications --- gitea/notifications.go | 30 ++++++++++++++++++ gitea/notifications_test.go | 32 +++++++++++++++++++ github/notifications.go | 46 ++++++++++++++++++++++++++- github/notifications_test.go | 35 +++++++++++++++++++++ gitlab/notifications.go | 11 +++++++ gitlab/notifications_test.go | 60 +++++++++++++++++++++++++++++++++++- types.go | 22 ++++++++----- 7 files changed, 226 insertions(+), 10 deletions(-) diff --git a/gitea/notifications.go b/gitea/notifications.go index f88b9d4..e1697c0 100644 --- a/gitea/notifications.go +++ b/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() { diff --git a/gitea/notifications_test.go b/gitea/notifications_test.go index dce7016..d2ea74b 100644 --- a/gitea/notifications_test.go +++ b/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) { diff --git a/github/notifications.go b/github/notifications.go index b6e0129..d553a41 100644 --- a/github/notifications.go +++ b/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() { diff --git a/github/notifications_test.go b/github/notifications_test.go index afcdaa2..e0b7c5f 100644 --- a/github/notifications_test.go +++ b/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) { diff --git a/gitlab/notifications.go b/gitlab/notifications.go index bfb7575..2f562ab 100644 --- a/gitlab/notifications.go +++ b/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,6 +46,16 @@ 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) + } + } + + // Todos carry no project web URL, but GitLab target pages live under the + // project at /-/issues/5, /-/merge_requests/3 and so on. + if repoURL, _, ok := strings.Cut(t.TargetURL, "/-/"); ok { + result.RepoURL = repoURL } if t.Project != nil { diff --git a/gitlab/notifications_test.go b/gitlab/notifications_test.go index ef5a4af..9b72f7e 100644 --- a/gitlab/notifications_test.go +++ b/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,63 @@ 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", + }, + 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", + }, + 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", + wantRepoURL: "https://gitlab.com/groups/group", + }, + { + 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) { diff --git a/types.go b/types.go index 65a0e6c..3562b5e 100644 --- a/types.go +++ b/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. From a6c59da008948a0741526f73113107b00aa9a014 Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Thu, 22 Oct 2026 20:12:19 +0530 Subject: [PATCH 2/2] Only set GitLab RepoURL for todos that belong to a project --- gitlab/notifications.go | 11 +++++------ gitlab/notifications_test.go | 5 +++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/gitlab/notifications.go b/gitlab/notifications.go index 2f562ab..11e3a40 100644 --- a/gitlab/notifications.go +++ b/gitlab/notifications.go @@ -52,14 +52,13 @@ func convertGitLabTodo(t *gitlab.Todo) forge.Notification { } } - // Todos carry no project web URL, but GitLab target pages live under the - // project at /-/issues/5, /-/merge_requests/3 and so on. - if repoURL, _, ok := strings.Cut(t.TargetURL, "/-/"); ok { - result.RepoURL = repoURL - } - if t.Project != nil { result.Repo = t.Project.PathWithNamespace + // Todos carry no project web URL, but GitLab target pages live under the + // project at /-/issues/5, /-/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 } diff --git a/gitlab/notifications_test.go b/gitlab/notifications_test.go index 9b72f7e..43217c0 100644 --- a/gitlab/notifications_test.go +++ b/gitlab/notifications_test.go @@ -72,6 +72,7 @@ func TestConvertGitLabTodoSubject(t *testing.T) { 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", @@ -83,6 +84,7 @@ func TestConvertGitLabTodoSubject(t *testing.T) { 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", @@ -94,8 +96,7 @@ func TestConvertGitLabTodoSubject(t *testing.T) { Target: &gitlab.TodoTarget{IID: 3}, TargetURL: "https://gitlab.com/groups/group/-/epics/3", }, - wantURL: "https://gitlab.com/groups/group/-/epics/3", - wantRepoURL: "https://gitlab.com/groups/group", + wantURL: "https://gitlab.com/groups/group/-/epics/3", }, { name: "no target",