diff --git a/plugin.json b/plugin.json index f0a2c923d..6f539e539 100644 --- a/plugin.json +++ b/plugin.json @@ -134,6 +134,19 @@ "type": "bool", "help_text": "When set to 'true' you will get a notification with less details when a draft pull request is created and a notification with complete details when they are marked as ready for review. When set to 'false' no notifications are delivered for draft pull requests.", "default": false + }, + { + "key": "ReviewTargetDays", + "display_name": "PR review target (days):", + "type": "number", + "default": "0", + "help_text": "Optional. Number of calendar days until a review counts as due. The start time is when you were requested as a reviewer (recorded from GitHub pull_request review_requested webhooks to this server); if unknown, the PR open date is used. When greater than zero, /github todo shows due/overdue text and the sidebar review counter can be color-coded. Set to 0 to disable." + }, + { + "key": "OverdueReviewsChannelID", + "display_name": "Post overdue review alerts to channel (ID):", + "type": "text", + "help_text": "Optional. Paste a channel ID (Channel menu > View Info). When set together with PR review target (days), the plugin posts one aggregated overdue-review digest to this channel each day shortly after midnight in the server local timezone (the GitHub bot must be a member of the channel)." } ], "footer": "* To report an issue, make a suggestion or a contribution, [check the repository](https://github.com/mattermost/mattermost-plugin-github)." diff --git a/server/plugin/api.go b/server/plugin/api.go index 52ae04105..a96520b21 100644 --- a/server/plugin/api.go +++ b/server/plugin/api.go @@ -1054,6 +1054,8 @@ func (p *Plugin) getSidebarData(c *UserContext) (*SidebarContent, error) { return nil, err } + p.enrichReviewsWithSLAStart(reviewResp, c.GHInfo.GitHubUsername) + return &SidebarContent{ PRs: openPRResp, Assignments: assignmentResp, diff --git a/server/plugin/configuration.go b/server/plugin/configuration.go index 9886560d1..a32b1739a 100644 --- a/server/plugin/configuration.go +++ b/server/plugin/configuration.go @@ -42,6 +42,10 @@ type Configuration struct { UsePreregisteredApplication bool `json:"usepreregisteredapplication"` ShowAuthorInCommitNotification bool `json:"showauthorincommitnotification"` GetNotificationForDraftPRs bool `json:"getnotificationfordraftprs"` + // ReviewTargetDays is the number of calendar days from PR open until a review is "due" (0 = SLA disabled). + ReviewTargetDays int `json:"reviewtargetdays"` + // OverdueReviewsChannelID is an optional channel ID for daily alerts when users have overdue review requests. + OverdueReviewsChannelID string `json:"overduereviewschannelid"` } func (c *Configuration) ToMap() (map[string]any, error) { @@ -100,6 +104,10 @@ func (c *Configuration) getBaseURL() string { func (c *Configuration) sanitize() { c.EnterpriseBaseURL = strings.TrimRight(c.EnterpriseBaseURL, "/") c.EnterpriseUploadURL = strings.TrimRight(c.EnterpriseUploadURL, "/") + c.OverdueReviewsChannelID = strings.TrimSpace(c.OverdueReviewsChannelID) + if c.ReviewTargetDays < 0 { + c.ReviewTargetDays = 0 + } // Trim spaces around org and OAuth credentials c.GitHubOrg = strings.TrimSpace(c.GitHubOrg) @@ -120,6 +128,7 @@ func (c *Configuration) IsSASS() bool { func (c *Configuration) ClientConfiguration() map[string]any { return map[string]any{ "left_sidebar_enabled": c.EnableLeftSidebar, + "review_target_days": c.ReviewTargetDays, } } diff --git a/server/plugin/graphql/lhs_request.go b/server/plugin/graphql/lhs_request.go index e3dc74399..2bcbb93c5 100644 --- a/server/plugin/graphql/lhs_request.go +++ b/server/plugin/graphql/lhs_request.go @@ -27,6 +27,8 @@ type GithubPRDetails struct { Additions *githubv4.Int `json:"additions,omitempty"` Deletions *githubv4.Int `json:"deletions,omitempty"` ChangedFiles *githubv4.Int `json:"changed_files,omitempty"` + // ReviewSLAStartAt is RFC3339 UTC time used for SLA when the plugin knows review request time (webhook); clients may fall back to created_at. + ReviewSLAStartAt *string `json:"review_sla_start,omitempty"` } func (c *Client) GetLHSData(ctx context.Context) ([]*GithubPRDetails, []*github.Issue, []*GithubPRDetails, error) { diff --git a/server/plugin/plugin.go b/server/plugin/plugin.go index 3fd21f43c..8cdaf39e5 100644 --- a/server/plugin/plugin.go +++ b/server/plugin/plugin.go @@ -14,6 +14,7 @@ import ( "slices" "strings" "sync" + "time" "github.com/google/go-github/v54/github" "github.com/gorilla/mux" @@ -103,6 +104,8 @@ type Plugin struct { webhookBroker *WebhookBroker oauthBroker *OAuthBroker + slaDigestCancel context.CancelFunc + emojiMap map[string]string } @@ -296,10 +299,18 @@ func (p *Plugin) OnActivate() error { p.client.Log.Debug("failed to reset user tokens", "error", resetErr.Error()) } }() + + ctx, cancel := context.WithCancel(context.Background()) + p.slaDigestCancel = cancel + go p.runSLADigestScheduler(ctx) + return nil } func (p *Plugin) OnDeactivate() error { + if p.slaDigestCancel != nil { + p.slaDigestCancel() + } p.webhookBroker.Close() p.oauthBroker.Close() return nil @@ -944,13 +955,21 @@ func (p *Plugin) GetToDo(ctx context.Context, info *GitHubUserInfo, githubClient text.WriteString("##### Review Requests\n") + targetDays := config.ReviewTargetDays + now := time.Now() + if issueResults.GetTotal() == 0 { text.WriteString("You don't have any pull requests awaiting your review.\n") } else { fmt.Fprintf(&text, "You have %v pull requests awaiting your review:\n", issueResults.GetTotal()) for _, pr := range issueResults.Issues { - text.WriteString(getToDoDisplayText(baseURL, pr.GetTitle(), pr.GetHTMLURL(), "", nil)) + line := strings.TrimSuffix(getToDoDisplayText(baseURL, pr.GetTitle(), pr.GetHTMLURL(), "", nil), "\n") + slaStart := p.effectiveReviewSLAStart(pr, baseURL, info.GitHubUsername) + if suffix, _ := reviewSLAMarkdown(slaStart, targetDays, now); suffix != "" { + line += suffix + } + text.WriteString(line + "\n") } } diff --git a/server/plugin/review_sla.go b/server/plugin/review_sla.go new file mode 100644 index 000000000..ee83b39f3 --- /dev/null +++ b/server/plugin/review_sla.go @@ -0,0 +1,136 @@ +// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin + +import ( + "crypto/sha256" + "encoding/hex" + "strconv" + "strings" + "time" + + "github.com/google/go-github/v54/github" + + "github.com/mattermost/mattermost-plugin-github/server/plugin/graphql" +) + +const slaReviewReqKeyPrefix = "slarr_v1_" + +// reviewSLAStartKey returns a stable KV key for (repo, PR, requested reviewer login). +func reviewSLAStartKey(owner, repo string, prNumber int, githubLogin string) string { + normalized := strings.ToLower(strings.TrimSpace(owner)) + "/" + strings.ToLower(strings.TrimSpace(repo)) + + "#" + strconv.Itoa(prNumber) + "@" + strings.ToLower(strings.TrimSpace(githubLogin)) + sum := sha256.Sum256([]byte(normalized)) + return slaReviewReqKeyPrefix + hex.EncodeToString(sum[:16]) +} + +// recordReviewRequestSLAStart stores when a reviewer was requested (from pull_request / review_requested webhook). +// Each new request overwrites the previous time for that (repo, PR, reviewer) pair so the SLA clock restarts on re-request. +func (p *Plugin) recordReviewRequestSLAStart(event *github.PullRequestEvent, requestedGitHubLogin string) { + if event.GetRepo() == nil || event.GetPullRequest() == nil { + return + } + owner := event.GetRepo().GetOwner().GetLogin() + repo := event.GetRepo().GetName() + num := event.GetPullRequest().GetNumber() + if owner == "" || repo == "" || num == 0 || requestedGitHubLogin == "" { + return + } + key := reviewSLAStartKey(owner, repo, num, requestedGitHubLogin) + at := time.Now().UTC() + val := []byte(at.Format(time.RFC3339Nano)) + if _, err := p.store.Set(key, val); err != nil { + p.client.Log.Warn("Failed to store review SLA start time", "key", key, "error", err.Error()) + } +} + +// cleanupReviewSLAKeys deletes all stored SLA start-time keys for a closed/merged PR. +func (p *Plugin) cleanupReviewSLAKeys(event *github.PullRequestEvent) { + if event.GetRepo() == nil || event.GetPullRequest() == nil { + return + } + owner := event.GetRepo().GetOwner().GetLogin() + repo := event.GetRepo().GetName() + num := event.GetPullRequest().GetNumber() + if owner == "" || repo == "" || num == 0 { + return + } + for _, reviewer := range event.GetPullRequest().RequestedReviewers { + login := reviewer.GetLogin() + if login == "" { + continue + } + key := reviewSLAStartKey(owner, repo, num, login) + if err := p.store.Delete(key); err != nil { + p.client.Log.Debug("Failed to delete SLA key on PR close", "key", key, "error", err.Error()) + } + } +} + +func (p *Plugin) getReviewSLAStartTime(owner, repo string, prNumber int, githubLogin string) time.Time { + key := reviewSLAStartKey(owner, repo, prNumber, githubLogin) + var raw []byte + if err := p.store.Get(key, &raw); err != nil { + return time.Time{} + } + if len(raw) == 0 { + return time.Time{} + } + t, err := time.Parse(time.RFC3339Nano, string(raw)) + if err != nil { + t, err = time.Parse(time.RFC3339, string(raw)) + } + if err != nil { + return time.Time{} + } + return t.UTC() +} + +// issueOwnerRepo resolves owner/name for a search result issue (prefers API fields, else HTML URL). +func issueOwnerRepo(pr *github.Issue, baseURL string) (owner, repo string) { + if pr.Repository != nil { + if o := pr.GetRepository().GetOwner(); o != nil { + owner = o.GetLogin() + } + repo = pr.GetRepository().GetName() + } + if owner != "" && repo != "" { + return owner, repo + } + return parseOwnerAndRepo(pr.GetHTMLURL(), baseURL) +} + +// effectiveReviewSLAStart returns the timestamp used for SLA: when we recorded a review_request webhook +// for this reviewer on this PR, else the PR created time. +func (p *Plugin) effectiveReviewSLAStart(pr *github.Issue, baseURL, reviewerGitHubLogin string) github.Timestamp { + owner, repo := issueOwnerRepo(pr, baseURL) + num := pr.GetNumber() + if owner == "" || repo == "" || num == 0 { + return pr.GetCreatedAt() + } + if t := p.getReviewSLAStartTime(owner, repo, num, reviewerGitHubLogin); !t.IsZero() { + return github.Timestamp{Time: t} + } + return pr.GetCreatedAt() +} + +// enrichReviewsWithSLAStart sets review_sla_start on LHS review items so the webapp can match server SLA logic. +func (p *Plugin) enrichReviewsWithSLAStart(reviews []*graphql.GithubPRDetails, reviewerLogin string) { + cfg := p.getConfiguration() + if cfg.ReviewTargetDays <= 0 { + return + } + baseURL := cfg.getBaseURL() + for _, d := range reviews { + if d == nil || d.Issue == nil { + continue + } + eff := p.effectiveReviewSLAStart(d.Issue, baseURL, reviewerLogin) + if eff.IsZero() { + continue + } + s := eff.Time.UTC().Format(time.RFC3339) + d.ReviewSLAStartAt = &s + } +} diff --git a/server/plugin/review_sla_test.go b/server/plugin/review_sla_test.go new file mode 100644 index 000000000..55dec9e08 --- /dev/null +++ b/server/plugin/review_sla_test.go @@ -0,0 +1,19 @@ +// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestReviewSLAStartKeyStable(t *testing.T) { + k1 := reviewSLAStartKey("Mattermost", "mattermost", 12345, "octocat") + k2 := reviewSLAStartKey("mattermost", "mattermost", 12345, "OctoCat") + assert.Equal(t, k1, k2, "key should be case-insensitive") + + k3 := reviewSLAStartKey("Mattermost", "mattermost", 99999, "octocat") + assert.NotEqual(t, k1, k3) +} diff --git a/server/plugin/sla_digest.go b/server/plugin/sla_digest.go new file mode 100644 index 000000000..eae14f275 --- /dev/null +++ b/server/plugin/sla_digest.go @@ -0,0 +1,201 @@ +// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin + +import ( + "context" + "fmt" + "sort" + "strings" + "time" + + "github.com/google/go-github/v54/github" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/pluginapi" + "github.com/mattermost/mattermost/server/public/pluginapi/cluster" + "golang.org/x/oauth2" +) + +const ( + // slaDigestDayKVKey stores the last local calendar day (server timezone) we posted or skipped the digest. + slaDigestDayKVKey = "github_sla_digest_local_day" + slaDigestMutexKey = "github_sla_digest_mutex" +) + +type slaDigestEntry struct { + DaysOverdue int + Line string +} + +// maybePostDailyOverdueSLADigest posts one aggregated message per local calendar day to the +// configured channel (at most once daily; see runSLADigestScheduler). +func (p *Plugin) maybePostDailyOverdueSLADigest(ctx context.Context) { + cfg := p.getConfiguration() + if cfg.OverdueReviewsChannelID == "" || cfg.ReviewTargetDays <= 0 { + return + } + + day := time.Now().In(time.Local).Format("2006-01-02") + var marker []byte + if err := p.store.Get(slaDigestDayKVKey, &marker); err != nil { + p.client.Log.Warn("Failed to read SLA digest day marker", "key", slaDigestDayKVKey, "error", err.Error()) + return + } + if string(marker) == day { + return + } + + m, err := cluster.NewMutex(p.API, slaDigestMutexKey) + if err != nil { + p.client.Log.Warn("Failed to create mutex for SLA digest", "error", err.Error()) + return + } + m.Lock() + defer m.Unlock() + + if err := p.store.Get(slaDigestDayKVKey, &marker); err != nil { + p.client.Log.Warn("Failed to read SLA digest day marker after lock", "key", slaDigestDayKVKey, "error", err.Error()) + return + } + if string(marker) == day { + return + } + + entries := p.collectAllOverdueSLAItems(ctx) + if len(entries) == 0 { + if _, err := p.store.Set(slaDigestDayKVKey, []byte(day)); err != nil { + p.client.Log.Warn("Failed to store SLA digest day marker", "error", err.Error()) + } + return + } + + msg := buildSLADigestMessage(entries) + post := &model.Post{ + ChannelId: cfg.OverdueReviewsChannelID, + UserId: p.BotUserID, + Message: msg, + } + if err := p.client.Post.CreatePost(post); err != nil { + p.client.Log.Warn("Failed to post SLA digest to channel", "error", err.Error()) + return + } + + if _, err := p.store.Set(slaDigestDayKVKey, []byte(day)); err != nil { + p.client.Log.Warn("Failed to store SLA digest day marker", "error", err.Error()) + } +} + +func (p *Plugin) collectAllOverdueSLAItems(ctx context.Context) []slaDigestEntry { + config := p.getConfiguration() + targetDays := config.ReviewTargetDays + baseURL := config.getBaseURL() + orgList := config.getOrganizations() + now := time.Now() + + checker := func(key string) (bool, error) { + return strings.HasSuffix(key, githubTokenKey), nil + } + + var out []slaDigestEntry + + for page := 0; ; page++ { + if ctx.Err() != nil { + return out + } + + keys, err := p.store.ListKeys(page, keysPerPage, pluginapi.WithChecker(checker)) + if err != nil { + p.client.Log.Warn("Failed to list keys for SLA digest", "error", err.Error()) + break + } + + for _, key := range keys { + if ctx.Err() != nil { + return out + } + + userID := strings.TrimSuffix(key, githubTokenKey) + ghInfo, apiErr := p.getGitHubUserInfo(userID) + if apiErr != nil || ghInfo == nil { + time.Sleep(delayBetweenUsers) + continue + } + + githubClient := p.githubConnectUser(ctx, ghInfo) + var allIssues []*github.Issue + cErr := p.useGitHubClient(ghInfo, func(gi *GitHubUserInfo, token *oauth2.Token) error { + query := getReviewSearchQuery(gi.GitHubUsername, orgList) + opts := &github.SearchOptions{ListOptions: github.ListOptions{PerPage: 100}} + for { + result, resp, searchErr := githubClient.Search.Issues(ctx, query, opts) + if searchErr != nil { + return searchErr + } + allIssues = append(allIssues, result.Issues...) + if resp.NextPage == 0 { + break + } + opts.Page = resp.NextPage + } + return nil + }) + if cErr != nil { + p.client.Log.Debug("SLA digest skipped user review search", "user_id", userID, "error", cErr.Error()) + time.Sleep(delayBetweenUsers) + continue + } + + for _, pr := range allIssues { + slaStart := p.effectiveReviewSLAStart(pr, baseURL, ghInfo.GitHubUsername) + diff := slaCalendarDiffDays(slaStart, targetDays, now) + if diff >= 0 { + continue + } + daysOverdue := -diff + line := formatChannelOverdueReviewLine(ghInfo.GitHubUsername, pr.GetTitle(), pr.GetHTMLURL(), baseURL) + out = append(out, slaDigestEntry{DaysOverdue: daysOverdue, Line: line}) + } + + time.Sleep(delayBetweenUsers) + } + + if len(keys) < keysPerPage { + break + } + } + + return out +} + +func buildSLADigestMessage(entries []slaDigestEntry) string { + buckets := make(map[int][]string) + for _, e := range entries { + buckets[e.DaysOverdue] = append(buckets[e.DaysOverdue], e.Line) + } + + days := make([]int, 0, len(buckets)) + for d := range buckets { + days = append(days, d) + } + sort.Sort(sort.Reverse(sort.IntSlice(days))) + + var b strings.Builder + b.WriteString("### Pull request reviews past SLA\n\n") + for _, d := range days { + lines := buckets[d] + sort.Strings(lines) + unit := "days" + if d == 1 { + unit = "day" + } + fmt.Fprintf(&b, "#### %d %s overdue\n", d, unit) + for _, line := range lines { + b.WriteString(line) + b.WriteString("\n") + } + b.WriteString("\n") + } + return strings.TrimSpace(b.String()) +} diff --git a/server/plugin/sla_digest_scheduler.go b/server/plugin/sla_digest_scheduler.go new file mode 100644 index 000000000..1ac943534 --- /dev/null +++ b/server/plugin/sla_digest_scheduler.go @@ -0,0 +1,66 @@ +// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin + +import ( + "context" + "time" +) + +// runSLADigestScheduler loops until ctx is cancelled: when SLA channel + target days are +// configured, it sleeps until the next midnight in the server's local timezone, then runs the +// overdue digest. This matches a simple "once per day" schedule without tying to user logins. +func (p *Plugin) runSLADigestScheduler(ctx context.Context) { + p.client.Log.Info("SLA digest scheduler started", "timezone", time.Local.String()) + for { + select { + case <-ctx.Done(): + return + default: + } + + if !p.slaDigestSchedulingEnabled() { + select { + case <-ctx.Done(): + return + case <-time.After(5 * time.Minute): + } + continue + } + + // Catch-up: run immediately for the current day (the day-marker deduplicates). + digestCtx, cancel := context.WithTimeout(ctx, 45*time.Minute) + p.maybePostDailyOverdueSLADigest(digestCtx) + cancel() + + d := durationUntilNextLocalMidnight() + select { + case <-ctx.Done(): + return + case <-time.After(d): + } + + digestCtx, cancel = context.WithTimeout(ctx, 45*time.Minute) + p.maybePostDailyOverdueSLADigest(digestCtx) + cancel() + + select { + case <-ctx.Done(): + return + case <-time.After(2 * time.Second): + } + } +} + +func (p *Plugin) slaDigestSchedulingEnabled() bool { + cfg := p.getConfiguration() + return cfg.OverdueReviewsChannelID != "" && cfg.ReviewTargetDays > 0 +} + +func durationUntilNextLocalMidnight() time.Duration { + loc := time.Local + now := time.Now().In(loc) + next := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, loc).AddDate(0, 0, 1) + return time.Until(next) +} diff --git a/server/plugin/utils.go b/server/plugin/utils.go index 1d5f0e771..67c262a08 100644 --- a/server/plugin/utils.go +++ b/server/plugin/utils.go @@ -16,6 +16,7 @@ import ( "path" "strconv" "strings" + "time" "unicode" "github.com/mattermost/mattermost/server/public/pluginapi" @@ -352,6 +353,61 @@ func getToDoDisplayText(baseURL, title, url, notifType string, repository *githu return fmt.Sprintf("* %s %s %s\n", repoPart, notifType, titlePart) } +// slaCalendarDiffDays returns dueDate minus today in calendar days (negative when the review is overdue). +func slaCalendarDiffDays(createdAt github.Timestamp, targetDays int, now time.Time) int { + if targetDays <= 0 || createdAt.IsZero() { + return 0 + } + + c := createdAt.UTC() + createdDay := time.Date(c.Year(), c.Month(), c.Day(), 0, 0, 0, 0, time.UTC) + dueDay := createdDay.AddDate(0, 0, targetDays) + + n := now.UTC() + todayDay := time.Date(n.Year(), n.Month(), n.Day(), 0, 0, 0, 0, time.UTC) + + return int(dueDay.Sub(todayDay) / (24 * time.Hour)) +} + +// formatChannelOverdueReviewLine formats a single line for the overdue-SLA channel digest. +func formatChannelOverdueReviewLine(githubLogin, title, htmlURL, baseURL string) string { + owner, repo := parseOwnerAndRepo(htmlURL, baseURL) + repoDisplay := fmt.Sprintf("%s/%s", owner, repo) + if owner == "" || repo == "" { + repoDisplay = htmlURL + } + if len(title) > 200 { + title = strings.TrimSpace(title[:200]) + "..." + } + return fmt.Sprintf("- %s - %s : %s", githubLogin, repoDisplay, title) +} + +// reviewSLAMarkdown returns a Markdown SLA suffix for Mattermost posts and whether the review is overdue. +func reviewSLAMarkdown(createdAt github.Timestamp, targetDays int, now time.Time) (suffix string, overdue bool) { + if targetDays <= 0 || createdAt.IsZero() { + return "", false + } + + diffDays := slaCalendarDiffDays(createdAt, targetDays, now) + + if diffDays < 0 { + overdueCount := -diffDays + unit := "days" + if overdueCount == 1 { + unit = "day" + } + return fmt.Sprintf(` **(:calendar: %d %s overdue)**`, overdueCount, unit), true + } + if diffDays == 0 { + return ` *(:calendar: Due today)*`, false + } + unit := "days" + if diffDays == 1 { + unit = "day" + } + return fmt.Sprintf(` *(:calendar: Due in %d %s)*`, diffDays, unit), false +} + // isValidURL checks if a given URL is a valid URL with a host and a http or http scheme. func isValidURL(rawURL string) error { u, err := url.ParseRequestURI(rawURL) diff --git a/server/plugin/utils_test.go b/server/plugin/utils_test.go index a6ea39d9b..c52a1f975 100644 --- a/server/plugin/utils_test.go +++ b/server/plugin/utils_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "testing" + "time" "github.com/google/go-github/v54/github" "github.com/stretchr/testify/assert" @@ -207,6 +208,40 @@ func TestInsideLink(t *testing.T) { } } +func TestReviewSLAMarkdown(t *testing.T) { + created := time.Date(2025, 3, 10, 15, 30, 0, 0, time.UTC) + ts := github.Timestamp{Time: created} + + t.Run("disabled when target is zero", func(t *testing.T) { + s, overdue := reviewSLAMarkdown(ts, 0, time.Date(2025, 3, 20, 0, 0, 0, 0, time.UTC)) + assert.Empty(t, s) + assert.False(t, overdue) + }) + + t.Run("overdue", func(t *testing.T) { + // Due March 15 (10th + 5), today March 19 -> 4 days overdue + now := time.Date(2025, 3, 19, 12, 0, 0, 0, time.UTC) + s, overdue := reviewSLAMarkdown(ts, 5, now) + assert.True(t, overdue) + assert.Contains(t, s, "4 days overdue") + }) + + t.Run("due in future", func(t *testing.T) { + // Due March 20 (10th + 10), today March 18 -> 2 days + now := time.Date(2025, 3, 18, 12, 0, 0, 0, time.UTC) + s, overdue := reviewSLAMarkdown(ts, 10, now) + assert.False(t, overdue) + assert.Contains(t, s, "Due in 2 days") + }) + + t.Run("due today", func(t *testing.T) { + now := time.Date(2025, 3, 20, 23, 59, 0, 0, time.UTC) + s, overdue := reviewSLAMarkdown(ts, 10, now) + assert.False(t, overdue) + assert.Contains(t, s, "Due today") + }) +} + func TestGetToDoDisplayText(t *testing.T) { type input struct { title string diff --git a/server/plugin/webhook.go b/server/plugin/webhook.go index 90d62ac4d..63fd7ae3e 100644 --- a/server/plugin/webhook.go +++ b/server/plugin/webhook.go @@ -1257,6 +1257,9 @@ func (p *Plugin) handlePullRequestNotification(event *github.PullRequestEvent) { switch event.GetAction() { case "review_requested": requestedReviewer = event.GetRequestedReviewer().GetLogin() + if requestedReviewer != "" { + p.recordReviewRequestSLAStart(event, requestedReviewer) + } if requestedReviewer == sender { return } @@ -1265,6 +1268,7 @@ func (p *Plugin) handlePullRequestNotification(event *github.PullRequestEvent) { requestedUserID = "" } case actionClosed: + p.cleanupReviewSLAKeys(event) if author == sender { return } diff --git a/webapp/src/components/sidebar_buttons/index.js b/webapp/src/components/sidebar_buttons/index.js index 6ecc4aebb..fae00bb0b 100644 --- a/webapp/src/components/sidebar_buttons/index.js +++ b/webapp/src/components/sidebar_buttons/index.js @@ -21,6 +21,7 @@ function mapStateToProps(state) { unreads: state[`plugins-${pluginId}`].sidebarContent.unreads, enterpriseURL: state[`plugins-${pluginId}`].enterpriseURL, showRHSPlugin: state[`plugins-${pluginId}`].rhsPluginAction, + reviewTargetDays: state[`plugins-${pluginId}`].configuration.review_target_days || 0, }; } diff --git a/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx b/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx index c4fb9847b..14c24272f 100644 --- a/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx +++ b/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx @@ -15,6 +15,7 @@ export default class SidebarButtons extends React.PureComponent { clientId: PropTypes.string, enterpriseURL: PropTypes.string, reviews: PropTypes.arrayOf(PropTypes.object), + reviewTargetDays: PropTypes.number, unreads: PropTypes.arrayOf(PropTypes.object), yourPrs: PropTypes.arrayOf(PropTypes.object), yourAssignments: PropTypes.arrayOf(PropTypes.object), @@ -118,6 +119,7 @@ export default class SidebarButtons extends React.PureComponent { } const reviews = this.props.reviews || []; + const reviewTargetDays = this.props.reviewTargetDays || 0; const yourPrs = this.props.yourPrs || []; const unreads = this.props.unreads || []; const yourAssignments = this.props.yourAssignments || []; @@ -159,7 +161,7 @@ export default class SidebarButtons extends React.PureComponent { > this.openRHS(RHSStates.REVIEWS)} - style={button} + style={reviewButtonStyle(button, reviews, reviewTargetDays)} > {' ' + reviews.length} @@ -209,6 +211,50 @@ export default class SidebarButtons extends React.PureComponent { } } +function reviewHasOverdue(reviews, targetDays) { + if (!targetDays || !reviews || reviews.length === 0) { + return false; + } + for (const pr of reviews) { + const startIso = pr.review_sla_start || pr.created_at; + if (!startIso) { + continue; + } + const created = new Date(startIso); + if (Number.isNaN(created.getTime())) { + continue; + } + const due = new Date(Date.UTC( + created.getUTCFullYear(), + created.getUTCMonth(), + created.getUTCDate(), + )); + due.setUTCDate(due.getUTCDate() + targetDays); + const today = new Date(); + const todayUTC = Date.UTC(today.getUTCFullYear(), today.getUTCMonth(), today.getUTCDate()); + const dueUTC = Date.UTC(due.getUTCFullYear(), due.getUTCMonth(), due.getUTCDate()); + const diffDays = Math.round((dueUTC - todayUTC) / (24 * 60 * 60 * 1000)); + if (diffDays < 0) { + return true; + } + } + return false; +} + +function reviewButtonStyle(base, reviews, targetDays) { + if (!targetDays) { + return base; + } + const list = reviews || []; + if (list.length === 0) { + return base; + } + if (reviewHasOverdue(list, targetDays)) { + return {...base, color: 'var(--dnd-indicator)'}; + } + return {...base, color: 'var(--online-indicator)'}; +} + const getStyle = makeStyleFromTheme((theme) => { return { buttonTeam: { diff --git a/webapp/src/reducers/index.ts b/webapp/src/reducers/index.ts index 0227f5184..f253aceb4 100644 --- a/webapp/src/reducers/index.ts +++ b/webapp/src/reducers/index.ts @@ -65,6 +65,7 @@ function userSettings(state = { function configuration(state = { left_sidebar_enabled: true, + review_target_days: 0, }, action: {type: string, data: ConnectedData | ConfigurationData}) { switch (action.type) { case ActionTypes.RECEIVED_CONNECTED: diff --git a/webapp/src/types/github_types.ts b/webapp/src/types/github_types.ts index 2bc95d5ea..e5b90415d 100644 --- a/webapp/src/types/github_types.ts +++ b/webapp/src/types/github_types.ts @@ -24,6 +24,9 @@ export type GithubItem = PrsDetailsData & { id: number; title: string; created_at: string; + + /** When set, instant used for review SLA (review request time if plugin recorded it). */ + review_sla_start?: string; updated_at: string; html_url: string; repository_url?: string; @@ -73,6 +76,7 @@ export type ConnectedData = { export type ConfigurationData = { left_sidebar_enabled: boolean; + review_target_days?: number; } export type PrsDetailsData = {