pr: enforce all maintainer approvals when ReviewRequired is true

- Update MaintainershipMap.IsApproved to require approvals from all listed
  maintainers (excluding the submitter) when ReviewRequired is enabled.
- Split ReviewInterface logic to differentiate between approvals and
  rejections by adding IsRejectedBy and restricting IsReviewedBy to
  the APPROVED state.
- Fix premature removal of maintainer review requests in
  FindMissingAndExtraReviewers; ensure requests remain active if
  ReviewRequired is true and approvals are still missing.
- Ensure any maintainer rejection correctly triggers the cleanup of
  other redundant review requests.

Analysis Summary
The failures in test_006 and test_007 were caused by two primary logic bugs:
1. Conflated Review States: IsReviewedBy treated both "Approved" and "Request Changes" as the same state. This meant a rejection didn't properly trigger the "cleanup" logic, and an approval was indistinguishable from a rejection in several checks.
2. Greedy Reviewer Removal: The bot assumed that any maintainer approval meant all other maintainer requests could be removed. This contradicted the ReviewRequired flag, which (as seen in test_007) is intended to mandate multiple/all maintainer approvals before the PR is considered ready.

common/maintainership.go

@@ -168,12 +168,32 @@ func (data *MaintainershipMap) IsApproved(pkg string, reviews []*models.PullRevi
 		return true
 	}
 
-	for _, review := range reviews {
-		if !review.Stale && review.State == ReviewStateApproved && slices.Contains(reviewers, review.User.UserName) {
-			LogDebug("Reviewed by", review.User.UserName)
-			return true
+	if !data.Config.ReviewRequired {
+		for _, review := range reviews {
+			if !review.Stale && review.State == ReviewStateApproved && slices.Contains(reviewers, review.User.UserName) {
+				LogDebug("Reviewed by", review.User.UserName)
+				return true
+			}
 		}
-
+	} else {
+		// all maintainers except submitter must approve
+		for _, m := range reviewers {
+			if m == submitter {
+				continue
+			}
+			approved := false
+			for _, review := range reviews {
+				if !review.Stale && review.State == ReviewStateApproved && review.User.UserName == m {
+					approved = true
+					break
+				}
+			}
+			if !approved {
+				LogDebug("Missing approval from maintainer:", m)
+				return false
+			}
+		}
+		return true
 	}
 	return false
 }

common/pr.go

@@ -328,7 +328,7 @@ func (rs *PRSet) FindMissingAndExtraReviewers(maintainers MaintainershipData, id
 		if len(rs.PRs) > 1 {
 			noReviewPRCreators = append(noReviewPRCreators, rs.BotUser)
 		}
-		if slices.Contains(noReviewPRCreators, pr.PR.User.UserName) || pr.Reviews.IsReviewedByOneOf(prjMaintainers...) {
+		if slices.Contains(noReviewPRCreators, pr.PR.User.UserName) || pr.Reviews.IsRejectedByOneOf(prjMaintainers...) || (pr.Reviews.IsReviewedByOneOf(prjMaintainers...) && !rs.Config.ReviewRequired) {
 			LogDebug("Project already reviewed by a project maintainer, remove rest")
 			// do not remove reviewers if they are also maintainers
 			prjMaintainers = slices.DeleteFunc(prjMaintainers, func(m string) bool { return slices.Contains(missing, m) })
@@ -355,8 +355,8 @@ func (rs *PRSet) FindMissingAndExtraReviewers(maintainers MaintainershipData, id
 		LogDebug("packakge maintainers:", Maintainers)
 
 		missing = slices.Concat(configReviewers.Pkg, configReviewers.PkgOptional)
-		if slices.Contains(noReviewPkgPRCreators, pr.PR.User.UserName) || pr.Reviews.IsReviewedByOneOf(Maintainers...) {
-			// submitter is maintainer or already reviewed
+		if slices.Contains(noReviewPkgPRCreators, pr.PR.User.UserName) || pr.Reviews.IsRejectedByOneOf(Maintainers...) || (pr.Reviews.IsReviewedByOneOf(Maintainers...) && !rs.Config.ReviewRequired) {
+			// submitter is maintainer or already reviewed/rejected
 			LogDebug("Package reviewed by maintainer (or subitter is maintainer), remove the rest of them")
 			// do not remove reviewers if they are also maintainers
 			Maintainers = slices.DeleteFunc(Maintainers, func(m string) bool { return slices.Contains(missing, m) })

common/reviews.go

@@ -16,6 +16,8 @@ type ReviewInterface interface {
 	HasPendingReviewBy(reviewer string) bool
 	IsReviewedBy(reviewer string) bool
 	IsReviewedByOneOf(reviewers ...string) bool
+	IsRejectedBy(reviewer string) bool
+	IsRejectedByOneOf(reviewers ...string) bool
 
 	SetRequiredReviewers(reviewers []string)
 }
@@ -230,7 +232,7 @@ func (r *PRReviews) IsReviewedBy(reviewer string) bool {
 	for _, r := range r.Reviews {
 		if r.User.UserName == reviewer && !r.Stale {
 			switch r.State {
-			case ReviewStateApproved, ReviewStateRequestChanges:
+			case ReviewStateApproved:
 				return true
 			default:
 				return false
@@ -244,6 +246,37 @@ func (r *PRReviews) IsReviewedBy(reviewer string) bool {
 func (r *PRReviews) IsReviewedByOneOf(reviewers ...string) bool {
 	for _, reviewer := range reviewers {
 		if r.IsReviewedBy(reviewer) {
+			LogDebug("  -- IsReviewedByOneOf: matched ", reviewer)
+			return true
+		}
+	}
+
+	return false
+}
+
+func (r *PRReviews) IsRejectedBy(reviewer string) bool {
+	if r == nil {
+		return false
+	}
+
+	for _, r := range r.Reviews {
+		if r.User.UserName == reviewer && !r.Stale {
+			switch r.State {
+			case ReviewStateRequestChanges:
+				return true
+			default:
+				return false
+			}
+		}
+	}
+
+	return false
+}
+
+func (r *PRReviews) IsRejectedByOneOf(reviewers ...string) bool {
+	for _, reviewer := range reviewers {
+		if r.IsRejectedBy(reviewer) {
+			LogDebug("  -- IsRejectedByOneOf: matched ", reviewer)
 			return true
 		}
 	}

gitea-events-rabbitmq-publisher/rabbitmq.go

@@ -92,10 +92,13 @@ func ConnectToExchangeForPublish(host, username, password string) {
 		auth = username + ":" + password + "@"
 	}
 
-	connection, err := rabbitmq.DialTLS("amqps://"+auth+host, &tls.Config{
-		ServerName: host,
+	connection, err := rabbitmq.DialConfig("amqps://"+auth+host, rabbitmq.Config{
+		Dial: rabbitmq.DefaultDial(10 * time.Second),
+		TLSClientConfig: &tls.Config{
+			ServerName: host,
+		},
 	})
-	failOnError(err, "Cannot connect to rabbit.opensuse.org")
+	failOnError(err, "Cannot connect to "+host)
 	defer connection.Close()
 
 	ch, err := connection.Channel()

integration/tests/workflow_pr_review_test.py

@@ -233,7 +233,6 @@ index 0000000..e69de29
 
 
 @pytest.mark.t006
-@pytest.mark.xfail(reason="tbd flacky in ci")
 def test_006_maintainer_rejection_removes_other_requests(maintainer_env, ownerA_client, ownerBB_client):
     """
     Test scenario:
@@ -287,7 +286,6 @@ index 0000000..e69de29
 
 
 @pytest.mark.t007
-@pytest.mark.xfail(reason="TBD troubleshoot")
 def test_007_review_required_needs_all_approvals(review_required_env, ownerA_client, ownerBB_client):
     """
     Test scenario:

pr-status-service/README.md

@@ -1,45 +0,0 @@
-OBS Status Service
-==================
-
-Caches and reports status of a PR as SVG or JSON
-
-Requests for individual PRs statuses:
-     GET /${PR_HASH}
-
-Update requests for individual PRs statuses:
-     POST /${PR_HASH}
-
-POST requires cert auth to function.
-
-
-Areas of Responsibility
------------------------
-
-  * Listens for PR status reports from workflow-pr bot (or other interface)
-  * Produces SVG output based on GET request
-  * Produces JSON output based on GET request
-
-
-Target Usage
-------------
-
-  * comment section of a PR
-  * 3rd party tooling
-
-
-PR Encoding
------------
-
-PRs are encoded as SHA256 hashes with a salt. This allows the existence of
-individual PRs to remain secret while the hash is known to tools that have
-read access to the PRs
-
-Encoding data is input into the sha256 hash as follows:
-
-   * Salt string, min 100 bytes.
-   * Organization name of the PR
-   * Repository name of the PR
-   * PR number (decimal string)
-
-Encoded PR is then passed to process as mime64 encoded without trailing =.
-

pr-status-service/main.go

@@ -1,113 +0,0 @@
-package main
-
-/*
- * This file is part of Autogits.
- *
- * Copyright © 2024 SUSE LLC
- *
- * Autogits is free software: you can redistribute it and/or modify it under
- * the terms of the GNU General Public License as published by the Free Software
- * Foundation, either version 2 of the License, or (at your option) any later
- * version.
- *
- * Autogits is distributed in the hope that it will be useful, but WITHOUT ANY
- * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
- * PARTICULAR PURPOSE. See the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * Foobar. If not, see <https://www.gnu.org/licenses/>.
- */
-
-import (
-	"encoding/json"
-	"flag"
-	"log"
-	"net/http"
-	"os"
-	"strings"
-
-	"src.opensuse.org/autogits/common"
-)
-
-const (
-	AppName = "obs-status-service"
-)
-
-var obs *common.ObsClient
-var debug bool
-var salt string
-var RabbitMQHost, Topic, orgs *string
-
-func LogDebug(v ...any) {
-	if debug {
-		log.Println(v...)
-	}
-}
-
-func main() {
-	cert := flag.String("cert-file", "", "TLS certificates file")
-	key := flag.String("key-file", "", "Private key for the TLS certificate")
-	listen := flag.String("listen", "[::1]:8080", "Listening string")
-	disableTls := flag.Bool("no-tls", false, "Disable TLS")
-	obsHost := flag.String("obs-host", "https://api.opensuse.org", "OBS API endpoint for package status information")
-	flag.BoolVar(&debug, "debug", false, "Enable debug logging")
-	RabbitMQHost = flag.String("rabbit-mq", "amqps://rabbit.opensuse.org", "RabbitMQ message bus server")
-	Topic = flag.String("topic", "opensuse.obs", "RabbitMQ topic prefix")
-	orgs = flag.String("orgs", "opensuse", "Comma separated list of orgs to watch")
-	flag.Parse()
-
-	salt = os.Getenv("PR_STATUS_SALT")
-	if len(salt) < 100 {
-		log.Fatal("PR_STATUS_SALT must be at least 100 bytes")
-	}
-
-	common.PanicOnError(common.RequireObsSecretToken())
-
-	var err error
-	if obs, err = common.NewObsClient(*obsHost); err != nil {
-		log.Fatal(err)
-	}
-
-	http.HandleFunc("GET /{PR_HASH}", func(res http.ResponseWriter, req *http.Request) {
-		hash := req.PathValue("PR_HASH")
-		isJSON := strings.HasSuffix(hash, ".json") || req.Header.Get("Accept") == "application/json"
-		hash = strings.TrimSuffix(hash, ".json")
-		hash = strings.TrimSuffix(hash, ".svg")
-
-		status := GetPRStatus(hash)
-		if status == nil {
-			res.WriteHeader(http.StatusNotFound)
-			return
-		}
-
-		if isJSON {
-			res.Header().Set("Content-Type", "application/json")
-			json.NewEncoder(res).Encode(status)
-			return
-		}
-
-		// default SVG
-		res.Header().Set("Content-Type", "image/svg+xml")
-		res.Write([]byte(status.ToSVG()))
-	})
-
-	http.HandleFunc("POST /{PR_HASH}", func(res http.ResponseWriter, req *http.Request) {
-		hash := req.PathValue("PR_HASH")
-
-		var status PRStatus
-		if err := json.NewDecoder(req.Body).Decode(&status); err != nil {
-			res.WriteHeader(http.StatusBadRequest)
-			return
-		}
-		UpdatePRStatus(hash, &status)
-		res.WriteHeader(http.StatusNoContent)
-	})
-
-	go ProcessUpdates()
-
-	if *disableTls {
-		log.Fatal(http.ListenAndServe(*listen, nil))
-	} else {
-		log.Fatal(http.ListenAndServeTLS(*listen, *cert, *key, nil))
-	}
-}

pr-status-service/rabbit.go

@@ -1,110 +0,0 @@
-package main
-
-import (
-	"fmt"
-	"log"
-	"net/url"
-	"strings"
-
-	"src.opensuse.org/autogits/common"
-)
-
-type PRHandler struct{}
-
-func (h *PRHandler) ProcessFunc(request *common.Request) error {
-	event, ok := request.Data.(*common.PullRequestWebhookEvent)
-	if !ok {
-		return nil
-	}
-
-	org := event.Repository.Owner.Username
-	repo := event.Repository.Name
-	prNum := fmt.Sprint(event.Number)
-	hash := CalculatePRHash(salt, org, repo, prNum)
-
-	status := &PRStatus{
-		PR:          fmt.Sprintf("%s/%s#%s", org, repo, prNum),
-		IsReviewed:  false,
-		IsMergeable: event.Pull_Request.State == "open",
-		MergeStatus: event.Pull_Request.State,
-	}
-
-	UpdatePRStatus(hash, status)
-	return nil
-}
-
-type ReviewHandler struct {
-	Approved bool
-}
-
-func (h *ReviewHandler) ProcessFunc(request *common.Request) error {
-	event, ok := request.Data.(*common.PullRequestWebhookEvent)
-	if !ok {
-		return nil
-	}
-
-	org := event.Repository.Owner.Username
-	repo := event.Repository.Name
-	prNum := fmt.Sprint(event.Number)
-	hash := CalculatePRHash(salt, org, repo, prNum)
-
-	status := GetPRStatus(hash)
-	if status == nil {
-		status = &PRStatus{
-			PR: fmt.Sprintf("%s/%s#%s", org, repo, prNum),
-		}
-	}
-
-	status.IsReviewed = true
-	isApproved := IsApproved_No
-	if h.Approved {
-		isApproved = IsApproved_Yes
-		status.MergeStatus = "approved"
-	} else {
-		status.MergeStatus = "rejected"
-	}
-
-	found := false
-	for i, r := range status.Reviews {
-		if r.Reviewer == event.Sender.Username {
-			status.Reviews[i].IsApproved = isApproved
-			found = true
-			break
-		}
-	}
-	if !found {
-		status.Reviews = append(status.Reviews, ReviewStatus{
-			Reviewer:   event.Sender.Username,
-			IsApproved: isApproved,
-		})
-	}
-
-	UpdatePRStatus(hash, status)
-	return nil
-}
-
-func ProcessUpdates() {
-	if RabbitMQHost == nil || *RabbitMQHost == "" {
-		return
-	}
-
-	u, err := url.Parse(*RabbitMQHost)
-	if err != nil {
-		log.Fatal(err)
-	}
-
-	processor := &common.RabbitMQGiteaEventsProcessor{
-		Handlers: map[string]common.RequestProcessor{
-			common.RequestType_PR:               &PRHandler{},
-			common.RequestType_PRReviewAccepted: &ReviewHandler{Approved: true},
-			common.RequestType_PRReviewRejected: &ReviewHandler{Approved: false},
-		},
-		Orgs: strings.Split(*orgs, ","),
-	}
-	processor.Connection().RabbitURL = u
-
-	err = common.ProcessRabbitMQEvents(processor)
-	if err != nil {
-		log.Fatal(err)
-	}
-}

pr-status-service/status.go

@@ -1,91 +0,0 @@
-package main
-
-import (
-	"crypto/sha256"
-	"encoding/base64"
-	"fmt"
-	"strings"
-	"sync"
-)
-
-const (
-	IsApproved_Pending = 0
-	IsApproved_Yes     = 1
-	IsApproved_No      = 2
-)
-
-type ReviewStatus struct {
-	Reviewer   string
-	IsApproved int
-}
-
-type PRStatus struct {
-	PR          string
-	IsReviewed  bool
-	IsMergeable bool
-
-	MergeStatus string
-	Reviews     []ReviewStatus
-}
-
-func (status *PRStatus) ToSVG() string {
-	mergeableText := "NO"
-	mergeableColor := "red"
-	if status.IsMergeable {
-		mergeableText = "YES"
-		mergeableColor = "green"
-	}
-
-	reviewedText := "NO"
-	reviewedColor := "red"
-	if status.IsReviewed {
-		reviewedText = "YES"
-		reviewedColor = "green"
-	}
-
-	var reviewsBuilder strings.Builder
-	for i, r := range status.Reviews {
-		color := "orange"
-		if r.IsApproved == IsApproved_Yes {
-			color = "green"
-		} else if r.IsApproved == IsApproved_No {
-			color = "red"
-		}
-		if i > 0 {
-			reviewsBuilder.WriteString(", ")
-		}
-		fmt.Fprintf(&reviewsBuilder, `<tspan fill="%s">%s</tspan>`, color, r.Reviewer)
-	}
-
-	return fmt.Sprintf(`<svg xmlns="http://www.w3.org/2000/svg" width="400" height="100">
-			<rect width="400" height="100" fill="#f8f9fa" stroke="#dee2e6"/>
-			<text x="10" y="25" font-family="sans-serif" font-size="14" fill="#333">Is Mergeable: <tspan fill="%s" font-weight="bold">%s</tspan></text>
-			<text x="10" y="45" font-family="sans-serif" font-size="14" fill="#333">Is Reviewed?: <tspan fill="%s" font-weight="bold">%s</tspan></text>
-			<text x="10" y="65" font-family="sans-serif" font-size="14" fill="#333">Merge Status: %s</text>
-			<text x="10" y="85" font-family="sans-serif" font-size="14" fill="#333">Reviews: %s</text>
-		</svg>`, mergeableColor, mergeableText, reviewedColor, reviewedText, status.MergeStatus, reviewsBuilder.String())
-}
-
-var prStatuses = make(map[string]*PRStatus)
-var prStatusesLock sync.RWMutex
-
-func GetPRStatus(hash string) *PRStatus {
-	prStatusesLock.RLock()
-	defer prStatusesLock.RUnlock()
-	return prStatuses[hash]
-}
-
-func UpdatePRStatus(hash string, status *PRStatus) {
-	prStatusesLock.Lock()
-	defer prStatusesLock.Unlock()
-	prStatuses[hash] = status
-}
-
-func CalculatePRHash(salt, org, repo, prNum string) string {
-	h := sha256.New()
-	h.Write([]byte(salt))
-	h.Write([]byte(org))
-	h.Write([]byte(repo))
-	h.Write([]byte(prNum))
-	return base64.RawStdEncoding.EncodeToString(h.Sum(nil))
-}

pr-status-service/status_test.go

@@ -1,185 +0,0 @@
-package main
-
-import (
-	"src.opensuse.org/autogits/common"
-	"testing"
-)
-
-func TestCalculatePRHash(t *testing.T) {
-	salt = "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" // 100 bytes
-	org := "opensuse"
-	repo := "pr-status-service"
-	prNum := "1"
-	
-	hash := CalculatePRHash(salt, org, repo, prNum)
-	if hash == "" {
-		t.Errorf("Hash is empty")
-	}
-	
-	hash2 := CalculatePRHash(salt, org, repo, prNum)
-	if hash != hash2 {
-		t.Errorf("Hash is not consistent")
-	}
-	
-	// Different inputs should give different hashes
-	hash3 := CalculatePRHash(salt, org, repo, "2")
-	if hash == hash3 {
-		t.Errorf("Hashes for different PR numbers are same")
-	}
-}
-
-func TestPRStatusToSVG(t *testing.T) {
-	status := &PRStatus{
-		PR:          "org/repo#1",
-		IsMergeable: true,
-		IsReviewed:  true,
-		MergeStatus: "can be merged",
-		Reviews: []ReviewStatus{
-			{Reviewer: "alice", IsApproved: IsApproved_Yes},
-			{Reviewer: "bob", IsApproved: IsApproved_No},
-			{Reviewer: "charlie", IsApproved: IsApproved_Pending},
-		},
-	}
-
-	svg := status.ToSVG()
-	
-	// Check for key elements in the SVG
-	expectedStrings := []string{
-		`Is Mergeable: <tspan fill="green" font-weight="bold">YES</tspan>`,
-		`Is Reviewed?: <tspan fill="green" font-weight="bold">YES</tspan>`,
-		`Merge Status: can be merged`,
-		`<tspan fill="green">alice</tspan>`,
-		`<tspan fill="red">bob</tspan>`,
-		`<tspan fill="orange">charlie</tspan>`,
-	}
-
-	for _, expected := range expectedStrings {
-		if !contains(svg, expected) {
-			t.Errorf("SVG missing expected string: %s", expected)
-		}
-	}
-}
-
-func TestPRStatusStore(t *testing.T) {
-	hash := "test-hash"
-	status := &PRStatus{PR: "org/repo#123"}
-	
-	UpdatePRStatus(hash, status)
-	
-	ret := GetPRStatus(hash)
-	if ret == nil || ret.PR != status.PR {
-		t.Errorf("Retrieved status does not match stored status")
-	}
-	
-	retNil := GetPRStatus("non-existent")
-	if retNil != nil {
-		t.Errorf("Expected nil for non-existent hash")
-	}
-}
-
-func TestHandlers(t *testing.T) {
-	salt = "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"
-
-	repo := &common.Repository{
-		Name: "test-repo",
-		Owner: &common.Organization{
-			Username: "test-org",
-		},
-	}
-	
-	prHandler := &PRHandler{}
-	prEvent := &common.PullRequestWebhookEvent{
-		Number: 1,
-		Repository: repo,
-		Pull_Request: &common.PullRequest{
-			State: "open",
-		},
-	}
-	
-	err := prHandler.ProcessFunc(&common.Request{Data: prEvent})
-	if err != nil {
-		t.Errorf("PRHandler error: %v", err)
-	}
-	
-	hash := CalculatePRHash(salt, "test-org", "test-repo", "1")
-	status := GetPRStatus(hash)
-	if status == nil {
-		t.Fatalf("Status not found after PRHandler")
-	}
-	
-	if status.IsMergeable != true {
-		t.Errorf("Expected mergeable true")
-	}
-
-	// Test ReviewHandler
-	reviewHandler := &ReviewHandler{Approved: true}
-	reviewEvent := &common.PullRequestWebhookEvent{
-		Number: 1,
-		Repository: repo,
-		Sender: common.User{
-			Username: "reviewer1",
-		},
-	}
-	
-	err = reviewHandler.ProcessFunc(&common.Request{Data: reviewEvent})
-	if err != nil {
-		t.Errorf("ReviewHandler error: %v", err)
-	}
-	
-	status = GetPRStatus(hash)
-	if !status.IsReviewed {
-		t.Errorf("Expected IsReviewed true")
-	}
-	if len(status.Reviews) != 1 || status.Reviews[0].Reviewer != "reviewer1" || status.Reviews[0].IsApproved != IsApproved_Yes {
-		t.Errorf("Review not correctly recorded")
-	}
-}
-
-func TestReviewUpdate(t *testing.T) {
-	repo := &common.Repository{
-		Name: "test-repo",
-		Owner: &common.Organization{
-			Username: "test-org",
-		},
-	}
-	hash := CalculatePRHash(salt, "test-org", "test-repo", "1")
-	UpdatePRStatus(hash, &PRStatus{PR: "test-org/test-repo#1"})
-	
-	reviewHandler := &ReviewHandler{Approved: true}
-	reviewEvent := &common.PullRequestWebhookEvent{
-		Number: 1,
-		Repository: repo,
-		Sender: common.User{
-			Username: "reviewer1",
-		},
-	}
-	
-	reviewHandler.ProcessFunc(&common.Request{Data: reviewEvent})
-	
-	// Update same review
-	reviewHandler.Approved = false
-	reviewHandler.ProcessFunc(&common.Request{Data: reviewEvent})
-	
-	status := GetPRStatus(hash)
-	if len(status.Reviews) != 1 || status.Reviews[0].IsApproved != IsApproved_No {
-		t.Errorf("Review not correctly updated")
-	}
-}
-
-func TestHandlerErrors(t *testing.T) {
-	prHandler := &PRHandler{}
-	err := prHandler.ProcessFunc(&common.Request{Data: nil})
-	if err != nil {
-		t.Errorf("Expected nil error for nil data in PRHandler")
-	}
-	
-	reviewHandler := &ReviewHandler{}
-	err = reviewHandler.ProcessFunc(&common.Request{Data: nil})
-	if err != nil {
-		t.Errorf("Expected nil error for nil data in ReviewHandler")
-	}
-}
-
-func contains(s, substr string) bool {
-	return len(s) >= len(substr) && (s == substr || (len(substr) > 0 && (s[:len(substr)] == substr || contains(s[1:], substr))))
-}

workflow-pr/pr_processor.go

@@ -470,7 +470,8 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error {
 				if pr.PR.State == "open" {
 					org, repo, idx := pr.PRComponents()
 					if prjGitPR.PR.HasMerged {
-						Gitea.AddComment(pr.PR, "This PR is merged via the associated Project PR.")
+						// TODO: use timeline here because this can spam if ManualMergePR fails
+						// Gitea.AddComment(pr.PR, "This PR is merged via the associated Project PR.")
 						err = Gitea.ManualMergePR(org, repo, idx, pr.PR.Head.Sha, false)
 						if _, ok := err.(*repository.RepoMergePullRequestConflict); !ok {
 							common.PanicOnError(err)