fix: regression: sort day by fileCreatedAt again (#18732)

* fix: regression: sort day by fileCreatedAt again * lint * e2e test * inline function * e2e * Address comments. Drop dayGroup and timezone in favor of localOffsetMinutes * lint and some api-doc * lint, more api-doc * format * Move minutes to fractional hours * make sql * merge/conflict * merge fallout, review comments * spelling * drop offset from returned date * move description into decorator where possible, regen api
2025-08-09 23:17:29 +02:00 · 2025-06-05 21:56:32 -04:00
parent 81423420c8
commit 55f4e93456
23 changed files with 687 additions and 247 deletions
--- a/server/src/dtos/asset-response.dto.ts
+++ b/server/src/dtos/asset-response.dto.ts
@@ -22,6 +22,13 @@ export class SanitizedAssetResponseDto {
  type!: AssetType;
  thumbhash!: string | null;
  originalMimeType?: string;
+  @ApiProperty({
+    type: 'string',
+    format: 'date-time',
+    description:
+      'The local date and time when the photo/video was taken, derived from EXIF metadata. This represents the photographer\'s local time regardless of timezone, stored as a timezone-agnostic timestamp. Used for timeline grouping by "local" days and months.',
+    example: '2024-01-15T14:30:00.000Z',
+  })
  localDateTime!: Date;
  duration!: string;
  livePhotoVideoId?: string | null;
@@ -37,8 +44,29 @@ export class AssetResponseDto extends SanitizedAssetResponseDto {
  libraryId?: string | null;
  originalPath!: string;
  originalFileName!: string;
+  @ApiProperty({
+    type: 'string',
+    format: 'date-time',
+    description:
+      'The actual UTC timestamp when the file was created/captured, preserving timezone information. This is the authoritative timestamp for chronological sorting within timeline groups. Combined with timezone data, this can be used to determine the exact moment the photo was taken.',
+    example: '2024-01-15T19:30:00.000Z',
+  })
  fileCreatedAt!: Date;
+  @ApiProperty({
+    type: 'string',
+    format: 'date-time',
+    description:
+      'The UTC timestamp when the file was last modified on the filesystem. This reflects the last time the physical file was changed, which may be different from when the photo was originally taken.',
+    example: '2024-01-16T10:15:00.000Z',
+  })
  fileModifiedAt!: Date;
+  @ApiProperty({
+    type: 'string',
+    format: 'date-time',
+    description:
+      'The UTC timestamp when the asset record was last updated in the database. This is automatically maintained by the database and reflects when any field in the asset was last modified.',
+    example: '2024-01-16T12:45:30.000Z',
+  })
  updatedAt!: Date;
  isFavorite!: boolean;
  isArchived!: boolean;
--- a/server/src/dtos/time-bucket.dto.ts
+++ b/server/src/dtos/time-bucket.dto.ts
@@ -5,72 +5,143 @@ import { AssetOrder, AssetVisibility } from 'src/enum';
 import { Optional, ValidateAssetVisibility, ValidateBoolean, ValidateUUID } from 'src/validation';

 export class TimeBucketDto {
-  @ValidateUUID({ optional: true })
+  @ValidateUUID({ optional: true, description: 'Filter assets by specific user ID' })
  userId?: string;

-  @ValidateUUID({ optional: true })
+  @ValidateUUID({ optional: true, description: 'Filter assets belonging to a specific album' })
  albumId?: string;

-  @ValidateUUID({ optional: true })
+  @ValidateUUID({ optional: true, description: 'Filter assets containing a specific person (face recognition)' })
  personId?: string;

-  @ValidateUUID({ optional: true })
+  @ValidateUUID({ optional: true, description: 'Filter assets with a specific tag' })
  tagId?: string;

-  @ValidateBoolean({ optional: true })
+  @ValidateBoolean({
+    optional: true,
+    description: 'Filter by favorite status (true for favorites only, false for non-favorites only)',
+  })
  isFavorite?: boolean;

-  @ValidateBoolean({ optional: true })
+  @ValidateBoolean({
+    optional: true,
+    description: 'Filter by trash status (true for trashed assets only, false for non-trashed only)',
+  })
  isTrashed?: boolean;

-  @ValidateBoolean({ optional: true })
+  @ValidateBoolean({
+    optional: true,
+    description: 'Include stacked assets in the response. When true, only primary assets from stacks are returned.',
+  })
  withStacked?: boolean;

-  @ValidateBoolean({ optional: true })
+  @ValidateBoolean({ optional: true, description: 'Include assets shared by partners' })
  withPartners?: boolean;

  @IsEnum(AssetOrder)
  @Optional()
-  @ApiProperty({ enum: AssetOrder, enumName: 'AssetOrder' })
+  @ApiProperty({
+    enum: AssetOrder,
+    enumName: 'AssetOrder',
+    description: 'Sort order for assets within time buckets (ASC for oldest first, DESC for newest first)',
+  })
  order?: AssetOrder;

-  @ValidateAssetVisibility({ optional: true })
+  @ValidateAssetVisibility({
+    optional: true,
+    description: 'Filter by asset visibility status (ARCHIVE, TIMELINE, HIDDEN, LOCKED)',
+  })
  visibility?: AssetVisibility;
 }

 export class TimeBucketAssetDto extends TimeBucketDto {
+  @ApiProperty({
+    type: 'string',
+    description: 'Time bucket identifier in YYYY-MM-DD format (e.g., "2024-01-01" for January 2024)',
+    example: '2024-01-01',
+  })
  @IsString()
  timeBucket!: string;
 }

-export class TimelineStackResponseDto {
-  id!: string;
-  primaryAssetId!: string;
-  assetCount!: number;
-}
-
 export class TimeBucketAssetResponseDto {
+  @ApiProperty({
+    type: 'array',
+    items: { type: 'string' },
+    description: 'Array of asset IDs in the time bucket',
+  })
  id!: string[];

+  @ApiProperty({
+    type: 'array',
+    items: { type: 'string' },
+    description: 'Array of owner IDs for each asset',
+  })
  ownerId!: string[];

+  @ApiProperty({
+    type: 'array',
+    items: { type: 'number' },
+    description: 'Array of aspect ratios (width/height) for each asset',
+  })
  ratio!: number[];

+  @ApiProperty({
+    type: 'array',
+    items: { type: 'boolean' },
+    description: 'Array indicating whether each asset is favorited',
+  })
  isFavorite!: boolean[];

-  @ApiProperty({ enum: AssetVisibility, enumName: 'AssetVisibility', isArray: true })
+  @ApiProperty({
+    enum: AssetVisibility,
+    enumName: 'AssetVisibility',
+    isArray: true,
+    description: 'Array of visibility statuses for each asset (e.g., ARCHIVE, TIMELINE, HIDDEN, LOCKED)',
+  })
  visibility!: AssetVisibility[];

+  @ApiProperty({
+    type: 'array',
+    items: { type: 'boolean' },
+    description: 'Array indicating whether each asset is in the trash',
+  })
  isTrashed!: boolean[];

+  @ApiProperty({
+    type: 'array',
+    items: { type: 'boolean' },
+    description: 'Array indicating whether each asset is an image (false for videos)',
+  })
  isImage!: boolean[];

-  @ApiProperty({ type: 'array', items: { type: 'string', nullable: true } })
+  @ApiProperty({
+    type: 'array',
+    items: { type: 'string', nullable: true },
+    description: 'Array of BlurHash strings for generating asset previews (base64 encoded)',
+  })
  thumbhash!: (string | null)[];

-  localDateTime!: string[];
+  @ApiProperty({
+    type: 'array',
+    items: { type: 'string' },
+    description: 'Array of file creation timestamps in UTC (ISO 8601 format, without timezone)',
+  })
+  fileCreatedAt!: string[];

-  @ApiProperty({ type: 'array', items: { type: 'string', nullable: true } })
+  @ApiProperty({
+    type: 'array',
+    items: { type: 'number' },
+    description:
+      "Array of UTC offset hours at the time each photo was taken. Positive values are east of UTC, negative values are west of UTC. Values may be fractional (e.g., 5.5 for +05:30, -9.75 for -09:45). Applying this offset to 'fileCreatedAt' will give you the time the photo was taken from the photographer's perspective.",
+  })
+  localOffsetHours!: number[];
+
+  @ApiProperty({
+    type: 'array',
+    items: { type: 'string', nullable: true },
+    description: 'Array of video durations in HH:MM:SS format (null for images)',
+  })
  duration!: (string | null)[];

  @ApiProperty({
@@ -82,27 +153,51 @@ export class TimeBucketAssetResponseDto {
      maxItems: 2,
      nullable: true,
    },
-    description: '(stack ID, stack asset count) tuple',
+    description: 'Array of stack information as [stackId, assetCount] tuples (null for non-stacked assets)',
  })
  stack?: ([string, string] | null)[];

-  @ApiProperty({ type: 'array', items: { type: 'string', nullable: true } })
+  @ApiProperty({
+    type: 'array',
+    items: { type: 'string', nullable: true },
+    description: 'Array of projection types for 360° content (e.g., "EQUIRECTANGULAR", "CUBEFACE", "CYLINDRICAL")',
+  })
  projectionType!: (string | null)[];

-  @ApiProperty({ type: 'array', items: { type: 'string', nullable: true } })
+  @ApiProperty({
+    type: 'array',
+    items: { type: 'string', nullable: true },
+    description: 'Array of live photo video asset IDs (null for non-live photos)',
+  })
  livePhotoVideoId!: (string | null)[];

-  @ApiProperty({ type: 'array', items: { type: 'string', nullable: true } })
+  @ApiProperty({
+    type: 'array',
+    items: { type: 'string', nullable: true },
+    description: 'Array of city names extracted from EXIF GPS data',
+  })
  city!: (string | null)[];

-  @ApiProperty({ type: 'array', items: { type: 'string', nullable: true } })
+  @ApiProperty({
+    type: 'array',
+    items: { type: 'string', nullable: true },
+    description: 'Array of country names extracted from EXIF GPS data',
+  })
  country!: (string | null)[];
 }

 export class TimeBucketsResponseDto {
-  @ApiProperty({ type: 'string' })
+  @ApiProperty({
+    type: 'string',
+    description: 'Time bucket identifier in YYYY-MM-DD format representing the start of the time period',
+    example: '2024-01-01',
+  })
  timeBucket!: string;

-  @ApiProperty({ type: 'integer' })
+  @ApiProperty({
+    type: 'integer',
+    description: 'Number of assets in this time bucket',
+    example: 42,
+  })
  count!: number;
 }
--- a/server/src/queries/asset.repository.sql
+++ b/server/src/queries/asset.repository.sql
@@ -242,7 +242,7 @@ with
      and "assets"."visibility" in ('archive', 'timeline')
  )
 select
-  "timeBucket",
+  "timeBucket"::date::text as "timeBucket",
  count(*) as "count"
 from
  "assets"
@@ -262,9 +262,16 @@ with
      assets.type = 'IMAGE' as "isImage",
      assets."deletedAt" is not null as "isTrashed",
      "assets"."livePhotoVideoId",
-      "assets"."localDateTime",
+      extract(
+        epoch
+        from
+          (
+            assets."localDateTime" - assets."fileCreatedAt" at time zone 'UTC'
+          )
+      )::real / 3600 as "localOffsetHours",
      "assets"."ownerId",
      "assets"."status",
+      assets."fileCreatedAt" at time zone 'utc' as "fileCreatedAt",
      encode("assets"."thumbhash", 'base64') as "thumbhash",
      "exif"."city",
      "exif"."country",
@@ -313,7 +320,7 @@ with
          and "asset_stack"."primaryAssetId" != "assets"."id"
      )
    order by
-      "assets"."localDateTime" desc
+      "assets"."fileCreatedAt" desc
  ),
  "agg" as (
    select
@@ -326,7 +333,8 @@ with
      coalesce(array_agg("isImage"), '{}') as "isImage",
      coalesce(array_agg("isTrashed"), '{}') as "isTrashed",
      coalesce(array_agg("livePhotoVideoId"), '{}') as "livePhotoVideoId",
-      coalesce(array_agg("localDateTime"), '{}') as "localDateTime",
+      coalesce(array_agg("fileCreatedAt"), '{}') as "fileCreatedAt",
+      coalesce(array_agg("localOffsetHours"), '{}') as "localOffsetHours",
      coalesce(array_agg("ownerId"), '{}') as "ownerId",
      coalesce(array_agg("projectionType"), '{}') as "projectionType",
      coalesce(array_agg("ratio"), '{}') as "ratio",
--- a/server/src/repositories/asset.repository.ts
+++ b/server/src/repositories/asset.repository.ts
@@ -532,51 +532,44 @@ export class AssetRepository {

  @GenerateSql({ params: [{ size: TimeBucketSize.MONTH }] })
  async getTimeBuckets(options: TimeBucketOptions): Promise<TimeBucketItem[]> {
-    return (
-      this.db
-        .with('assets', (qb) =>
-          qb
-            .selectFrom('assets')
-            .select(truncatedDate<Date>(TimeBucketSize.MONTH).as('timeBucket'))
-            .$if(!!options.isTrashed, (qb) => qb.where('assets.status', '!=', AssetStatus.DELETED))
-            .where('assets.deletedAt', options.isTrashed ? 'is not' : 'is', null)
-            .$if(options.visibility === undefined, withDefaultVisibility)
-            .$if(!!options.visibility, (qb) => qb.where('assets.visibility', '=', options.visibility!))
-            .$if(!!options.albumId, (qb) =>
-              qb
-                .innerJoin('albums_assets_assets', 'assets.id', 'albums_assets_assets.assetsId')
-                .where('albums_assets_assets.albumsId', '=', asUuid(options.albumId!)),
-            )
-            .$if(!!options.personId, (qb) => hasPeople(qb, [options.personId!]))
-            .$if(!!options.withStacked, (qb) =>
-              qb
-                .leftJoin('asset_stack', (join) =>
-                  join
-                    .onRef('asset_stack.id', '=', 'assets.stackId')
-                    .onRef('asset_stack.primaryAssetId', '=', 'assets.id'),
-                )
-                .where((eb) => eb.or([eb('assets.stackId', 'is', null), eb(eb.table('asset_stack'), 'is not', null)])),
-            )
-            .$if(!!options.userIds, (qb) => qb.where('assets.ownerId', '=', anyUuid(options.userIds!)))
-            .$if(options.isFavorite !== undefined, (qb) => qb.where('assets.isFavorite', '=', options.isFavorite!))
-            .$if(!!options.assetType, (qb) => qb.where('assets.type', '=', options.assetType!))
-            .$if(options.isDuplicate !== undefined, (qb) =>
-              qb.where('assets.duplicateId', options.isDuplicate ? 'is not' : 'is', null),
-            )
-            .$if(!!options.tagId, (qb) => withTagId(qb, options.tagId!)),
-        )
-        .selectFrom('assets')
-        .select('timeBucket')
-        /*
-        TODO: the above line outputs in ISO format, which bloats the response.
-        The line below outputs in YYYY-MM-DD format, but needs a change in the web app to work.
-          .select(sql<string>`"timeBucket"::date::text`.as('timeBucket'))
-        */
-        .select((eb) => eb.fn.countAll<number>().as('count'))
-        .groupBy('timeBucket')
-        .orderBy('timeBucket', options.order ?? 'desc')
-        .execute() as any as Promise<TimeBucketItem[]>
-    );
+    return this.db
+      .with('assets', (qb) =>
+        qb
+          .selectFrom('assets')
+          .select(truncatedDate<Date>(TimeBucketSize.MONTH).as('timeBucket'))
+          .$if(!!options.isTrashed, (qb) => qb.where('assets.status', '!=', AssetStatus.DELETED))
+          .where('assets.deletedAt', options.isTrashed ? 'is not' : 'is', null)
+          .$if(options.visibility === undefined, withDefaultVisibility)
+          .$if(!!options.visibility, (qb) => qb.where('assets.visibility', '=', options.visibility!))
+          .$if(!!options.albumId, (qb) =>
+            qb
+              .innerJoin('albums_assets_assets', 'assets.id', 'albums_assets_assets.assetsId')
+              .where('albums_assets_assets.albumsId', '=', asUuid(options.albumId!)),
+          )
+          .$if(!!options.personId, (qb) => hasPeople(qb, [options.personId!]))
+          .$if(!!options.withStacked, (qb) =>
+            qb
+              .leftJoin('asset_stack', (join) =>
+                join
+                  .onRef('asset_stack.id', '=', 'assets.stackId')
+                  .onRef('asset_stack.primaryAssetId', '=', 'assets.id'),
+              )
+              .where((eb) => eb.or([eb('assets.stackId', 'is', null), eb(eb.table('asset_stack'), 'is not', null)])),
+          )
+          .$if(!!options.userIds, (qb) => qb.where('assets.ownerId', '=', anyUuid(options.userIds!)))
+          .$if(options.isFavorite !== undefined, (qb) => qb.where('assets.isFavorite', '=', options.isFavorite!))
+          .$if(!!options.assetType, (qb) => qb.where('assets.type', '=', options.assetType!))
+          .$if(options.isDuplicate !== undefined, (qb) =>
+            qb.where('assets.duplicateId', options.isDuplicate ? 'is not' : 'is', null),
+          )
+          .$if(!!options.tagId, (qb) => withTagId(qb, options.tagId!)),
+      )
+      .selectFrom('assets')
+      .select(sql<string>`"timeBucket"::date::text`.as('timeBucket'))
+      .select((eb) => eb.fn.countAll<number>().as('count'))
+      .groupBy('timeBucket')
+      .orderBy('timeBucket', options.order ?? 'desc')
+      .execute() as any as Promise<TimeBucketItem[]>;
  }

  @GenerateSql({
@@ -596,9 +589,12 @@ export class AssetRepository {
            sql`assets.type = 'IMAGE'`.as('isImage'),
            sql`assets."deletedAt" is not null`.as('isTrashed'),
            'assets.livePhotoVideoId',
-            'assets.localDateTime',
+            sql`extract(epoch from (assets."localDateTime" - assets."fileCreatedAt" at time zone 'UTC'))::real / 3600`.as(
+              'localOffsetHours',
+            ),
            'assets.ownerId',
            'assets.status',
+            sql`assets."fileCreatedAt" at time zone 'utc'`.as('fileCreatedAt'),
            eb.fn('encode', ['assets.thumbhash', sql.lit('base64')]).as('thumbhash'),
            'exif.city',
            'exif.country',
@@ -666,7 +662,7 @@ export class AssetRepository {
          )
          .$if(!!options.isTrashed, (qb) => qb.where('assets.status', '!=', AssetStatus.DELETED))
          .$if(!!options.tagId, (qb) => withTagId(qb, options.tagId!))
-          .orderBy('assets.localDateTime', options.order ?? 'desc'),
+          .orderBy('assets.fileCreatedAt', options.order ?? 'desc'),
      )
      .with('agg', (qb) =>
        qb
@@ -682,7 +678,8 @@ export class AssetRepository {
            // TODO: isTrashed is redundant as it will always be all true or false depending on the options
            eb.fn.coalesce(eb.fn('array_agg', ['isTrashed']), sql.lit('{}')).as('isTrashed'),
            eb.fn.coalesce(eb.fn('array_agg', ['livePhotoVideoId']), sql.lit('{}')).as('livePhotoVideoId'),
-            eb.fn.coalesce(eb.fn('array_agg', ['localDateTime']), sql.lit('{}')).as('localDateTime'),
+            eb.fn.coalesce(eb.fn('array_agg', ['fileCreatedAt']), sql.lit('{}')).as('fileCreatedAt'),
+            eb.fn.coalesce(eb.fn('array_agg', ['localOffsetHours']), sql.lit('{}')).as('localOffsetHours'),
            eb.fn.coalesce(eb.fn('array_agg', ['ownerId']), sql.lit('{}')).as('ownerId'),
            eb.fn.coalesce(eb.fn('array_agg', ['projectionType']), sql.lit('{}')).as('projectionType'),
            eb.fn.coalesce(eb.fn('array_agg', ['ratio']), sql.lit('{}')).as('ratio'),
--- a/server/src/validation.ts
+++ b/server/src/validation.ts
@@ -6,7 +6,7 @@ import {
  ParseUUIDPipe,
  applyDecorators,
 } from '@nestjs/common';
-import { ApiProperty } from '@nestjs/swagger';
+import { ApiProperty, ApiPropertyOptions } from '@nestjs/swagger';
 import { Transform } from 'class-transformer';
 import {
  IsArray,
@@ -72,22 +72,28 @@ export class UUIDParamDto {
 }

 type PinCodeOptions = { optional?: boolean } & OptionalOptions;
-export const PinCode = ({ optional, ...options }: PinCodeOptions = {}) => {
+export const PinCode = (options?: PinCodeOptions & ApiPropertyOptions) => {
+  const { optional, nullable, emptyToNull, ...apiPropertyOptions } = {
+    optional: false,
+    nullable: false,
+    emptyToNull: false,
+    ...options,
+  };
  const decorators = [
    IsString(),
    IsNotEmpty(),
    Matches(/^\d{6}$/, { message: ({ property }) => `${property} must be a 6-digit numeric string` }),
-    ApiProperty({ example: '123456' }),
+    ApiProperty({ example: '123456', ...apiPropertyOptions }),
  ];

  if (optional) {
-    decorators.push(Optional(options));
+    decorators.push(Optional({ nullable, emptyToNull }));
  }

  return applyDecorators(...decorators);
 };

-export interface OptionalOptions extends ValidationOptions {
+export interface OptionalOptions {
  nullable?: boolean;
  /** convert empty strings to null */
  emptyToNull?: boolean;
@@ -127,22 +133,32 @@ export const ValidateHexColor = () => {
 };

 type UUIDOptions = { optional?: boolean; each?: boolean; nullable?: boolean };
-export const ValidateUUID = (options?: UUIDOptions) => {
-  const { optional, each, nullable } = { optional: false, each: false, nullable: false, ...options };
+export const ValidateUUID = (options?: UUIDOptions & ApiPropertyOptions) => {
+  const { optional, each, nullable, ...apiPropertyOptions } = {
+    optional: false,
+    each: false,
+    nullable: false,
+    ...options,
+  };
  return applyDecorators(
    IsUUID('4', { each }),
-    ApiProperty({ format: 'uuid' }),
+    ApiProperty({ format: 'uuid', ...apiPropertyOptions }),
    optional ? Optional({ nullable }) : IsNotEmpty(),
    each ? IsArray() : IsString(),
  );
 };

 type DateOptions = { optional?: boolean; nullable?: boolean; format?: 'date' | 'date-time' };
-export const ValidateDate = (options?: DateOptions) => {
-  const { optional, nullable, format } = { optional: false, nullable: false, format: 'date-time', ...options };
+export const ValidateDate = (options?: DateOptions & ApiPropertyOptions) => {
+  const { optional, nullable, format, ...apiPropertyOptions } = {
+    optional: false,
+    nullable: false,
+    format: 'date-time',
+    ...options,
+  };

  const decorators = [
-    ApiProperty({ format }),
+    ApiProperty({ format, ...apiPropertyOptions }),
    IsDate(),
    optional ? Optional({ nullable: true }) : IsNotEmpty(),
    Transform(({ key, value }) => {
@@ -166,9 +182,12 @@ export const ValidateDate = (options?: DateOptions) => {
 };

 type AssetVisibilityOptions = { optional?: boolean };
-export const ValidateAssetVisibility = (options?: AssetVisibilityOptions) => {
-  const { optional } = { optional: false, ...options };
-  const decorators = [IsEnum(AssetVisibility), ApiProperty({ enumName: 'AssetVisibility', enum: AssetVisibility })];
+export const ValidateAssetVisibility = (options?: AssetVisibilityOptions & ApiPropertyOptions) => {
+  const { optional, ...apiPropertyOptions } = { optional: false, ...options };
+  const decorators = [
+    IsEnum(AssetVisibility),
+    ApiProperty({ enumName: 'AssetVisibility', enum: AssetVisibility, ...apiPropertyOptions }),
+  ];

  if (optional) {
    decorators.push(Optional());
@@ -177,10 +196,10 @@ export const ValidateAssetVisibility = (options?: AssetVisibilityOptions) => {
 };

 type BooleanOptions = { optional?: boolean };
-export const ValidateBoolean = (options?: BooleanOptions) => {
-  const { optional } = { optional: false, ...options };
+export const ValidateBoolean = (options?: BooleanOptions & ApiPropertyOptions) => {
+  const { optional, ...apiPropertyOptions } = { optional: false, ...options };
  const decorators = [
-    // ApiProperty(),
+    ApiProperty(apiPropertyOptions),
    IsBoolean(),
    Transform(({ value }) => {
      if (value == 'true') {