From 99a64946f423cdca21ebb353acfb69f4df9b134c Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Thu, 27 Apr 2023 17:15:51 +0200
Subject: dashboard: prioritize Merge=true KernelRepo links

It might be the case that the same repo is mentioned both as Merge=false
and via some chain of Merge=true links. Adjust the code to properly
handle such scenarios and add a test.
---
 dashboard/app/tree.go      | 10 ++++++--
 dashboard/app/tree_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/dashboard/app/tree.go b/dashboard/app/tree.go
index caaa06bde..263c4651b 100644
--- a/dashboard/app/tree.go
+++ b/dashboard/app/tree.go
@@ -694,10 +694,17 @@ func (g *repoGraph) nodeByAlias(alias string) *repoNode {
 // reachable returns a map *repoNode -> bool (whether commits are merged).
 func (n *repoNode) reachable(in bool) map[*repoNode]bool {
 	ret := map[*repoNode]bool{}
+	// First collect nodes only reachable via merge=true links.
+	n.reachableMerged(in, true, ret)
+	n.reachableMerged(in, false, ret)
+	return ret
+}
+
+func (n *repoNode) reachableMerged(in, onlyMerge bool, ret map[*repoNode]bool) {
 	var dfs func(*repoNode, bool)
 	dfs = func(node *repoNode, merge bool) {
 		for _, edge := range node.edges {
-			if edge.in != in {
+			if edge.in != in || onlyMerge && !edge.merge {
 				continue
 			}
 			if _, ok := ret[edge.other]; ok {
@@ -708,7 +715,6 @@ func (n *repoNode) reachable(in bool) map[*repoNode]bool {
 		}
 	}
 	dfs(n, true)
-	return ret
 }
 
 func (n *repoNode) allReachable() map[*repoNode]bool {
diff --git a/dashboard/app/tree_test.go b/dashboard/app/tree_test.go
index 555b543d9..5c4af14ec 100644
--- a/dashboard/app/tree_test.go
+++ b/dashboard/app/tree_test.go
@@ -126,11 +126,11 @@ var downstreamUpstreamRepos = []KernelRepo{
 		LabelIntroduced: `downstream`,
 		CommitInflow: []KernelRepoLink{
 			{
-				Alias: `lts`,
-				Merge: true,
+				Alias: `upstream`,
 			},
 			{
-				Alias: `upstream`,
+				Alias: `lts`,
+				Merge: true,
 			},
 		},
 	},
@@ -794,3 +794,58 @@ func TestRepoGraph(t *testing.T) {
 		t.Fatal(diff)
 	}
 }
+
+func TestRepoGraphMergeFirst(t *testing.T) {
+	// Test whether we prioritize merge links.
+	g, err := makeRepoGraph([]KernelRepo{
+		{
+			URL:    `https://downstream.repo/repo`,
+			Branch: `master`,
+			Alias:  `downstream`,
+			CommitInflow: []KernelRepoLink{
+				{
+					Alias: `upstream`,
+					Merge: false,
+				},
+				{
+					Alias: `lts`,
+					Merge: true,
+				},
+			},
+		},
+		{
+			URL:    `https://lts.repo/repo`,
+			Branch: `lts-master`,
+			Alias:  `lts`,
+			CommitInflow: []KernelRepoLink{
+				{
+					Alias: `upstream`,
+					Merge: true,
+				},
+			},
+		},
+		{
+			URL:    `https://upstream.repo/repo`,
+			Branch: `upstream-master`,
+			Alias:  `upstream`,
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	downstream := g.nodeByAlias(`downstream`)
+	lts := g.nodeByAlias(`lts`)
+	upstream := g.nodeByAlias(`upstream`)
+
+	// Test the downstream node.
+	if diff := cmp.Diff(map[*repoNode]bool{
+		lts:      true,
+		upstream: true,
+	}, downstream.reachable(true)); diff != "" {
+		t.Fatal(diff)
+	}
+	if diff := cmp.Diff(map[*repoNode]bool{}, downstream.reachable(false)); diff != "" {
+		t.Fatal(diff)
+	}
+}
-- 
cgit mrf-deployment