Fix the incorrect thumbnails for albums issue (#1198)

* The fix with test

* Fix test's foreign key issue

* 2 more test cases

* Fix the empty album test expectation

* Return Nil for no thumbnail case instead of an empty album, as expected by frontend. More tests for thumbnail priority choice

* Fix failed tests

---------

Co-authored-by: Konstantin Koval

api/graphql/models/album.go

@@ -98,7 +98,6 @@ func (a *Album) Thumbnail(db *gorm.DB) (*Media, error) {
 			INNER JOIN sub_albums ON children.parent_album_id = sub_albums.id
 		)
 		SELECT * FROM media
-		INNER JOIN media_urls ON media_urls.media_id = media.id
 		WHERE media.album_id IN (SELECT id FROM sub_albums)
 		LIMIT 1
 	`
@@ -107,5 +106,9 @@ func (a *Album) Thumbnail(db *gorm.DB) (*Media, error) {
 		return nil, err
 	}
 
+	if media.ID == 0 {
+		return nil, nil // Return nil for empty albums
+	}
+
 	return &media, nil
 }

api/graphql/models/album_test.go

@@ -1,7 +1,9 @@
 package models_test
 
 import (
+	"fmt"
 	"testing"
+	"time"
 
 	"github.com/photoview/photoview/api/graphql/models"
 	"github.com/photoview/photoview/api/test_utils"
@@ -124,3 +126,244 @@ func TestAlbumGetChildrenAndParents(t *testing.T) {
 	})
 
 }
+
+func TestAlbumThumbnail(t *testing.T) {
+	db := test_utils.DatabaseTest(t)
+
+	mediaAlbum := models.Album{
+		Title: "Media album",
+		Path:  "/media_album",
+	}
+	if !assert.NoError(t, db.Save(&mediaAlbum).Error) {
+		return
+	}
+
+	media := models.Media{
+		Path:    "thumb.jpg",
+		AlbumID: mediaAlbum.ID,
+	}
+	if !assert.NoError(t, db.Save(&media).Error) {
+		return
+	}
+
+	t.Run("Thumbnail from CoverID", func(t *testing.T) {
+		album := models.Album{
+			Title:   "Album with cover",
+			Path:    "/cover_album",
+			CoverID: &media.ID,
+		}
+		if !assert.NoError(t, db.Save(&album).Error) {
+			return
+		}
+
+		result, err := album.Thumbnail(db)
+		assert.NoError(t, err)
+		assert.NotNil(t, result)
+		assert.Equal(t, media.ID, result.ID)
+	})
+
+	t.Run("Thumbnail from child media", func(t *testing.T) {
+		parentAlbum := models.Album{
+			Title: "Parent album",
+			Path:  "/parent",
+		}
+		if !assert.NoError(t, db.Save(&parentAlbum).Error) {
+			return
+		}
+
+		childAlbum := models.Album{
+			Title:         "Child album",
+			Path:          "/parent/child",
+			ParentAlbumID: &parentAlbum.ID,
+		}
+		if !assert.NoError(t, db.Save(&childAlbum).Error) {
+			return
+		}
+
+		childMedia := models.Media{
+			Path:    "child_media.jpg",
+			AlbumID: childAlbum.ID,
+		}
+		if !assert.NoError(t, db.Save(&childMedia).Error) {
+			return
+		}
+
+		result, err := parentAlbum.Thumbnail(db)
+		assert.NoError(t, err)
+		assert.NotNil(t, result)
+		assert.Equal(t, childMedia.ID, result.ID)
+	})
+
+	t.Run("Empty album with no media", func(t *testing.T) {
+		emptyAlbum := models.Album{
+			Title: "Empty album",
+			Path:  "/empty",
+		}
+		if !assert.NoError(t, db.Save(&emptyAlbum).Error) {
+			return
+		}
+
+		result, err := emptyAlbum.Thumbnail(db)
+		assert.NoError(t, err)
+		assert.Nil(t, result, "Empty albums should have nil thumbnail")
+	})
+
+	t.Run("Thumbnail from grandchild media", func(t *testing.T) {
+		// Create grandparent-parent-child relationship with media only in child
+		grandparentAlbum := models.Album{
+			Title: "Grandparent",
+			Path:  "/grandparent",
+		}
+		if !assert.NoError(t, db.Save(&grandparentAlbum).Error) {
+			return
+		}
+
+		parentAlbum := models.Album{
+			Title:         "Parent",
+			Path:          "/grandparent/parent",
+			ParentAlbumID: &grandparentAlbum.ID,
+		}
+		if !assert.NoError(t, db.Save(&parentAlbum).Error) {
+			return
+		}
+
+		childAlbum := models.Album{
+			Title:         "Child",
+			Path:          "/grandparent/parent/child",
+			ParentAlbumID: &parentAlbum.ID,
+		}
+		if !assert.NoError(t, db.Save(&childAlbum).Error) {
+			return
+		}
+
+		childMedia := models.Media{
+			Path:    "deep_media.jpg",
+			AlbumID: childAlbum.ID,
+		}
+		if !assert.NoError(t, db.Save(&childMedia).Error) {
+			return
+		}
+
+		result, err := grandparentAlbum.Thumbnail(db)
+		assert.NoError(t, err)
+		assert.NotNil(t, result)
+		assert.Equal(t, childMedia.ID, result.ID)
+	})
+
+	t.Run("CoverID takes precedence over any media", func(t *testing.T) {
+		// Create album with both direct media and a cover ID
+		priorityAlbum := models.Album{
+			Title:   "Priority album",
+			Path:    "/priority",
+			CoverID: &media.ID, // Using existing media as cover
+		}
+		if !assert.NoError(t, db.Save(&priorityAlbum).Error) {
+			return
+		}
+
+		// Add direct media to the album with unique path
+		directMedia := models.Media{
+			Path:    fmt.Sprintf("direct_media_%d.jpg", time.Now().UnixNano()),
+			AlbumID: priorityAlbum.ID,
+		}
+		if !assert.NoError(t, db.Save(&directMedia).Error) {
+			return
+		}
+
+		// Test that CoverID takes precedence
+		result, err := priorityAlbum.Thumbnail(db)
+		assert.NoError(t, err)
+		assert.Equal(t, media.ID, result.ID, "CoverID should take precedence over direct media")
+	})
+
+	t.Run("Some media is returned when multiple exist in hierarchy", func(t *testing.T) {
+		// Create a parent album
+		parentAlbum := models.Album{
+			Title: "Parent album",
+			Path:  "/parent_media_test",
+		}
+		if !assert.NoError(t, db.Save(&parentAlbum).Error) {
+			return
+		}
+
+		// Add direct media to parent with unique path
+		parentMedia := models.Media{
+			Path:    fmt.Sprintf("parent_media_%d.jpg", time.Now().UnixNano()),
+			AlbumID: parentAlbum.ID,
+		}
+		if !assert.NoError(t, db.Save(&parentMedia).Error) {
+			return
+		}
+
+		// Create child album with media
+		childAlbum := models.Album{
+			Title:         "Child album",
+			Path:          "/parent_media_test/child",
+			ParentAlbumID: &parentAlbum.ID,
+		}
+		if !assert.NoError(t, db.Save(&childAlbum).Error) {
+			return
+		}
+
+		// Add child media with unique path
+		childMedia := models.Media{
+			Path:    fmt.Sprintf("child_media_%d.jpg", time.Now().UnixNano()),
+			AlbumID: childAlbum.ID,
+		}
+		if !assert.NoError(t, db.Save(&childMedia).Error) {
+			return
+		}
+
+		// Test that some media is returned
+		result, err := parentAlbum.Thumbnail(db)
+		assert.NoError(t, err)
+		assert.NotNil(t, result)
+		assert.True(t, result.ID == parentMedia.ID || result.ID == childMedia.ID,
+			"Should return either direct media or child album media")
+		t.Logf("For reference - Selected: %d, Parent media: %d, Child media: %d",
+			result.ID, parentMedia.ID, childMedia.ID)
+	})
+
+	t.Run("Database order determines which media is selected", func(t *testing.T) {
+		// Create album with multiple media
+		multiMediaAlbum := models.Album{
+			Title: "Album with multiple media",
+			Path:  "/multi_media",
+		}
+		if !assert.NoError(t, db.Save(&multiMediaAlbum).Error) {
+			return
+		}
+
+		// Add multiple media to the album with unique paths
+		mediaItems := []models.Media{
+			{Path: fmt.Sprintf("media1_%d.jpg", time.Now().UnixNano()), AlbumID: multiMediaAlbum.ID},
+			// Sleep briefly to ensure different timestamps
+			{Path: fmt.Sprintf("media2_%d.jpg", time.Now().UnixNano()+1), AlbumID: multiMediaAlbum.ID},
+			{Path: fmt.Sprintf("media3_%d.jpg", time.Now().UnixNano()+2), AlbumID: multiMediaAlbum.ID},
+		}
+		if !assert.NoError(t, db.Save(&mediaItems).Error) {
+			return
+		}
+
+		// Test which media is selected
+		result, err := multiMediaAlbum.Thumbnail(db)
+		assert.NoError(t, err)
+		assert.NotNil(t, result)
+
+		// Log which item was selected for documentation purposes
+		t.Logf("Selected media ID: %d", result.ID)
+		for i, item := range mediaItems {
+			t.Logf("Media %d: ID %d, Path %s", i+1, item.ID, item.Path)
+		}
+
+		// Verify one of our media items was selected
+		found := false
+		for _, item := range mediaItems {
+			if result.ID == item.ID {
+				found = true
+				break
+			}
+		}
+		assert.True(t, found, "One of the album's media should be selected")
+	})
+}