From e7cffc378fe8dd3b77959ad318483e829013726a Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Mon, 2 Dec 2024 00:43:54 +0100
Subject: [PATCH] Fix: return correct type in `GetSubModule`

- `GetSubModules` already solely stores the URL of the submodule and not
a `*SubModule` entry, so don't try to type assert it to be a struct.
- I am not able to pinpoint when this was regressed but if I had to
guess it might be #4941.
- Added integration test.
---
 modules/git/commit.go                | 10 ++++----
 modules/git/commit_info.go           |  6 ++---
 services/repository/files/content.go |  6 ++---
 tests/integration/repo_test.go       | 36 ++++++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/modules/git/commit.go b/modules/git/commit.go
index 0f3b749227..78468b907f 100644
--- a/modules/git/commit.go
+++ b/modules/git/commit.go
@@ -395,20 +395,20 @@ func parseSubmoduleContent(bs []byte) (*ObjectCache, error) {
 	return submoduleCache, nil
 }
 
-// GetSubModule get the sub module according entryname
-func (c *Commit) GetSubModule(entryname string) (*SubModule, error) {
+// GetSubModule returns the URL to the submodule according entryname
+func (c *Commit) GetSubModule(entryname string) (string, error) {
 	modules, err := c.GetSubModules()
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 
 	if modules != nil {
 		module, has := modules.Get(entryname)
 		if has {
-			return module.(*SubModule), nil
+			return module.(string), nil
 		}
 	}
-	return nil, nil
+	return "", nil
 }
 
 // GetBranchName gets the closest branch name (as returned by 'git name-rev --name-only')
diff --git a/modules/git/commit_info.go b/modules/git/commit_info.go
index 3b34b7933a..39e30b127d 100644
--- a/modules/git/commit_info.go
+++ b/modules/git/commit_info.go
@@ -72,17 +72,15 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 
 		// If the entry if a submodule add a submodule file for this
 		if entry.IsSubModule() {
-			subModuleURL := ""
 			var fullPath string
 			if len(treePath) > 0 {
 				fullPath = treePath + "/" + entry.Name()
 			} else {
 				fullPath = entry.Name()
 			}
-			if subModule, err := commit.GetSubModule(fullPath); err != nil {
+			subModuleURL, err := commit.GetSubModule(fullPath)
+			if err != nil {
 				return nil, nil, err
-			} else if subModule != nil {
-				subModuleURL = subModule.URL
 			}
 			subModuleFile := NewSubModuleFile(commitsInfo[i].Commit, subModuleURL, entry.ID.String())
 			commitsInfo[i].SubModuleFile = subModuleFile
diff --git a/services/repository/files/content.go b/services/repository/files/content.go
index 8dfd8b846d..32517e8d91 100644
--- a/services/repository/files/content.go
+++ b/services/repository/files/content.go
@@ -211,12 +211,12 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref
 		contentsResponse.Target = &targetFromContent
 	} else if entry.IsSubModule() {
 		contentsResponse.Type = string(ContentTypeSubmodule)
-		submodule, err := commit.GetSubModule(treePath)
+		submoduleURL, err := commit.GetSubModule(treePath)
 		if err != nil {
 			return nil, err
 		}
-		if submodule != nil && submodule.URL != "" {
-			contentsResponse.SubmoduleGitURL = &submodule.URL
+		if submoduleURL != "" {
+			contentsResponse.SubmoduleGitURL = &submoduleURL
 		}
 	}
 	// Handle links
diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go
index a424863346..145fb68a4c 100644
--- a/tests/integration/repo_test.go
+++ b/tests/integration/repo_test.go
@@ -1413,3 +1413,39 @@ func TestRepoIssueFilterLinks(t *testing.T) {
 		assert.True(t, called)
 	})
 }
+
+func TestRepoSubmoduleView(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+		repo, _, f := tests.CreateDeclarativeRepo(t, user2, "", []unit_model.Type{unit_model.TypeCode}, nil, nil)
+		defer f()
+
+		// Clone the repository, add a submodule and push it.
+		dstPath := t.TempDir()
+
+		uClone := *u
+		uClone.Path = repo.FullName()
+		uClone.User = url.UserPassword(user2.Name, userPassword)
+
+		t.Run("Clone", doGitClone(dstPath, &uClone))
+
+		_, _, err := git.NewCommand(git.DefaultContext, "submodule", "add").AddDynamicArguments(u.JoinPath("/user2/repo1").String()).RunStdString(&git.RunOpts{Dir: dstPath})
+		require.NoError(t, err)
+
+		_, _, err = git.NewCommand(git.DefaultContext, "add", "repo1", ".gitmodules").RunStdString(&git.RunOpts{Dir: dstPath})
+		require.NoError(t, err)
+
+		_, _, err = git.NewCommand(git.DefaultContext, "commit", "-m", "add submodule").RunStdString(&git.RunOpts{Dir: dstPath})
+		require.NoError(t, err)
+
+		_, _, err = git.NewCommand(git.DefaultContext, "push").RunStdString(&git.RunOpts{Dir: dstPath})
+		require.NoError(t, err)
+
+		// Check that the submodule entry exist and the link is correct.
+		req := NewRequest(t, "GET", "/"+repo.FullName())
+		resp := MakeRequest(t, req, http.StatusOK)
+
+		htmlDoc := NewHTMLParser(t, resp.Body)
+		htmlDoc.AssertElement(t, fmt.Sprintf(`tr[data-entryname="repo1"] a[href="%s"]`, u.JoinPath("/user2/repo1").String()), true)
+	})
+}