MM-70112: Add business-days option for PR review SLA due dates

plugin.json

@@ -140,7 +140,24 @@
                 "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."
+                "help_text": "Optional. Number of days until a review counts as due (see PR review target day type). 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": "ReviewTargetDayType",
+                "display_name": "PR review target day type:",
+                "type": "dropdown",
+                "default": "calendar",
+                "help_text": "How PR review target (days) is counted when computing the due date. Calendar days count every day including weekends. Business days count Monday–Friday only (e.g. Friday + 2 business days = Tuesday). Overdue age shown in /github todo, sidebar badges, and digest buckets remains in calendar days either way.",
+                "options": [
+                    {
+                        "display_name": "Calendar days",
+                        "value": "calendar"
+                    },
+                    {
+                        "display_name": "Business days (Mon–Fri)",
+                        "value": "business"
+                    }
+                ]
             },
             {
                 "key": "OverdueReviewsChannelID",

server/plugin/configuration.go

@@ -42,8 +42,11 @@ 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 is the number of days from the review-request start until a review is "due" (0 = SLA disabled).
+	// Day counting follows ReviewTargetDayType.
 	ReviewTargetDays int `json:"reviewtargetdays"`
+	// ReviewTargetDayType is "calendar" (default) or "business" (Mon–Fri) for computing the due date.
+	ReviewTargetDayType string `json:"reviewtargetdaytype"`
 	// OverdueReviewsChannelID is an optional channel ID for daily alerts when users have overdue review requests.
 	OverdueReviewsChannelID string `json:"overduereviewschannelid"`
 	// DigestServiceUsername is the Mattermost username whose GitHub connection runs the overdue review digest.
@@ -112,6 +115,7 @@ func (c *Configuration) sanitize() {
 	if c.ReviewTargetDays < 0 {
 		c.ReviewTargetDays = 0
 	}
+	c.ReviewTargetDayType = normalizeSLADayType(c.ReviewTargetDayType)
 
 	// Trim spaces around org and OAuth credentials
 	c.GitHubOrg = strings.TrimSpace(c.GitHubOrg)
@@ -131,11 +135,16 @@ 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,
+		"left_sidebar_enabled":   c.EnableLeftSidebar,
+		"review_target_days":     c.ReviewTargetDays,
+		"review_target_day_type": c.reviewTargetDayType(),
 	}
 }
 
+func (c *Configuration) reviewTargetDayType() string {
+	return normalizeSLADayType(c.ReviewTargetDayType)
+}
+
 // Clone shallow copies the configuration. Your implementation may require a deep copy if
 // your configuration has reference types.
 func (c *Configuration) Clone() *Configuration {

server/plugin/plugin.go

@@ -1131,6 +1131,7 @@ func (p *Plugin) GetToDo(ctx context.Context, info *GitHubUserInfo, githubClient
 	text.WriteString("##### Review Requests\n")
 
 	targetDays := config.ReviewTargetDays
+	dayType := config.reviewTargetDayType()
 	now := time.Now()
 
 	if issueResults.GetTotal() == 0 {
@@ -1141,7 +1142,7 @@ func (p *Plugin) GetToDo(ctx context.Context, info *GitHubUserInfo, githubClient
 		for _, pr := range issueResults.Issues {
 			line := strings.TrimSuffix(getToDoDisplayText(baseURL, pr.GetTitle(), pr.GetHTMLURL(), "", nil), "\n")
 			slaStart := p.effectiveReviewSLAStart(prRefFromIssue(pr, baseURL), info.GitHubUsername)
-			if suffix, _ := reviewSLAMarkdown(slaStart, targetDays, now); suffix != "" {
+			if suffix, _ := reviewSLAMarkdown(slaStart, targetDays, now, dayType); suffix != "" {
 				line += suffix
 			}
 			text.WriteString(line + "\n")

server/plugin/sla_digest.go

@@ -45,6 +45,8 @@ func (p *Plugin) maybePostDailyOverdueSLADigest(ctx context.Context) {
 	if cfg.OverdueReviewsChannelID == "" || cfg.ReviewTargetDays <= 0 {
 		return
 	}
+	targetDays := cfg.ReviewTargetDays
+	dayType := cfg.reviewTargetDayType()
 
 	day := time.Now().In(time.Local).Format("2006-01-02")
 	var marker []byte
@@ -72,7 +74,7 @@ func (p *Plugin) maybePostDailyOverdueSLADigest(ctx context.Context) {
 		return
 	}
 
-	entries, ok := p.collectAllOverdueSLAItems(ctx)
+	entries, ok := p.collectAllOverdueSLAItems(ctx, targetDays, dayType)
 	if !ok {
 		// Distinguishes "digest could not complete a real scan" (config issue, no service user,
 		// or every configured org's GraphQL fetch failed) from "scan ran and found nothing
@@ -87,7 +89,7 @@ func (p *Plugin) maybePostDailyOverdueSLADigest(ctx context.Context) {
 		return
 	}
 
-	msg := clipSLADigestMessage(buildSLADigestMessage(entries, cfg.ReviewTargetDays))
+	msg := clipSLADigestMessage(buildSLADigestMessage(entries, targetDays, dayType))
 	post := &model.Post{
 		ChannelId: cfg.OverdueReviewsChannelID,
 		UserId:    p.BotUserID,
@@ -192,9 +194,8 @@ func (p *Plugin) pickServiceGitHubUser(ctx context.Context) *GitHubUserInfo {
 // configured org's GraphQL fetch failed); the caller should retry on the next scheduler tick
 // rather than treat that as "ran successfully and found nothing." A successful scan returns
 // ok=true even when entries is empty.
-func (p *Plugin) collectAllOverdueSLAItems(ctx context.Context) ([]slaDigestEntry, bool) {
+func (p *Plugin) collectAllOverdueSLAItems(ctx context.Context, targetDays int, dayType string) ([]slaDigestEntry, bool) {
 	config := p.getConfiguration()
-	targetDays := config.ReviewTargetDays
 	orgList := config.getOrganizations()
 	now := time.Now()
 
@@ -242,7 +243,7 @@ func (p *Plugin) collectAllOverdueSLAItems(ctx context.Context) ([]slaDigestEntr
 			CreatedAt: github.Timestamp{Time: pr.CreatedAt},
 		}
 		for _, rr := range gatherReviewersForPR(pr, resolveTeam) {
-			entry := p.evaluateOverdueForReviewer(ref, pr, rr, targetDays, now, seen, resolveSLAStart)
+			entry := p.evaluateOverdueForReviewer(ref, pr, rr, targetDays, dayType, now, seen, resolveSLAStart)
 			if entry != nil {
 				out = append(out, *entry)
 			}
@@ -449,6 +450,7 @@ func (p *Plugin) evaluateOverdueForReviewer(
 	pr graphql.DigestPR,
 	rr reviewerRequest,
 	targetDays int,
+	dayType string,
 	now time.Time,
 	seen map[string]bool,
 	resolveSLAStart func(prRef, reviewerRequest) github.Timestamp,
@@ -463,7 +465,7 @@ func (p *Plugin) evaluateOverdueForReviewer(
 	seen[dedupeKey] = true
 
 	slaStart := resolveSLAStart(ref, rr)
-	diff := slaCalendarDiffDays(slaStart, targetDays, now)
+	diff := slaDiffDays(slaStart, targetDays, now, dayType)
 	if diff >= 0 {
 		return nil
 	}
@@ -542,7 +544,7 @@ func groupBucketEntriesByReviewer(entries []slaDigestEntry) []reviewerBucketGrou
 	return out
 }
 
-func buildSLADigestMessage(entries []slaDigestEntry, targetDays int) string {
+func buildSLADigestMessage(entries []slaDigestEntry, targetDays int, dayType string) string {
 	bucketEntries := make([][]slaDigestEntry, len(slaBuckets))
 	for _, e := range entries {
 		idx := slaBucketIndex(e.DaysOverdue)
@@ -555,7 +557,13 @@ func buildSLADigestMessage(entries []slaDigestEntry, targetDays int) string {
 	var b strings.Builder
 	if targetDays > 0 {
 		unit := "days"
-		if targetDays == 1 {
+		if normalizeSLADayType(dayType) == slaDayTypeBusiness {
+			if targetDays == 1 {
+				unit = "business day"
+			} else {
+				unit = "business days"
+			}
+		} else if targetDays == 1 {
 			unit = "day"
 		}
 		fmt.Fprintf(&b, "### Pull request reviews past SLA (target: %d %s from most recent review request)\n\n", targetDays, unit)

server/plugin/sla_digest_test.go

@@ -130,7 +130,7 @@ func TestBuildSLADigestMessage(t *testing.T) {
 			entry(400, "@e (e-gh)", "owner/repo - [E](url)"),
 		}
 
-		msg := buildSLADigestMessage(entries, 3)
+		msg := buildSLADigestMessage(entries, 3, slaDayTypeCalendar)
 
 		assert.True(t, strings.HasPrefix(msg, "### Pull request reviews past SLA (target: 3 days from most recent review request)"))
 
@@ -152,19 +152,24 @@ func TestBuildSLADigestMessage(t *testing.T) {
 			entry(-2, "@skip2 (skip2-gh)", "owner/repo - [skipped-too](url)"),
 			entry(1, "@keep (keep-gh)", "owner/repo - [kept](url)"),
 		}
-		msg := buildSLADigestMessage(entries, 3)
+		msg := buildSLADigestMessage(entries, 3, slaDayTypeCalendar)
 		assert.Contains(t, msg, "[kept]")
 		assert.NotContains(t, msg, "[skipped]")
 		assert.NotContains(t, msg, "[skipped-too]")
 	})
 
 	t.Run("singular target days uses 'day'", func(t *testing.T) {
-		msg := buildSLADigestMessage([]slaDigestEntry{entry(1, "@x (x-gh)", "owner/repo - [X](url)")}, 1)
+		msg := buildSLADigestMessage([]slaDigestEntry{entry(1, "@x (x-gh)", "owner/repo - [X](url)")}, 1, slaDayTypeCalendar)
 		assert.Contains(t, msg, "target: 1 day from")
 	})
 
+	t.Run("business day type uses business day wording", func(t *testing.T) {
+		msg := buildSLADigestMessage([]slaDigestEntry{entry(1, "@x (x-gh)", "owner/repo - [X](url)")}, 2, slaDayTypeBusiness)
+		assert.Contains(t, msg, "target: 2 business days from")
+	})
+
 	t.Run("zero target days falls back to plain header", func(t *testing.T) {
-		msg := buildSLADigestMessage([]slaDigestEntry{entry(1, "@x (x-gh)", "owner/repo - [X](url)")}, 0)
+		msg := buildSLADigestMessage([]slaDigestEntry{entry(1, "@x (x-gh)", "owner/repo - [X](url)")}, 0, slaDayTypeCalendar)
 		assert.True(t, strings.HasPrefix(msg, "### Pull request reviews past SLA\n"))
 	})
 
@@ -174,7 +179,7 @@ func TestBuildSLADigestMessage(t *testing.T) {
 			entry(2, "@alpha (alpha-gh)", "owner/repo - [PR-a](url)"),
 			entry(2, "@Mu (mu-gh)", "owner/repo - [PR-m](url)"),
 		}
-		msg := buildSLADigestMessage(entries, 3)
+		msg := buildSLADigestMessage(entries, 3, slaDayTypeCalendar)
 		ai := strings.Index(msg, "@alpha")
 		mi := strings.Index(msg, "@Mu")
 		zi := strings.Index(msg, "@Zeta")
@@ -188,7 +193,7 @@ func TestBuildSLADigestMessage(t *testing.T) {
 			entry(2, reviewer, "owner/repo - [alpha-pr](https://example/pr/1)"),
 			entry(2, reviewer, "owner/repo - [mu-pr](https://example/pr/2)"),
 		}
-		msg := buildSLADigestMessage(entries, 3)
+		msg := buildSLADigestMessage(entries, 3, slaDayTypeCalendar)
 
 		// The reviewer header must appear EXACTLY once in this bucket — that's the whole
 		// point of grouping; otherwise the digest still @-spams the reviewer per-PR.
@@ -212,7 +217,7 @@ func TestBuildSLADigestMessage(t *testing.T) {
 			entry(2, "@alice (alice-gh)", "o/r - [b-pr](url)"),
 			entry(2, "@bob (bob-gh)", "o/r - [c-pr](url)"),
 		}
-		msg := buildSLADigestMessage(entries, 3)
+		msg := buildSLADigestMessage(entries, 3, slaDayTypeCalendar)
 		bucketStart := strings.Index(msg, "#### Overdue\n")
 		require.True(t, bucketStart >= 0)
 		bucket := msg[bucketStart:]

server/plugin/utils.go

@@ -367,15 +367,46 @@ 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 {
+const (
+	slaDayTypeCalendar = "calendar"
+	slaDayTypeBusiness = "business"
+)
+
+func normalizeSLADayType(dayType string) string {
+	if strings.EqualFold(strings.TrimSpace(dayType), slaDayTypeBusiness) {
+		return slaDayTypeBusiness
+	}
+	return slaDayTypeCalendar
+}
+
+// addBusinessDays returns start advanced by n weekdays (Mon–Fri). Weekends are skipped.
+func addBusinessDays(start time.Time, n int) time.Time {
+	d := start
+	for remaining := n; remaining > 0; {
+		d = d.AddDate(0, 0, 1)
+		wd := d.Weekday()
+		if wd != time.Saturday && wd != time.Sunday {
+			remaining--
+		}
+	}
+	return d
+}
+
+// slaDiffDays returns dueDate minus today in calendar days (negative when the review is overdue).
+// dayType controls how the due date is computed from targetDays; the returned difference is always calendar days.
+func slaDiffDays(createdAt github.Timestamp, targetDays int, now time.Time, dayType string) 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)
+	var dueDay time.Time
+	if normalizeSLADayType(dayType) == slaDayTypeBusiness {
+		dueDay = addBusinessDays(createdDay, targetDays)
+	} else {
+		dueDay = createdDay.AddDate(0, 0, targetDays)
+	}
 
 	n := now.UTC()
 	todayDay := time.Date(n.Year(), n.Month(), n.Day(), 0, 0, 0, 0, time.UTC)
@@ -438,12 +469,12 @@ func escapeMarkdownLinkText(s string) string {
 }
 
 // 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) {
+func reviewSLAMarkdown(createdAt github.Timestamp, targetDays int, now time.Time, dayType string) (suffix string, overdue bool) {
 	if targetDays <= 0 || createdAt.IsZero() {
 		return "", false
 	}
 
-	diffDays := slaCalendarDiffDays(createdAt, targetDays, now)
+	diffDays := slaDiffDays(createdAt, targetDays, now, dayType)
 
 	if diffDays < 0 {
 		overdueCount := -diffDays

server/plugin/utils_test.go

@@ -213,35 +213,65 @@ func TestReviewSLAMarkdown(t *testing.T) {
 	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))
+		s, overdue := reviewSLAMarkdown(ts, 0, time.Date(2025, 3, 20, 0, 0, 0, 0, time.UTC), slaDayTypeCalendar)
 		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)
+		s, overdue := reviewSLAMarkdown(ts, 5, now, slaDayTypeCalendar)
 		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)
+		s, overdue := reviewSLAMarkdown(ts, 10, now, slaDayTypeCalendar)
 		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)
+		s, overdue := reviewSLAMarkdown(ts, 10, now, slaDayTypeCalendar)
 		assert.False(t, overdue)
 		assert.Contains(t, s, "Due today")
 	})
 }
 
+func TestAddBusinessDays(t *testing.T) {
+	// Friday 2025-03-14
+	fri := time.Date(2025, 3, 14, 0, 0, 0, 0, time.UTC)
+	assert.Equal(t, time.Date(2025, 3, 17, 0, 0, 0, 0, time.UTC), addBusinessDays(fri, 1)) // Mon
+	assert.Equal(t, time.Date(2025, 3, 18, 0, 0, 0, 0, time.UTC), addBusinessDays(fri, 2)) // Tue
+	assert.Equal(t, time.Date(2025, 3, 21, 0, 0, 0, 0, time.UTC), addBusinessDays(fri, 5)) // next Fri
+
+	sat := time.Date(2025, 3, 15, 0, 0, 0, 0, time.UTC)
+	assert.Equal(t, time.Date(2025, 3, 17, 0, 0, 0, 0, time.UTC), addBusinessDays(sat, 1)) // Mon
+
+	sun := time.Date(2025, 3, 16, 0, 0, 0, 0, time.UTC)
+	assert.Equal(t, time.Date(2025, 3, 17, 0, 0, 0, 0, time.UTC), addBusinessDays(sun, 1)) // Mon
+}
+
+func TestSLADiffDaysBusiness(t *testing.T) {
+	// Friday evening request, 2 business-day target → due Tuesday.
+	fri := github.Timestamp{Time: time.Date(2025, 3, 14, 18, 0, 0, 0, time.UTC)}
+	mon := time.Date(2025, 3, 17, 12, 0, 0, 0, time.UTC)
+	assert.Equal(t, 1, slaDiffDays(fri, 2, mon, slaDayTypeBusiness), "Mon should be due in 1 calendar day")
+
+	tue := time.Date(2025, 3, 18, 12, 0, 0, 0, time.UTC)
+	assert.Equal(t, 0, slaDiffDays(fri, 2, tue, slaDayTypeBusiness), "Tue should be due today")
+
+	wed := time.Date(2025, 3, 19, 12, 0, 0, 0, time.UTC)
+	assert.Equal(t, -1, slaDiffDays(fri, 2, wed, slaDayTypeBusiness), "Wed should be 1 calendar day overdue")
+
+	// Same start/target in calendar mode: Fri+2 = Sun → Mon is already overdue.
+	assert.Equal(t, -1, slaDiffDays(fri, 2, mon, slaDayTypeCalendar))
+}
+
 func TestGetToDoDisplayText(t *testing.T) {
 	type input struct {
 		title      string

webapp/src/components/sidebar_buttons/index.js

@@ -22,6 +22,7 @@ function mapStateToProps(state) {
         enterpriseURL: state[`plugins-${pluginId}`].enterpriseURL,
         showRHSPlugin: state[`plugins-${pluginId}`].rhsPluginAction,
         reviewTargetDays: state[`plugins-${pluginId}`].configuration.review_target_days || 0,
+        reviewTargetDayType: state[`plugins-${pluginId}`].configuration.review_target_day_type || 'calendar',
     };
 }