fix: dateTimeOriginal timezone updates

* fix: merged video in On This Device played with incorrect dimension

* chore: pr feedback

mobile/lib/domain/services/asset.service.dart

@@ -6,6 +6,8 @@ import 'package:immich_mobile/infrastructure/repositories/local_asset.repository
 import 'package:immich_mobile/infrastructure/repositories/remote_asset.repository.dart';
 import 'package:immich_mobile/infrastructure/utils/exif.converter.dart';
 
+typedef _AssetVideoDimension = ({double? width, double? height, bool isFlipped});
+
 class AssetService {
   final RemoteAssetRepository _remoteAssetRepository;
   final DriftLocalAssetRepository _localAssetRepository;
@@ -58,44 +60,48 @@ class AssetService {
   }
 
   Future<double> getAspectRatio(BaseAsset asset) async {
-    bool isFlipped;
-    double? width;
-    double? height;
+    final dimension = asset is LocalAsset
+        ? await _getLocalAssetDimensions(asset)
+        : await _getRemoteAssetDimensions(asset as RemoteAsset);
 
-    if (asset.hasRemote) {
-      final exif = await getExif(asset);
-      isFlipped = ExifDtoConverter.isOrientationFlipped(exif?.orientation);
-      width = asset.width?.toDouble();
-      height = asset.height?.toDouble();
-    } else if (asset is LocalAsset) {
-      isFlipped = CurrentPlatform.isAndroid && (asset.orientation == 90 || asset.orientation == 270);
-      width = asset.width?.toDouble();
-      height = asset.height?.toDouble();
-    } else {
-      isFlipped = false;
+    if (dimension.width == null || dimension.height == null || dimension.height == 0) {
+      return 1.0;
     }
 
+    return dimension.isFlipped ? dimension.height! / dimension.width! : dimension.width! / dimension.height!;
+  }
+
+  Future<_AssetVideoDimension> _getLocalAssetDimensions(LocalAsset asset) async {
+    double? width = asset.width?.toDouble();
+    double? height = asset.height?.toDouble();
+    int orientation = asset.orientation;
+
     if (width == null || height == null) {
-      if (asset.hasRemote) {
-        final id = asset is LocalAsset ? asset.remoteId! : (asset as RemoteAsset).id;
-        final remoteAsset = await _remoteAssetRepository.get(id);
-        width = remoteAsset?.width?.toDouble();
-        height = remoteAsset?.height?.toDouble();
-      } else {
-        final id = asset is LocalAsset ? asset.id : (asset as RemoteAsset).localId!;
-        final localAsset = await _localAssetRepository.get(id);
-        width = localAsset?.width?.toDouble();
-        height = localAsset?.height?.toDouble();
-      }
+      final fetched = await _localAssetRepository.get(asset.id);
+      width = fetched?.width?.toDouble();
+      height = fetched?.height?.toDouble();
+      orientation = fetched?.orientation ?? 0;
     }
 
-    final orientedWidth = isFlipped ? height : width;
-    final orientedHeight = isFlipped ? width : height;
-    if (orientedWidth != null && orientedHeight != null && orientedHeight > 0) {
-      return orientedWidth / orientedHeight;
+    // On Android, local assets need orientation correction for 90°/270° rotations
+    // On iOS, the Photos framework pre-corrects dimensions
+    final isFlipped = CurrentPlatform.isAndroid && (orientation == 90 || orientation == 270);
+    return (width: width, height: height, isFlipped: isFlipped);
+  }
+
+  Future<_AssetVideoDimension> _getRemoteAssetDimensions(RemoteAsset asset) async {
+    double? width = asset.width?.toDouble();
+    double? height = asset.height?.toDouble();
+
+    if (width == null || height == null) {
+      final fetched = await _remoteAssetRepository.get(asset.id);
+      width = fetched?.width?.toDouble();
+      height = fetched?.height?.toDouble();
     }
 
-    return 1.0;
+    final exif = await getExif(asset);
+    final isFlipped = ExifDtoConverter.isOrientationFlipped(exif?.orientation);
+    return (width: width, height: height, isFlipped: isFlipped);
   }
 
   Future<List<(String, String)>> getPlaces(String userId) {

mobile/test/domain/services/asset.service_test.dart

@@ -87,6 +87,25 @@ void main() {
       verify(() => mockLocalAssetRepository.get('local-1')).called(1);
     });
 
+    test('uses fetched asset orientation when dimensions are missing on Android', () async {
+      debugDefaultTargetPlatformOverride = TargetPlatform.android;
+      addTearDown(() => debugDefaultTargetPlatformOverride = null);
+
+      // Original asset has default orientation 0, but dimensions are missing
+      final localAsset = TestUtils.createLocalAsset(id: 'local-1', width: null, height: null, orientation: 0);
+
+      // Fetched asset has 90° orientation and proper dimensions
+      final fetchedAsset = TestUtils.createLocalAsset(id: 'local-1', width: 1920, height: 1080, orientation: 90);
+
+      when(() => mockLocalAssetRepository.get('local-1')).thenAnswer((_) async => fetchedAsset);
+
+      final result = await sut.getAspectRatio(localAsset);
+
+      // Should flip dimensions since fetched asset has 90° orientation
+      expect(result, 1080 / 1920);
+      verify(() => mockLocalAssetRepository.get('local-1')).called(1);
+    });
+
     test('returns 1.0 when dimensions are still unavailable after fetching', () async {
       final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-1', width: null, height: null);
 
@@ -112,7 +131,9 @@ void main() {
       expect(result, 1.0);
     });
 
-    test('handles local asset with remoteId and uses exif from remote', () async {
+    test('handles local asset with remoteId using local orientation not remote exif', () async {
+      // When a LocalAsset has a remoteId (merged), we should use local orientation
+      // because the width/height come from the local asset (pre-corrected on iOS)
       final localAsset = TestUtils.createLocalAsset(
         id: 'local-1',
         remoteId: 'remote-1',
@@ -121,9 +142,24 @@ void main() {
         orientation: 0,
       );
 
-      final exif = const ExifInfo(orientation: '6');
+      final result = await sut.getAspectRatio(localAsset);
 
-      when(() => mockRemoteAssetRepository.getExif('remote-1')).thenAnswer((_) async => exif);
+      expect(result, 1920 / 1080);
+      // Should not call remote exif for LocalAsset
+      verifyNever(() => mockRemoteAssetRepository.getExif(any()));
+    });
+
+    test('handles local asset with remoteId and 90 degree rotation on Android', () async {
+      debugDefaultTargetPlatformOverride = TargetPlatform.android;
+      addTearDown(() => debugDefaultTargetPlatformOverride = null);
+
+      final localAsset = TestUtils.createLocalAsset(
+        id: 'local-1',
+        remoteId: 'remote-1',
+        width: 1920,
+        height: 1080,
+        orientation: 90,
+      );
 
       final result = await sut.getAspectRatio(localAsset);
 

server/src/services/asset.service.ts

@@ -144,14 +144,28 @@ export class AssetService extends BaseService {
     await this.requireAccess({ auth, permission: Permission.AssetUpdate, ids });
 
     const assetDto = _.omitBy({ isFavorite, visibility, duplicateId }, _.isUndefined);
-    const exifDto = _.omitBy({ latitude, longitude, rating, description, dateTimeOriginal }, _.isUndefined);
+    const exifDto = _.omitBy(
+      {
+        latitude,
+        longitude,
+        rating,
+        description,
+        dateTimeOriginal,
+      },
+      _.isUndefined,
+    );
+    const extractedTimeZone = dateTimeOriginal ? DateTime.fromISO(dateTimeOriginal, { setZone: true }).zone : undefined;
 
     if (Object.keys(exifDto).length > 0) {
       await this.assetRepository.updateAllExif(ids, exifDto);
     }
 
-    if ((dateTimeRelative !== undefined && dateTimeRelative !== 0) || timeZone !== undefined) {
-      await this.assetRepository.updateDateTimeOriginal(ids, dateTimeRelative, timeZone);
+    if (
+      (dateTimeRelative !== undefined && dateTimeRelative !== 0) ||
+      timeZone !== undefined ||
+      extractedTimeZone?.type === 'fixed'
+    ) {
+      await this.assetRepository.updateDateTimeOriginal(ids, dateTimeRelative, timeZone ?? extractedTimeZone?.name);
     }
 
     if (Object.keys(assetDto).length > 0) {
@@ -436,7 +450,19 @@ export class AssetService extends BaseService {
     rating?: number;
   }) {
     const { id, description, dateTimeOriginal, latitude, longitude, rating } = dto;
-    const writes = _.omitBy({ description, dateTimeOriginal, latitude, longitude, rating }, _.isUndefined);
+    const extractedTimeZone = dateTimeOriginal ? DateTime.fromISO(dateTimeOriginal, { setZone: true }).zone : undefined;
+    const writes = _.omitBy(
+      {
+        description,
+        dateTimeOriginal,
+        timeZone: extractedTimeZone?.type === 'fixed' ? extractedTimeZone.name : undefined,
+        latitude,
+        longitude,
+        rating,
+      },
+      _.isUndefined,
+    );
+
     if (Object.keys(writes).length > 0) {
       await this.assetRepository.upsertExif(
         updateLockedColumns({

server/test/medium/specs/services/asset.service.spec.ts

@@ -285,7 +285,7 @@ describe(AssetService.name, () => {
           .where('assetId', '=', asset.id)
           .executeTakeFirstOrThrow(),
       ).resolves.toEqual({ lockedProperties: null });
-      await sut.update(auth, asset.id, { dateTimeOriginal: '2023-11-19T18:11:00.000-07:00' });
+      await sut.update(auth, asset.id, { dateTimeOriginal: '2023-11-19T18:11:00' });
 
       await expect(
         ctx.database
@@ -296,7 +296,38 @@ describe(AssetService.name, () => {
       ).resolves.toEqual({ lockedProperties: ['dateTimeOriginal'] });
       await expect(ctx.get(AssetRepository).getById(asset.id, { exifInfo: true })).resolves.toEqual(
         expect.objectContaining({
-          exifInfo: expect.objectContaining({ dateTimeOriginal: '2023-11-20T01:11:00+00:00' }),
+          exifInfo: expect.objectContaining({ dateTimeOriginal: '2023-11-19T18:11:00+00:00', timeZone: null }),
+        }),
+      );
+    });
+
+    it('should update dateTimeOriginal with time zone', async () => {
+      const { sut, ctx } = setup();
+      ctx.getMock(JobRepository).queue.mockResolvedValue();
+      const { user } = await ctx.newUser();
+      const auth = factory.auth({ user });
+      const { asset } = await ctx.newAsset({ ownerId: user.id });
+      await ctx.newExif({ assetId: asset.id, description: 'test' });
+
+      await expect(
+        ctx.database
+          .selectFrom('asset_exif')
+          .select('lockedProperties')
+          .where('assetId', '=', asset.id)
+          .executeTakeFirstOrThrow(),
+      ).resolves.toEqual({ lockedProperties: null });
+      await sut.update(auth, asset.id, { dateTimeOriginal: '2023-11-19T18:11:00.000-07:00' });
+
+      await expect(
+        ctx.database
+          .selectFrom('asset_exif')
+          .select('lockedProperties')
+          .where('assetId', '=', asset.id)
+          .executeTakeFirstOrThrow(),
+      ).resolves.toEqual({ lockedProperties: ['timeZone', 'dateTimeOriginal'] });
+      await expect(ctx.get(AssetRepository).getById(asset.id, { exifInfo: true })).resolves.toEqual(
+        expect.objectContaining({
+          exifInfo: expect.objectContaining({ dateTimeOriginal: '2023-11-20T01:11:00+00:00', timeZone: 'UTC-7' }),
         }),
       );
     });
@@ -329,4 +360,65 @@ describe(AssetService.name, () => {
       );
     });
   });
+
+  it('should update dateTimeOriginal', async () => {
+    const { sut, ctx } = setup();
+    ctx.getMock(JobRepository).queueAll.mockResolvedValue();
+    const { user } = await ctx.newUser();
+    const auth = factory.auth({ user });
+    const { asset } = await ctx.newAsset({ ownerId: user.id });
+    await ctx.newExif({ assetId: asset.id, description: 'test' });
+
+    await expect(
+      ctx.database
+        .selectFrom('asset_exif')
+        .select('lockedProperties')
+        .where('assetId', '=', asset.id)
+        .executeTakeFirstOrThrow(),
+    ).resolves.toEqual({ lockedProperties: null });
+    await sut.updateAll(auth, { ids: [asset.id], dateTimeOriginal: '2023-11-19T18:11:00' });
+
+    await expect(
+      ctx.database
+        .selectFrom('asset_exif')
+        .select('lockedProperties')
+        .where('assetId', '=', asset.id)
+        .executeTakeFirstOrThrow(),
+    ).resolves.toEqual({ lockedProperties: ['dateTimeOriginal'] });
+    await expect(ctx.get(AssetRepository).getById(asset.id, { exifInfo: true })).resolves.toEqual(
+      expect.objectContaining({
+        exifInfo: expect.objectContaining({ dateTimeOriginal: '2023-11-19T18:11:00+00:00', timeZone: null }),
+      }),
+    );
+  });
+
+  it('should update dateTimeOriginal with time zone', async () => {
+    const { sut, ctx } = setup();
+    ctx.getMock(JobRepository).queueAll.mockResolvedValue();
+    const { user } = await ctx.newUser();
+    const auth = factory.auth({ user });
+    const { asset } = await ctx.newAsset({ ownerId: user.id });
+    await ctx.newExif({ assetId: asset.id, description: 'test' });
+
+    await expect(
+      ctx.database
+        .selectFrom('asset_exif')
+        .select('lockedProperties')
+        .where('assetId', '=', asset.id)
+        .executeTakeFirstOrThrow(),
+    ).resolves.toEqual({ lockedProperties: null });
+    await sut.updateAll(auth, { ids: [asset.id], dateTimeOriginal: '2023-11-19T18:11:00.000-07:00' });
+    await expect(
+      ctx.database
+        .selectFrom('asset_exif')
+        .select('lockedProperties')
+        .where('assetId', '=', asset.id)
+        .executeTakeFirstOrThrow(),
+    ).resolves.toEqual({ lockedProperties: ['timeZone', 'dateTimeOriginal'] });
+    await expect(ctx.get(AssetRepository).getById(asset.id, { exifInfo: true })).resolves.toEqual(
+      expect.objectContaining({
+        exifInfo: expect.objectContaining({ dateTimeOriginal: '2023-11-20T01:11:00+00:00', timeZone: 'UTC-7' }),
+      }),
+    );
+  });
 });