From eb6336ce7a34f84e0787e8bf438b042f2d206d73 Mon Sep 17 00:00:00 2001
From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com>
Date: Fri, 26 Jul 2024 11:45:42 +0530
Subject: [PATCH] Fixed bug around channel file sidebar (#27705)

* Fixed the issue for DB layer, ES to go

* Handled channel bookmarks

* Handled Bleve

* Lint fix

* Added channel bookmark test

* Skip bleve test

* Used common function

* SKipping ES as indexing logic in unavailable in test
---
 .../store/searchtest/file_info_layer.go       | 41 +++++++++++++++++++
 .../store/sqlstore/file_info_store.go         |  4 ++
 .../bleveengine/indexer/indexing_job.go       |  2 +-
 server/public/model/post.go                   | 16 ++++++++
 4 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/server/channels/store/searchtest/file_info_layer.go b/server/channels/store/searchtest/file_info_layer.go
index 9d44b5353b..cf21046ef6 100644
--- a/server/channels/store/searchtest/file_info_layer.go
+++ b/server/channels/store/searchtest/file_info_layer.go
@@ -183,6 +183,16 @@ var searchFileInfoStoreTests = []searchTest{
 		Fn:   testFileInfoSearchEmailsWithoutQuotes,
 		Tags: []string{EngineElasticSearch},
 	},
+	{
+		Name: "Should not search files not attached to a post",
+		Fn:   testFileInfoSearchNoResultForPostlessFileInfos,
+		Tags: []string{EnginePostgres, EngineMySQL},
+	},
+	{
+		Name: "Should search files part of channel bookmarks",
+		Fn:   testFileInfoSearchShowChannelBookmarkFiles,
+		Tags: []string{EnginePostgres, EngineMySQL},
+	},
 }
 
 func TestSearchFileInfoStore(t *testing.T, s store.Store, testEngine *SearchTestEngine) {
@@ -1645,3 +1655,34 @@ func testFileInfoSearchEmailsWithoutQuotes(t *testing.T, th *SearchTestHelper) {
 	require.Len(t, results.FileInfos, 1)
 	th.checkFileInfoInSearchResults(t, p1.Id, results.FileInfos)
 }
+
+func testFileInfoSearchNoResultForPostlessFileInfos(t *testing.T, th *SearchTestHelper) {
+	_, err := th.createFileInfo(th.User.Id, "", th.ChannelBasic.Id, "message test@test.com", "message test@test.com", "jpg", "image/jpeg", 0, 0)
+	require.NoError(t, err)
+
+	defer th.deleteUserFileInfos(th.User.Id)
+
+	params := &model.SearchParams{
+		InChannels: []string{th.ChannelBasic.Id},
+	}
+	results, err := th.Store.FileInfo().Search(th.Context, []*model.SearchParams{params}, th.User.Id, th.Team.Id, 0, 20)
+	require.NoError(t, err)
+
+	require.Len(t, results.FileInfos, 0)
+}
+
+func testFileInfoSearchShowChannelBookmarkFiles(t *testing.T, th *SearchTestHelper) {
+	file, err := th.createFileInfo("bookmark", "", th.ChannelBasic.Id, "message test@test.com", "message test@test.com", "jpg", "image/jpeg", 0, 0)
+	require.NoError(t, err)
+
+	defer th.deleteUserFileInfos("bookmark")
+
+	params := &model.SearchParams{
+		InChannels: []string{th.ChannelBasic.Id},
+	}
+	results, err := th.Store.FileInfo().Search(th.Context, []*model.SearchParams{params}, th.User.Id, th.Team.Id, 0, 20)
+	require.NoError(t, err)
+
+	require.Len(t, results.FileInfos, 1)
+	require.Equal(t, "message test@test.com", results.FileInfos[file.Id].Name)
+}
diff --git a/server/channels/store/sqlstore/file_info_store.go b/server/channels/store/sqlstore/file_info_store.go
index a54c784e18..ad4d8933c3 100644
--- a/server/channels/store/sqlstore/file_info_store.go
+++ b/server/channels/store/sqlstore/file_info_store.go
@@ -514,6 +514,10 @@ func (fs SqlFileInfoStore) Search(rctx request.CTX, paramsList []*model.SearchPa
 		LeftJoin("ChannelMembers as CM ON C.Id=CM.ChannelId").
 		Where(sq.Or{sq.Eq{"C.TeamId": teamId}, sq.Eq{"C.TeamId": ""}}).
 		Where(sq.Eq{"FileInfo.DeleteAt": 0}).
+		Where(sq.Or{
+			sq.Eq{"FileInfo.CreatorId": model.BookmarkFileOwner},
+			sq.NotEq{"FileInfo.PostId": ""},
+		}).
 		OrderBy("FileInfo.CreateAt DESC").
 		Limit(100)
 
diff --git a/server/platform/services/searchengine/bleveengine/indexer/indexing_job.go b/server/platform/services/searchengine/bleveengine/indexer/indexing_job.go
index ca70652ea2..890e394100 100644
--- a/server/platform/services/searchengine/bleveengine/indexer/indexing_job.go
+++ b/server/platform/services/searchengine/bleveengine/indexer/indexing_job.go
@@ -472,7 +472,7 @@ func (worker *BleveIndexerWorker) BulkIndexFiles(files []*model.FileForIndexing,
 	batch := worker.engine.FileIndex.NewBatch()
 
 	for _, file := range files {
-		if file.DeleteAt == 0 {
+		if file.ShouldIndex() {
 			searchFile := bleveengine.BLVFileFromFileForIndexing(file)
 			batch.Index(searchFile.Id, searchFile)
 		} else {
diff --git a/server/public/model/post.go b/server/public/model/post.go
index 9f3a2a0063..21f4eb8308 100644
--- a/server/public/model/post.go
+++ b/server/public/model/post.go
@@ -260,6 +260,22 @@ type FileForIndexing struct {
 	Content   string `json:"content"`
 }
 
+// ShouldIndex tells if a file should be indexed or not.
+// index files which are-
+// a. not deleted
+// b. have an associated post ID, if no post ID, then,
+// b.i. the file should belong to the channel's bookmarks, as indicated by the "CreatorId" field.
+//
+// Files not passing this criteria will be deleted from ES index.
+// We're deleting those files from ES index instead of simply skipping them while fetching a batch of files
+// because existing ES indexes might have these files already indexed, so we need to remove them from index.
+func (file *FileForIndexing) ShouldIndex() bool {
+	// NOTE - this function is used in server as well as Enterprise code.
+	// Make sure to update public package dependency in both server and Enterprise code when
+	// updating the logic here and to test both places.
+	return file != nil && file.DeleteAt == 0 && (file.PostId != "" || file.CreatorId == BookmarkFileOwner)
+}
+
 // ShallowCopy is an utility function to shallow copy a Post to the given
 // destination without touching the internal RWMutex.
 func (o *Post) ShallowCopy(dst *Post) error {