Batch Edit: Improve backend unit test coverage and code docs #271 #5324

Signed-off-by: Michael Mayer <michael@photoprism.app>

internal/entity/photo.go

@@ -166,6 +166,10 @@ func SavePhotoForm(m *Photo, form form.Photo) error {
 	}
 
 	// Update time fields.
+	// Batch edit (and other callers) treat TakenAtLocal as a naive timestamp that already includes
+	// any new Day/Month/Year. Here we normalize it back to UTC using the photo's TimeZone so MariaDB
+	// (which lacks TZ support) stores a consistent pair of TakenAt / TakenAtLocal values. See
+	// ComputeDateChange for details on why it returns UTC.
 	if m.TimeZoneUTC() {
 		m.TakenAtLocal = m.TakenAt
 	} else {

internal/entity/photo_test.go

@@ -74,6 +74,49 @@ func TestSavePhotoForm(t *testing.T) {
 
 		t.Log(m.GetDetails().Keywords)
 	})
+	t.Run("BatchDateChangeKeepsTimeZone", func(t *testing.T) {
+		photo := PhotoFixtures.Get("Photo09")
+		require.Equal(t, "America/Mexico_City", photo.TimeZone)
+
+		formSnapshot, err := form.NewPhoto(&photo)
+		require.NoError(t, err)
+
+		newDay := photo.PhotoDay + 1
+		formSnapshot.PhotoDay = newDay
+		formSnapshot.TakenSrc = SrcBatch
+		formSnapshot.TimeZone = photo.TimeZone
+		formSnapshot.TakenAtLocal = time.Date(
+			photo.PhotoYear,
+			time.Month(photo.PhotoMonth),
+			newDay,
+			photo.TakenAtLocal.Hour(),
+			photo.TakenAtLocal.Minute(),
+			photo.TakenAtLocal.Second(),
+			0,
+			time.UTC,
+		)
+
+		require.NoError(t, SavePhotoForm(&photo, formSnapshot))
+		require.NoError(t, Db().First(&photo, photo.ID).Error)
+
+		location := tz.Find(photo.TimeZone)
+		require.NotNil(t, location)
+		expectedUTC := time.Date(
+			photo.PhotoYear,
+			time.Month(photo.PhotoMonth),
+			newDay,
+			photo.TakenAtLocal.Hour(),
+			photo.TakenAtLocal.Minute(),
+			photo.TakenAtLocal.Second(),
+			0,
+			location,
+		).UTC()
+
+		assert.Equal(t, newDay, photo.PhotoDay)
+		assert.Equal(t, "America/Mexico_City", photo.TimeZone)
+		assert.Equal(t, SrcBatch, photo.TakenSrc)
+		assert.True(t, photo.TakenAt.Equal(expectedUTC))
+	})
 }
 
 func TestPhoto_LabelKeywords(t *testing.T) {

internal/photoprism/batch/actions_test.go

@@ -45,7 +45,6 @@ func TestApplyAlbums(t *testing.T) {
 			t.Fatal(err)
 		}
 	})
-
 	t.Run("AddPhotoToNewAlbumByTitle", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Get("Photo02")
 		albumTitle := "Test Album for Actions"
@@ -75,7 +74,6 @@ func TestApplyAlbums(t *testing.T) {
 			t.Fatal(err)
 		}
 	})
-
 	t.Run("RemovePhotoFromAlbum", func(t *testing.T) {
 		// First add photo to album
 		photo := entity.PhotoFixtures.Get("Photo03")
@@ -128,7 +126,6 @@ func TestApplyAlbums(t *testing.T) {
 			t.Error("expected photo to be marked as hidden in album")
 		}
 	})
-
 	// Error cases
 	t.Run("AddPhotoToNonExistingAlbumByUID", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Get("Photo04")
@@ -145,7 +142,6 @@ func TestApplyAlbums(t *testing.T) {
 			t.Error("expected error when adding photo to non-existing album, but got none")
 		}
 	})
-
 	t.Run("AddPhotoToAlbumWithInvalidUID", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Get("Photo04")
 		invalidUID := "invalid-uid-format" // Invalid UID format
@@ -161,7 +157,6 @@ func TestApplyAlbums(t *testing.T) {
 			t.Error("expected error when adding photo to album with invalid UID, but got none")
 		}
 	})
-
 	t.Run("RemovePhotoFromNonExistingAlbum", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Get("Photo05")
 		nonExistingAlbumUID := "at9lxuqxpobbbbbb" // Non-existing UID
@@ -177,7 +172,6 @@ func TestApplyAlbums(t *testing.T) {
 			t.Error("expected error when removing photo from non-existing album, but got none")
 		}
 	})
-
 	t.Run("InvalidActionOnAlbum", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Get("Photo06")
 		albumUID := entity.AlbumFixtures.Get("christmas2030").AlbumUID
@@ -193,7 +187,6 @@ func TestApplyAlbums(t *testing.T) {
 			t.Error("expected error for invalid action, but got none")
 		}
 	})
-
 	t.Run("EmptyAlbumItems", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Get("Photo07")
 
@@ -207,7 +200,6 @@ func TestApplyAlbums(t *testing.T) {
 			t.Errorf("expected no error for empty album items, but got: %v", err)
 		}
 	})
-
 	t.Run("AddPhotoToAlbumWithEmptyValueAndTitle", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Get("Photo08")
 
@@ -222,7 +214,6 @@ func TestApplyAlbums(t *testing.T) {
 			t.Error("expected error when both Value and Title are empty, but got none")
 		}
 	})
-
 	t.Run("InvalidPhotoUID", func(t *testing.T) {
 		invalidPhotoUID := "invalid-photo-uid"
 		albumUID := entity.AlbumFixtures.Get("christmas2030").AlbumUID
@@ -269,7 +260,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Errorf("expected label source %s, got %s", entity.SrcBatch, photoLabel.LabelSrc)
 		}
 	})
-
 	t.Run("AddNewLabelByTitle", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo07")
 		labelTitle := "Test Label for Actions"
@@ -307,7 +297,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Errorf("expected label source %s, got %s", entity.SrcBatch, photoLabel.LabelSrc)
 		}
 	})
-
 	t.Run("RemoveLabelByUID", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo08")
 		label := entity.LabelFixtures.Get("flower")
@@ -342,7 +331,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Error("expected label to be deleted, but it was found")
 		}
 	})
-
 	t.Run("RemoveAutoLabelSetsUncertaintyTo100", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo09")
 		label := entity.LabelFixtures.Get("cake")
@@ -377,7 +365,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Errorf("expected label source %s, got %s", entity.SrcBatch, blockedLabel.LabelSrc)
 		}
 	})
-
 	t.Run("UpdateExistingLabelConfidence", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo10")
 		label := entity.LabelFixtures.Get("landscape")
@@ -425,7 +412,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Errorf("expected label source %s, got %s", entity.SrcBatch, updatedLabel.LabelSrc)
 		}
 	})
-
 	t.Run("InvalidPhotoReturnsError", func(t *testing.T) {
 		labels := Items{
 			Items: []Item{
@@ -444,7 +430,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Error("expected error for empty photo")
 		}
 	})
-
 	// Additional error cases
 	t.Run("AddNonExistingLabelByUID", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo11")
@@ -461,7 +446,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Error("expected error when adding non-existing label by UID, but got none")
 		}
 	})
-
 	t.Run("AddLabelWithInvalidUID", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo12")
 		invalidUID := "invalid-label-uid-format" // Invalid UID format
@@ -477,7 +461,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Error("expected error when adding label with invalid UID, but got none")
 		}
 	})
-
 	t.Run("RemoveNonExistingLabelByUID", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo13")
 		nonExistingLabelUID := "lt9lxuqxpobbbbbb" // Non-existing UID
@@ -493,7 +476,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Error("expected error when removing non-existing label, but got none")
 		}
 	})
-
 	t.Run("InvalidActionOnLabel", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo14")
 		labelUID := entity.LabelFixtures.Get("landscape").LabelUID
@@ -509,7 +491,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Error("expected error for invalid action, but got none")
 		}
 	})
-
 	t.Run("EmptyLabelItems", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo15")
 
@@ -523,7 +504,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Errorf("expected no error for empty label items, but got: %v", err)
 		}
 	})
-
 	t.Run("AddLabelWithEmptyValueAndTitle", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo16")
 
@@ -538,7 +518,6 @@ func TestApplyLabels(t *testing.T) {
 			t.Error("expected error when both Value and Title are empty, but got none")
 		}
 	})
-
 	t.Run("RemoveLabelNotAssignedToPhoto", func(t *testing.T) {
 		photo := entity.PhotoFixtures.Pointer("Photo17")
 		labelUID := entity.LabelFixtures.Get("bird").LabelUID

internal/photoprism/batch/convert_test.go

@@ -14,7 +14,6 @@ func TestConvertToPhotoForm(t *testing.T) {
 		assert.Nil(t, form)
 		assert.Error(t, err)
 	})
-
 	t.Run("UpdateTitleCaptionTypeAndBooleans", func(t *testing.T) {
 		photo := &entity.Photo{
 			PhotoTitle:   "Old Title",
@@ -61,7 +60,6 @@ func TestConvertToPhotoForm(t *testing.T) {
 		assert.True(t, form.PhotoScan)
 		assert.True(t, form.PhotoPanorama)
 	})
-
 	t.Run("UpdateLocationSetsPlaceSrc", func(t *testing.T) {
 		photo := &entity.Photo{}
 
@@ -84,7 +82,6 @@ func TestConvertToPhotoForm(t *testing.T) {
 		assert.Equal(t, 15, form.PhotoAltitude)
 		assert.Equal(t, entity.SrcBatch, form.PlaceSrc)
 	})
-
 	t.Run("TitleRemove_CaptionUpdate", func(t *testing.T) {
 		photo := &entity.Photo{PhotoTitle: "Old", PhotoCaption: "OldCap"}
 
@@ -100,7 +97,6 @@ func TestConvertToPhotoForm(t *testing.T) {
 		assert.Equal(t, "NewCap", form.PhotoCaption)
 		assert.Equal(t, entity.SrcBatch, form.CaptionSrc)
 	})
-
 	t.Run("TimeZoneUpdateSetsTakenSrc", func(t *testing.T) {
 		photo := &entity.Photo{}
 
@@ -113,7 +109,6 @@ func TestConvertToPhotoForm(t *testing.T) {
 		assert.Equal(t, "Europe/Berlin", form.TimeZone)
 		assert.Equal(t, entity.SrcBatch, form.TakenSrc)
 	})
-
 	t.Run("YearUpdate_RecomputesTakenAtLocalAndOutputs", func(t *testing.T) {
 		photo := &entity.Photo{}
 		// Assume current date fields are set via NewPhoto(photo)
@@ -130,7 +125,6 @@ func TestConvertToPhotoForm(t *testing.T) {
 		// PhotoDay is set via ComputeDateChange; must be non-zero or -1 depending on base
 		assert.NotZero(t, form.PhotoDay)
 	})
-
 	t.Run("UpdateDetails", func(t *testing.T) {
 		photo := &entity.Photo{}
 
@@ -156,7 +150,6 @@ func TestConvertToPhotoForm(t *testing.T) {
 		assert.Equal(t, "License", form.Details.License)
 		assert.Equal(t, entity.SrcBatch, form.Details.LicenseSrc)
 	})
-
 	t.Run("RemoveDetailsFieldsAndCarryPhotoID", func(t *testing.T) {
 		photo := &entity.Photo{ID: 42}
 

internal/photoprism/batch/datelogic.go

@@ -3,8 +3,9 @@ package batch
 import "time"
 
 // ComputeDateChange calculates a new date and output values based on the provided actions and values.
-// It takes a base time and current date components, along with actions and values for day, month, and year updates.
-// Returns the computed new local time and the output year, month, and day values.
+// The returned time is deliberately constructed in UTC: we store TakenAtLocal as a naive timestamp and
+// later derive the real UTC instant via Photo.GetTakenAt() using the photo's TimeZone. See SavePhotoForm
+// for details on how TakenAt / TakenAtLocal stay consistent in the database.
 func ComputeDateChange(
 	baseLocal time.Time,
 	curYear, curMonth, curDay int,

internal/photoprism/batch/datelogic_test.go

@@ -20,7 +20,6 @@ func TestComputeDateChange(t *testing.T) {
 		assert.Equal(t, time.February, newLocal.Month())
 		assert.Equal(t, 28, newLocal.Day())
 	})
-
 	t.Run("DayUnknown_YearNotForcedUnknown", func(t *testing.T) {
 		// Setting Day=-1 should NOT force Year=-1
 		base := time.Date(2023, 11, 12, 9, 0, 0, 0, time.UTC)
@@ -31,7 +30,6 @@ func TestComputeDateChange(t *testing.T) {
 		assert.Equal(t, -1, d)
 		assert.Equal(t, 1, newLocal.Day()) // day used for TakenAtLocal when unknown
 	})
-
 	t.Run("MonthUnknown_KeepsYearValue", func(t *testing.T) {
 		// Setting Month=-1 should NOT force Year=-1
 		base := time.Date(2020, 4, 30, 8, 0, 0, 0, time.UTC)
@@ -43,7 +41,6 @@ func TestComputeDateChange(t *testing.T) {
 		assert.Equal(t, 30, newLocal.Day())
 		assert.Equal(t, time.April, newLocal.Month())
 	})
-
 	t.Run("MixedMonth_UpdateDayClampsPerPhoto", func(t *testing.T) {
 		// Day 31 → March (31 days) should work
 		baseMar := time.Date(2020, 3, 20, 11, 29, 54, 0, time.UTC)
@@ -61,7 +58,6 @@ func TestComputeDateChange(t *testing.T) {
 		assert.Equal(t, 30, d2)
 		assert.Equal(t, 30, newLocal2.Day())
 	})
-
 	t.Run("UnknownCurrentMonth_DayUpdateClampsPerPhoto", func(t *testing.T) {
 		// Base in March, current month unknown, update Day=31
 		baseMar := time.Date(2020, 3, 20, 11, 29, 54, 0, time.UTC)
@@ -81,7 +77,6 @@ func TestComputeDateChange(t *testing.T) {
 		assert.Equal(t, time.April, newLocal2.Month())
 		assert.Equal(t, 30, newLocal2.Day())
 	})
-
 	t.Run("MixedDay_UpdateMonthToFeb2020", func(t *testing.T) {
 		// Photo 1: Day 20 → Feb works fine
 		base1 := time.Date(2020, 4, 20, 11, 29, 54, 0, time.UTC)
@@ -101,7 +96,6 @@ func TestComputeDateChange(t *testing.T) {
 		assert.Equal(t, time.February, newLocal2.Month())
 		assert.Equal(t, 29, newLocal2.Day())
 	})
-
 	t.Run("Mixed_UpdateYearTo2021", func(t *testing.T) {
 		// Photo 1: Feb 29, 2020 → 2021 should clamp to Feb 28, 2021
 		base1 := time.Date(2020, 2, 29, 11, 29, 54, 0, time.UTC)
@@ -121,7 +115,6 @@ func TestComputeDateChange(t *testing.T) {
 		assert.Equal(t, 2021, newLocal2.Year())
 		assert.Equal(t, 31, newLocal2.Day())
 	})
-
 	t.Run("AllUnknown_KeepBaseYearMonth_SetDayOne", func(t *testing.T) {
 		// Current values: 2024-04-30 13:29:54 (local)
 		base := time.Date(2024, 4, 30, 13, 29, 54, 0, time.UTC)
@@ -148,4 +141,24 @@ func TestComputeDateChange(t *testing.T) {
 		assert.Equal(t, 29, newLocal.Minute())
 		assert.Equal(t, 54, newLocal.Second())
 	})
+	t.Run("PreservesClockForNonUTCBase", func(t *testing.T) {
+		loc := time.FixedZone("UTC-7", -7*3600)
+		base := time.Date(2023, 6, 15, 22, 45, 30, 0, loc)
+		newLocal, y, m, d := ComputeDateChange(
+			base,
+			2023, 6, 15,
+			ActionUpdate, 18,
+			ActionNone, 0,
+			ActionNone, 0,
+		)
+
+		assert.Equal(t, 2023, y)
+		assert.Equal(t, 6, m)
+		assert.Equal(t, 18, d)
+		// Returned time is UTC but keeps the local clock so SavePhotoForm can reapply the zone.
+		assert.Equal(t, time.UTC, newLocal.Location())
+		assert.Equal(t, base.Hour(), newLocal.Hour())
+		assert.Equal(t, base.Minute(), newLocal.Minute())
+		assert.Equal(t, base.Second(), newLocal.Second())
+	})
 }