From 83b32fe8f20dc1e910866fa110b0d872df283f03 Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Tue, 9 Jan 2024 16:43:36 +0100
Subject: dashboard: include repro messages into daily limits

A recent failure (#4412) could have been more constrained if our
reproducer emails were included in our daily limits. Let's change that.
---
 dashboard/app/app_test.go       |  4 ++--
 dashboard/app/reporting.go      | 16 ++++++-------
 dashboard/app/reporting_test.go | 50 +++++++++++++++++++++++++++++++++++++++--
 3 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go
index 81b836ed6..a93a3e9ac 100644
--- a/dashboard/app/app_test.go
+++ b/dashboard/app/app_test.go
@@ -103,7 +103,7 @@ var testConfig = &GlobalConfig{
 			Reporting: []Reporting{
 				{
 					Name:       "reporting1",
-					DailyLimit: 3,
+					DailyLimit: 5,
 					Embargo:    14 * 24 * time.Hour,
 					Filter:     skipWithRepro,
 					Config: &TestConfig{
@@ -112,7 +112,7 @@ var testConfig = &GlobalConfig{
 				},
 				{
 					Name:       "reporting2",
-					DailyLimit: 3,
+					DailyLimit: 5,
 					Config: &TestConfig{
 						Index: 2,
 					},
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index 8b85702ca..92749f203 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -136,20 +136,18 @@ func needReport(c context.Context, typ string, state *ReportingState, bug *Bug)
 		return
 	}
 
-	// Limit number of reports sent per day,
-	// but don't limit sending repros to already reported bugs.
-	if bugReporting.Reported.IsZero() && ent.Sent >= reporting.DailyLimit {
+	// Limit number of reports sent per day.
+	if ent.Sent >= reporting.DailyLimit {
 		status = fmt.Sprintf("%v: out of quota for today", reporting.DisplayTitle)
 		reporting, bugReporting = nil, nil
 		return
 	}
 
 	// Ready to be reported.
-	if bugReporting.Reported.IsZero() {
-		// This update won't be committed, but it is useful as a best effort measure
-		// so that we don't overflow the limit in a single poll.
-		ent.Sent++
-	}
+	// This update won't be committed, but it is useful as a best effort measure
+	// so that we don't overflow the limit in a single poll.
+	ent.Sent++
+
 	status = fmt.Sprintf("%v: ready to report", reporting.DisplayTitle)
 	if !bugReporting.Reported.IsZero() {
 		status += fmt.Sprintf(" (reported%v on %v)",
@@ -1067,9 +1065,9 @@ func incomingCommandCmd(c context.Context, now time.Time, cmd *dashapi.BugUpdate
 	case dashapi.BugStatusOpen:
 		bug.Status = BugStatusOpen
 		bug.Closed = time.Time{}
+		stateEnt.Sent++
 		if bugReporting.Reported.IsZero() {
 			bugReporting.Reported = now
-			stateEnt.Sent++ // sending repro does not count against the quota
 		}
 		if bugReporting.OnHold.IsZero() && cmd.OnHold {
 			bugReporting.OnHold = now
diff --git a/dashboard/app/reporting_test.go b/dashboard/app/reporting_test.go
index 841a63285..343310d15 100644
--- a/dashboard/app/reporting_test.go
+++ b/dashboard/app/reporting_test.go
@@ -272,15 +272,22 @@ func TestReportingQuota(t *testing.T) {
 	c := NewCtx(t)
 	defer c.Close()
 
+	c.updateReporting("test1", "reporting1",
+		func(r Reporting) Reporting {
+			// Set a low daily limit.
+			r.DailyLimit = 2
+			return r
+		})
+
 	build := testBuild(1)
 	c.client.UploadBuild(build)
 
-	const numReports = 8 // quota is 3 per day
+	const numReports = 5
 	for i := 0; i < numReports; i++ {
 		c.client.ReportCrash(testCrash(build, i))
 	}
 
-	for _, reports := range []int{3, 3, 2, 0, 0} {
+	for _, reports := range []int{2, 2, 1, 0, 0} {
 		c.advanceTime(24 * time.Hour)
 		c.client.pollBugs(reports)
 		// Out of quota for today, so must get 0 reports.
@@ -288,6 +295,45 @@ func TestReportingQuota(t *testing.T) {
 	}
 }
 
+func TestReproReportingQuota(t *testing.T) {
+	// Test that new repro reports are also covered by daily limits.
+	c := NewCtx(t)
+	defer c.Close()
+
+	client := c.client
+	c.updateReporting("test1", "reporting1",
+		func(r Reporting) Reporting {
+			// Set a low daily limit.
+			r.DailyLimit = 2
+			return r
+		})
+
+	build := testBuild(1)
+	client.UploadBuild(build)
+
+	// First report of two.
+	c.advanceTime(time.Minute)
+	client.ReportCrash(testCrash(build, 1))
+	client.pollBug()
+
+	// Second report of two.
+	c.advanceTime(time.Minute)
+	crash := testCrash(build, 2)
+	client.ReportCrash(crash)
+	client.pollBug()
+
+	// Now we "find" a reproducer.
+	c.advanceTime(time.Minute)
+	client.ReportCrash(testCrashWithRepro(build, 1))
+
+	// But there's no quota for it.
+	client.pollBugs(0)
+
+	// Wait a day and the quota appears.
+	c.advanceTime(time.Hour * 24)
+	client.pollBug()
+}
+
 // Basic dup scenario: mark one bug as dup of another.
 func TestReportingDup(t *testing.T) {
 	c := NewCtx(t)
-- 
cgit mrf-deployment