feat(server, web): album orders #7819

martabal · 2024-03-10T15:42:13Z

What's changed

This PR adds the ability to see album assets in ascending/descending order

Screenshots

2024-03-10.16-41-20.mp4

cloudflare-pages · 2024-03-10T15:44:44Z

Deploying with Cloudflare Pages

Latest commit:	`b790fcc`
Status:	✅ Deploy successful!
Preview URL:	https://c699755e.immich.pages.dev
Branch Preview URL:	https://feat-album-orders.immich.pages.dev

View logs

aviv926 · 2024-03-10T16:34:08Z

Fixing:
#7031
#6781
#6624

Maybe:
#7489
#1689

jrasm91

We have other existing query params, etc. We should use consistent naming here as well, not ascendingOrder. I think an enum would make more sense probably.

server/src/infra/repositories/asset.repository.ts

web/src/lib/components/album-page/album-options.svelte

web/src/lib/components/photos-page/asset-grid.svelte

web/src/lib/stores/assets.store.ts

web/src/lib/components/album-page/album-options.svelte

martabal · 2024-03-10T20:21:28Z

I think an enum would make more sense probably

Something like this ?

enum Order {
  ASCENDING = 'ascending',
  DESCENDING = 'descending',
}

web/src/lib/stores/assets.store.ts

jrasm91 · 2024-03-11T15:15:05Z

I was referring to this, which we already have

mobile/pubspec.lock

package-lock.json

jrasm91 · 2024-03-12T13:22:49Z

server/src/domain/album/album-response.dto.ts

@@ -23,6 +23,8 @@ export class AlbumResponseDto {
  startDate?: Date;
  endDate?: Date;
  isActivityEnabled!: boolean;
+  @ApiProperty({ enumName: 'AssetOrder', enum: AssetOrder })
+  order!: AssetOrder;


@shenlong-tanwen will this new property break the mobile app? Do we need to make it order?: AssetOrder?

I am pretty sure it will break the mobile app because the SDK will check for this property and it will be null from previous version, so the users will not see any albums

Let's make it optional then to avoid the breaking change. We can change it back in the future after a few releases.

@shenlong-tanwen will this new property break the mobile app? Do we need to make it order?: AssetOrder?

Yes. The mobile app will not sync albums with older server versions. Making it optional would be best.

server/src/infra/entities/album.entity.ts

danieldietzler · 2024-03-13T17:11:44Z

server/src/infra/repositories/asset.repository.ts

@@ -598,7 +598,7 @@ export class AssetRepository implements IAssetRepository {
      .select(`COUNT(asset.id)::int`, 'count')
      .addSelect(truncated, 'timeBucket')
      .groupBy(truncated)
-      .orderBy(truncated, 'DESC')
+      .orderBy(truncated, options.order === AssetOrder.ASC ? 'ASC' : 'DESC')


Can't we just do options.order.toUpperCase() instead?

I don't know about that. I don't like the idea of mixing SQL syntax and enum values

Hm ok that's fair. Coupling those would result in the query breaking in case we were to ever rename the enum value. That's actually bad, true!

As soon as we add new values to this enum, we'll have to update that part too and I don't know if typescript / typeorm can alert it. It will definitely be a disaster 😆

server/src/infra/repositories/asset.repository.ts

web/src/lib/components/album-page/album-options.svelte

…rders

alextran1502 · 2024-03-13T22:34:33Z

Hello, thank you for the PR!

I just ran some tests, and the order of the assets in the album doesn't seem to change. Can you double-check?

martabal · 2024-03-14T00:19:49Z

Hello, thank you for the PR!

I just ran some tests, and the order of the assets in the album doesn't seem to change. Can you double-check?

Can you try with make open-api && make dev-update ?

alextran1502 · 2024-03-14T01:37:41Z

@martabal It works now, sorry about my glitchy system

… feat/album-orders

alextran1502 · 2024-03-14T01:44:32Z

@martabal, actually, it doesn't work again, so there is probably a bug somewhere. Now, if you change the order, it changes once, and when you change it back, it doesn't register the change. I suspect your most recent commit 6de5367 has caused some issues

martabal · 2024-03-14T04:57:10Z

Ok, it should be fixed

…rders

waclaw66 · 2024-03-19T08:52:20Z

It would be nice to have Immich-wide or user-wide configuration for default album sorting. I personally prefer "oldest first" order for most of story-like albums, however timelapse albums looks better with "newest first" order.

martabal · 2024-03-19T08:59:43Z

It would be nice to have Immich-wide or user-wide configuration for default album sorting. I personally prefer "oldest first" order for most of story-like albums, however timelapse albums looks better with "newest first" order.

User-wide makes sense. You probably can open a FR for that

waclaw66 · 2024-03-19T09:48:45Z

User-wide makes sense. You probably can open a FR for that

Created... #8068

mondeg0 · 2024-03-21T14:16:44Z

This feature is awesome, and I was waiting for it for a long time!
However, the options to sort seems erased when the album is shared via a link. In this scenario, the album presentation is still recent to oldest, even if set to the inverse in the albums settings.

Is this a bug or desired behavior ?

martabal · 2024-03-21T15:15:17Z

This is definitely a missing feature. We prefer to add features piece by piece, it's easier to correct bugs and get user feedback.

feat: album orders

89da154

martabal added 🖥️web 🗄️server labels Mar 10, 2024

martabal force-pushed the feat/album-orders branch 2 times, most recently from 988b052 to 1162336 Compare March 10, 2024 15:54

fix: tests

1ca9b43

martabal force-pushed the feat/album-orders branch from 1162336 to 1ca9b43 Compare March 10, 2024 16:00

jrasm91 reviewed Mar 10, 2024

View reviewed changes

pr feedback

76ddaa2

jrasm91 reviewed Mar 11, 2024

View reviewed changes

web/src/lib/stores/assets.store.ts Outdated Show resolved Hide resolved

pr feedback

fa7b2f1

martabal force-pushed the feat/album-orders branch 4 times, most recently from b43110e to 356a512 Compare March 11, 2024 23:16

jrasm91 reviewed Mar 12, 2024

View reviewed changes

pr feedback

be1c39e

martabal force-pushed the feat/album-orders branch from 356a512 to be1c39e Compare March 12, 2024 17:06

fix: tests

7744fab

martabal force-pushed the feat/album-orders branch from 70c2261 to 7744fab Compare March 12, 2024 17:11

add comment

4585eff

danieldietzler reviewed Mar 13, 2024

View reviewed changes

martabal and others added 2 commits March 13, 2024 18:30

pr feedback

292392e

Merge branch 'main' of github.com:immich-app/immich into feat/album-o…

8d79099

…rders

fix: rendering issue

6de5367

alextran1502 added 2 commits March 13, 2024 20:41

wording

6fec829

Merge branch 'feat/album-orders' of github.com:immich-app/immich into…

7480766

… feat/album-orders

fix: order value doesn't change

207579e

Merge branch 'main' of github.com:immich-app/immich into feat/album-o…

b790fcc

…rders

alextran1502 approved these changes Mar 14, 2024

View reviewed changes

alextran1502 merged commit 31f7e1a into main Mar 14, 2024
24 checks passed

alextran1502 deleted the feat/album-orders branch March 14, 2024 16:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(server, web): album orders #7819

feat(server, web): album orders #7819

martabal commented Mar 10, 2024

cloudflare-pages bot commented Mar 10, 2024 •

edited

aviv926 commented Mar 10, 2024 •

edited

jrasm91 left a comment •

edited

martabal commented Mar 10, 2024

jrasm91 commented Mar 11, 2024

jrasm91 Mar 12, 2024 •

edited

alextran1502 Mar 12, 2024

jrasm91 Mar 12, 2024

shenlong-tanwen Mar 12, 2024

danieldietzler Mar 13, 2024

martabal Mar 13, 2024

danieldietzler Mar 13, 2024

martabal Mar 13, 2024

alextran1502 commented Mar 13, 2024

martabal commented Mar 14, 2024

alextran1502 commented Mar 14, 2024

alextran1502 commented Mar 14, 2024 •

edited

martabal commented Mar 14, 2024

waclaw66 commented Mar 19, 2024 •

edited

martabal commented Mar 19, 2024

waclaw66 commented Mar 19, 2024

mondeg0 commented Mar 21, 2024

martabal commented Mar 21, 2024

feat(server, web): album orders #7819

feat(server, web): album orders #7819

Conversation

martabal commented Mar 10, 2024

What's changed

Screenshots

cloudflare-pages bot commented Mar 10, 2024 • edited

Deploying with Cloudflare Pages

aviv926 commented Mar 10, 2024 • edited

jrasm91 left a comment • edited

Choose a reason for hiding this comment

martabal commented Mar 10, 2024

jrasm91 commented Mar 11, 2024

jrasm91 Mar 12, 2024 • edited

Choose a reason for hiding this comment

alextran1502 Mar 12, 2024

Choose a reason for hiding this comment

jrasm91 Mar 12, 2024

Choose a reason for hiding this comment

shenlong-tanwen Mar 12, 2024

Choose a reason for hiding this comment

danieldietzler Mar 13, 2024

Choose a reason for hiding this comment

martabal Mar 13, 2024

Choose a reason for hiding this comment

danieldietzler Mar 13, 2024

Choose a reason for hiding this comment

martabal Mar 13, 2024

Choose a reason for hiding this comment

alextran1502 commented Mar 13, 2024

martabal commented Mar 14, 2024

alextran1502 commented Mar 14, 2024

alextran1502 commented Mar 14, 2024 • edited

martabal commented Mar 14, 2024

waclaw66 commented Mar 19, 2024 • edited

martabal commented Mar 19, 2024

waclaw66 commented Mar 19, 2024

mondeg0 commented Mar 21, 2024

martabal commented Mar 21, 2024

cloudflare-pages bot commented Mar 10, 2024 •

edited

aviv926 commented Mar 10, 2024 •

edited

jrasm91 left a comment •

edited

jrasm91 Mar 12, 2024 •

edited

alextran1502 commented Mar 14, 2024 •

edited

waclaw66 commented Mar 19, 2024 •

edited