Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): show partners archived photos on mobile timeline #9194

Merged
merged 1 commit into from May 7, 2024

Conversation

alextran1502
Copy link
Contributor

@alextran1502 alextran1502 commented May 1, 2024

Regression from #7580. The mobile app needs this property to not render partners' archived photos on the timeline

Fixes #9190

Copy link

cloudflare-pages bot commented May 1, 2024

Deploying immich with  Cloudflare Pages  Cloudflare Pages

Latest commit: ee02b31
Status: ✅  Deploy successful!
Preview URL: https://2830f2cf.immich.pages.dev
Branch Preview URL: https://fix-9190.immich.pages.dev

View logs

@alextran1502 alextran1502 changed the title fix(mobile): show partners archived photos on timeline fix(server): show partners archived photos on timeline May 1, 2024
@alextran1502 alextran1502 changed the title fix(server): show partners archived photos on timeline fix(server): show partners archived photos on mobile timeline May 1, 2024
Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you have archived partner assets to begin with?

@martabal
Copy link
Member

martabal commented May 1, 2024

I'm really not a fan: if the asset is shared, everyone can see which asset has been archived by the owner.

Seeing an album as a non-owner (all assets are from the album owner) :
image

@jrasm91
Copy link
Contributor

jrasm91 commented May 1, 2024

I'm with @martabal on this one. I don't think partners are even supposed to have access to archived assets to begin with.

@alextran1502
Copy link
Contributor Author

Ok, I can tailor the request from the mobile app to not include partners assets in the getAllAssets call

@jrasm91
Copy link
Contributor

jrasm91 commented May 1, 2024

Ok, I can tailor the request from the mobile app to not include partners assets in the getAllAssets call

Sure, and then maybe we get retest @fyfrey's PR, taking into account it does not include archived photos from partners.

@fyfrey
Copy link
Contributor

fyfrey commented May 4, 2024

That'd be great. I'd like to get the new sync working in server+app :)
So, @alextran1502 please test without archived partner assets, then I can update my sync server PR accordingly - thanks!

@alextran1502
Copy link
Contributor Author

I am merging this PR, so that we can make sure the mobile's sync mechanism will work first before refactoring it to not include the archive status.

@alextran1502 alextran1502 merged commit bbb9453 into main May 7, 2024
28 of 29 checks passed
@alextran1502 alextran1502 deleted the fix-9190 branch May 7, 2024 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Partner Sharing Showing Archived Photos
5 participants