feat(server): visibility column #17939

alextran1502 · 2025-04-28T20:13:52Z

No description provided.

open-api/typescript-sdk/src/fetch-client.ts

server/src/dtos/asset-media.dto.ts

mertalev · 2025-04-29T19:55:34Z

server/src/queries/asset.job.repository.sql

-  and "assets"."isVisible" = $1
+  and "assets"."visibility" = $1


isVisible is a mechanism to handle live photos while visibility is to handle which pages an asset should be viewable from. If the effective page visibility comes from the image part, you need to be very careful when linking and unlinking them.

server/src/schema/migrations/1745902563899-AddAssetVisibilityColumn.ts

mertalev · 2025-04-29T20:05:45Z

server/src/services/metadata.service.ts

-    if (!isSync && (!asset.isVisible || asset.sidecarPath) && !asset.isExternal) {
+    if (!isSync && (asset.visibility === AssetVisibility.HIDDEN || asset.sidecarPath) && !asset.isExternal) {


May not be an issue in practice, but there may have been cases where booleans like isVisible were not present in the object and comparisons were just operating on truthiness. If there are any cases like this in the client or server, these equality comparisons would be a change in behavior. Shouldn't happen theoretically but could be good to keep in mind.

server/src/dtos/search.dto.ts

server/src/dtos/sync.dto.ts

server/src/schema/migrations/1745864846978-AddAssetVisibilityColumn.ts

server/src/dtos/asset-media.dto.ts

danieldietzler

This PR is quite hard to review actually, since so many queries are affected. From what I can tell you're changing the semantics of a bunch of queries (some of them I flagged, though most likely not all), which isn't ideal and should be done in a later PR if we actually consider the current semantics "wrong".
The issue is that it's already quite hard making sure the new query still has the same meaning as the previous query. Adding extra mental overhead by also re-thinking logic shouldn't be in here IMO

e2e/src/api/specs/timeline.e2e-spec.ts

server/src/repositories/asset.repository.ts

server/src/repositories/library.repository.ts

server/src/repositories/person.repository.ts

server/src/repositories/search.repository.ts

server/src/repositories/user.repository.ts

alextran1502 · 2025-05-05T13:21:41Z

@jrasm91 @danieldietzler @mertalev Queries' semantics should be preserved. The PR is ready for review again

danieldietzler

Looks much better!

danieldietzler · 2025-05-05T16:19:36Z

server/src/repositories/asset.repository.ts

@@ -393,7 +392,7 @@ export class AssetRepository {
                    .whereRef('stacked.stackId', '=', 'asset_stack.id')
                    .whereRef('stacked.id', '!=', 'asset_stack.primaryAssetId')
                    .where('stacked.deletedAt', 'is', null)
-                    .where('stacked.isArchived', '=', false)
+                    .where('stacked.visibility', '=', AssetVisibility.TIMELINE)


We used to be including hidden assets here as well, though they'd fly out in the join anyways afaict. Not 100% though.

@mertalev Do you have any comments about this query?

It should be fine to filter for timeline assets. It's kind of debatable whether archived or trashed assets should also be included here since getById doesn't filter for them in the top level asset, but the PR effectively maintains the current behavior here.

server/src/repositories/asset.repository.ts

server/src/repositories/map.repository.ts

server/src/utils/asset.util.ts

e2e/test-assets

…d live photos

jrasm91

LGTM

server/src/queries/access.repository.sql

jrasm91 · 2025-05-06T15:46:50Z

server/src/repositories/asset.repository.ts

@@ -542,7 +548,15 @@ export class AssetRepository {
            .select(truncatedDate<Date>(options.size).as('timeBucket'))
            .$if(!!options.isTrashed, (qb) => qb.where('assets.status', '!=', AssetStatus.DELETED))
            .where('assets.deletedAt', options.isTrashed ? 'is not' : 'is', null)
-            .where('assets.isVisible', '=', true)
+            .$if(options.visibility === undefined, (qb) =>


wow, you have this same thing like 5 times, which might have to change in the future. can you make a utility method like $if(visibility !== undefined, withDefaultVisibility) or something?

server/src/schema/enums.ts

server/src/utils/asset.util.ts

server/src/services/media.service.ts

* feat: private view * pr feedback * sql generation * feat: visibility column * fix: set visibility value as the same as the still part after unlinked live photos * fix: test * pr feedback

alextran1502 requested a review from danieldietzler as a code owner April 28, 2025 20:13

github-actions bot added the 🗄️server label Apr 28, 2025

alextran1502 requested review from jrasm91 and mertalev April 28, 2025 20:14

alextran1502 added the changelog:feature label Apr 28, 2025

alextran1502 force-pushed the private-view-1 branch 3 times, most recently from 03f6c3c to 9169598 Compare April 28, 2025 21:54

alextran1502 marked this pull request as draft April 29, 2025 05:29

alextran1502 force-pushed the private-view-1 branch 4 times, most recently from 7820549 to d0e75c3 Compare April 29, 2025 17:03

github-actions bot added the 📱mobile label Apr 29, 2025

alextran1502 force-pushed the private-view-1 branch from d0e75c3 to 7c6cd82 Compare April 29, 2025 19:16

github-actions bot added the 🖥️web label Apr 29, 2025

feat: private view

dcef370

alextran1502 force-pushed the private-view-1 branch from 7c6cd82 to dcef370 Compare April 29, 2025 19:18

github-advanced-security bot found potential problems Apr 29, 2025

View reviewed changes

open-api/typescript-sdk/src/fetch-client.ts Dismissed Show dismissed Hide dismissed

alextran1502 force-pushed the private-view-1 branch 2 times, most recently from 8d93dbd to 14f5b05 Compare April 29, 2025 19:38

mertalev reviewed Apr 29, 2025

View reviewed changes

alextran1502 force-pushed the private-view-1 branch 2 times, most recently from 42c3bf3 to 67ae3e7 Compare April 29, 2025 22:20

jrasm91 reviewed Apr 29, 2025

View reviewed changes

server/src/dtos/search.dto.ts Outdated Show resolved Hide resolved

server/src/dtos/sync.dto.ts Show resolved Hide resolved

server/src/schema/migrations/1745864846978-AddAssetVisibilityColumn.ts Outdated Show resolved Hide resolved

server/src/dtos/asset-media.dto.ts Show resolved Hide resolved

feat: visibility column

9f252f8

alextran1502 force-pushed the private-view-1 branch from 67ae3e7 to 9f252f8 Compare April 30, 2025 04:00

feat: visibility column

a1ea473

alextran1502 force-pushed the private-view-1 branch 2 times, most recently from ce877c0 to 9649ec5 Compare May 1, 2025 14:25

alextran1502 force-pushed the private-view-1 branch 2 times, most recently from 2aabb09 to 0be57cf Compare May 1, 2025 19:28

feat: visibility column

4335bac

alextran1502 force-pushed the private-view-1 branch from 0be57cf to 4335bac Compare May 2, 2025 04:36

alextran1502 marked this pull request as ready for review May 2, 2025 04:47

alextran1502 requested review from jrasm91 and mertalev May 2, 2025 17:31

danieldietzler reviewed May 2, 2025

View reviewed changes

pr feedback

7e83a17

alextran1502 force-pushed the private-view-1 branch from e044259 to 7e83a17 Compare May 2, 2025 21:27

mmomjian mentioned this pull request May 3, 2025

feat: find large files utility #18040

Open

8 tasks

alextran1502 added 2 commits May 4, 2025 22:00

merge main

166b103

sql generation

e9da74a

danieldietzler reviewed May 5, 2025

View reviewed changes

feat: visibility column

f1aac23

alextran1502 force-pushed the private-view-1 branch from 52491ff to f1aac23 Compare May 5, 2025 19:02

mertalev reviewed May 5, 2025

View reviewed changes

server/src/utils/asset.util.ts Outdated Show resolved Hide resolved

jrasm91 reviewed May 6, 2025

View reviewed changes

e2e/test-assets Outdated Show resolved Hide resolved

merge main

f0c0bc6

alextran1502 force-pushed the private-view-1 branch from 280536f to f0c0bc6 Compare May 6, 2025 15:54

alextran1502 added 2 commits May 6, 2025 11:00

fix: set visibility value as the same as the still part after unlinke…

a8f7836

…d live photos

fix: test

9ef5d47

jrasm91 approved these changes May 6, 2025

View reviewed changes

pr feedback

eb8e110

alextran1502 merged commit d33ce13 into main May 6, 2025
51 checks passed

alextran1502 deleted the private-view-1 branch May 6, 2025 17:12

wuzihao051119 mentioned this pull request May 8, 2025

fix(web): add photos to album #18166

Merged

renovate bot mentioned this pull request May 30, 2025

chore(deps): update docker-compose non-major dependencies haakemon/.dotfiles#95

Merged

1 task

		if (!isSync && (!asset.isVisible \|\| asset.sidecarPath) && !asset.isExternal) {
		if (!isSync && (asset.visibility === AssetVisibility.HIDDEN \|\| asset.sidecarPath) && !asset.isExternal) {

Uh oh!

feat(server): visibility column #17939

feat(server): visibility column #17939

Uh oh!

Conversation

alextran1502 commented Apr 28, 2025

Uh oh!

Uh oh!

Uh oh!

mertalev Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mertalev Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danieldietzler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alextran1502 commented May 5, 2025

Uh oh!

danieldietzler left a comment

Choose a reason for hiding this comment

Uh oh!

danieldietzler May 5, 2025

Choose a reason for hiding this comment

Uh oh!

alextran1502 May 5, 2025

Choose a reason for hiding this comment

Uh oh!

mertalev May 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrasm91 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jrasm91 May 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mertalev Apr 29, 2025 •

edited

Loading