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

Albums: Ignore public filter if "Private" feature is disabled in Settings #2570

Closed
Kafkalasch opened this issue Jul 31, 2022 · 22 comments
Closed
Assignees
Labels
docs 📚 Write, improve, or review documentation enhancement Refactoring, improvement or maintenance task released Available in the stable release

Comments

@Kafkalasch
Copy link

1. What is not working as documented?

The Setting Settings -> General -> Private states:

Excludes content marked as private from search results, shared albums, labels and places.

This works as expected, when turned on. But if I turn it off, all that happens is that the private section on the left hand site disappears. The photos are cannot be viewed anymore. This does not seem to be a cache issue as this stays such even after refreshing the browser.

2. How can we reproduce it?

Steps to reproduce the behavior:
Go to demo version of photoprism

  1. Verify that settings -> private is on (or turn it on)
  2. Go to calender -> Feb 2017, and select the flower and mark it private.
  3. Verify that it is not shown any more of you re-open the view
  4. Turn settings -> private off
  5. Go to calender -> Feb 2017, you still cannot see the flower.

3. What behavior do you expect?

I would expect to see the photos again when turning the settings->private flag off.

Basically this is my usecase:
-> If I want to show the photos to friends/family, I set the flag in the settings to true
-> If I want to show the photos to my partner, I would like to disable the privacy setting to see all photos in folders/places/month view. But this does not work

4. What could be the cause of your problem?

I assume that the UI/backend still check the private flag. No matter how the settings are.
So basically, instead of (pseudo-code):
shouldIncludePhoto = photo.private !== true
one should check
shouldIncludePhoto = settings.private === true && photo.private !== true

But this is just an assumption, I haven't checked this in the source code.

@Kafkalasch Kafkalasch added the bug Something isn't working label Jul 31, 2022
@lastzero
Copy link
Member

This actually only affects the user interface, i.e. pictures that are already marked as private will not be displayed just because the section and action buttons are not visible. Our documentation should be improved in this regard. Feel free to contribute by sending a pull request!

@lastzero lastzero added help wanted Well suited for external contributors! easy Easy issue for beginners docs 📚 Write, improve, or review documentation and removed bug Something isn't working labels Jul 31, 2022
@Kafkalasch
Copy link
Author

Kafkalasch commented Jul 31, 2022

Hmm... but this doesn't feel at least like it should be. when I disable the "setting: private", I would expect that

  • section and action buttons are not visible any more
  • any private photos are visible everywhere again
    So basically that the concept of "private" does not exist any more.

@Kafkalasch
Copy link
Author

Kafkalasch commented Jul 31, 2022

So, I could have a look into how this is done in the frontend and make it such that it behaves as I described above. Is this ok? Then the documentation wouldn't need no update

Is it clear what I mean? Or shall I try to elaborate?

@Kafkalasch
Copy link
Author

Funny thing is, that this seems to not affect the search results. So if "settings:private" is off, photos previously marked as private appear in search result. they simply don't appear in folders/months/places/etc.

@lastzero
Copy link
Member

lastzero commented Aug 1, 2022

These are automatically created search filters and have a visibility flag based on if there are any matching pictures. Also they have cached counts. So possible that it's a tradeoff to avoid inconsistencies. We are currently on vacation and can't look into it. Long time since I implement it. To display all photos, setting photos.photo_private to 0 via SQL and then run index once to trigger an update is the fastest way.

@Kafkalasch
Copy link
Author

photos.photo_private to 0 via SQL

I guess this would delete all the markers from the photos. This is not what I want to accomplish.

Basically what I want:

  • Settings.Private-Mode ON => Private marker of photos can be edited. Photos are shown in "Private" Section. Photos are not shown anywhere else
  • Settings.Private-Mode OFF => Private marker of photos cannot be edited. "Private" Section does not exist. Photos are shown everywhere. (Simply as if the word "private" would not exist)
  • Settings.Private-Mode ON => All photos that have the private marker are invisible again

Note, that the private marker on photos are not deleted or reset if set to Settings.Private-Mode OFF. They are simply ignored.

Do you understand what I mean? Sorry, if I don't use the correct terms. Haven't wen't through the entire Developer-Docs and code base yet.

@Kafkalasch
Copy link
Author

But this is nothing urgent, please enjoy your vacation =)

@lastzero
Copy link
Member

lastzero commented Aug 1, 2022

I understand what you are trying to accomplish. The reason it doesn't currently work as you expect is that "public:true" is part of the auto-generated search filter used to implement folders, places, states, months and so on:

path:"2001/18. Oktober 2001" public:true

See albums.album_filter. If you change the feature flag, the values of this column are not affected in any way. So nothing changes with respect to existing album filters.

Theoretically, we could ignore the public filter completely, but I want to make absolutely sure that this has no unintended side effects. Meaning, this is not a "quick win." In the short term, it seems best to update the docs.

lastzero added a commit that referenced this issue Aug 1, 2022
This needs to be very well tested and discussed, as these changes can
lead to private photos being accidentally published. Thank you!

Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

lastzero commented Aug 1, 2022

This commit to implement your request has changed 91 files, which means it's pretty significant. Please test these changes extensively while we are on vacation! Thank you 👍

Also note that the "Private" feature is a (possibly user-specific) setting, not a global configuration option. For this reason, I have refrained from showing photos marked as private in the shares, even if "Private" is turned off. Users might otherwise unintentionally share photos they don't want to show. At the very least, this should be well documented and discussed with the community.

@lastzero lastzero changed the title Pictures marked private do not reappear in Folders/Search/Places etc. when Settings -> Private Flag is disabled Search: Ignore public album filter if "Private" feature is disabled in Settings Aug 1, 2022
@lastzero lastzero changed the title Search: Ignore public album filter if "Private" feature is disabled in Settings Albums: Ignore public filter if "Private" feature is disabled in Settings Aug 1, 2022
@lastzero lastzero added enhancement Refactoring, improvement or maintenance task and removed easy Easy issue for beginners labels Aug 1, 2022
@lastzero lastzero self-assigned this Aug 1, 2022
@lastzero
Copy link
Member

lastzero commented Aug 1, 2022

Started a new preview build for testing! 👍

@Kafkalasch
Copy link
Author

Kafkalasch commented Aug 1, 2022

thank you very much. that was pretty fast!

@Kafkalasch
Copy link
Author

Also note that the "Private" feature is a (possibly user-specific) setting, not a global configuration option. For this reason, I have refrained from showing photos marked as private in the shares, even if "Private" is turned off. Users might otherwise unintentionally share photos they don't want to show. At the very least, this should be well documented and discussed with the community.

This makes totally sense!

@Kafkalasch
Copy link
Author

Kafkalasch commented Aug 1, 2022

oh wow, i briefly glimpsed over your changes. that was quite a lot! Sorry for interrupting your vacations!

@Kafkalasch
Copy link
Author

Hi, I am testing it right now.

The current docs state that:

By default, photos marked as private will not appear in the following sections:

  • Search
  • Videos
  • Labels
  • Places
  • Favorites
  • Shared albums

Testing, with Settings Private ON:

  • Search: Private photos are hidden (check)
  • Shared Albums: Private photos are hidden (check)
  • Albums:
    • Private Fotos appear in album (same behavior as in productive). Is this really wanted?
  • Videos: Private videos are hidden (check)
  • Favorites: Private photos are hideen (check)
  • People: Private photos are hidden if the person is clicked.
    • But: "Contains x pictures" shows the wrong sum. It shows the sum of private and non-private pictures
    • But: If the picture indicating the face is marked private, it is still used as thumbnail for the face. (which is bad) If you click on it, the full picture is not included in the collection. (which is good)
    • [DOCS-MISSMATCH] Also it is not mentioned in the docs
  • Moments: They were not created with my test-images. So could not test
    • But: In the productive version: If you mark the title image (or is it the thumbnail of it?) of a moment private, it will not show in the moment's collection any more, but it stays the title image of the moment itself. So this is not a new bug!!
    • [DOCS-MISSMATCH] Also it is not mentioned in the docs
  • Calendar: Private photos are hidden.
    • But: [DOCS-MISSMATCH] this is missing in the docs
  • Places: Private photos are NOT hidden. They are also visible in the current productive version. So this is not a new bug!!
  • Labels: Private photos are hidden (check)
    • But: [DOCS-MISSMATCH] this is missing in the docs
  • Folders: Private photos are hidden (check)
    • But: [DOCS-MISSMATCH] this is missing in the docs

Testing, with Settings Private OFF:

  • Search: Private photos are shown (check)
  • Shared Albums: Private photos are still hidden (check)
  • Albums: Private photos appear in album (same as with Settings Private ON) (check)
  • Videos: Private videos are shown (check)
  • Favorites: Private photos are shown (check)
  • People: Private photos are shown (check)
  • Moments: They were not created with my test-images. So could not test
  • Calendar: Private photos are shown (check)
  • Places: Private photos are shown (check)
  • Labels: Private photos are shown (check)
  • Folders: Private photos are shown (check)

Summary

  • Private Settings ON: There were a couple of bugs. Also in the productive version which need some attention
  • Private Settings OFF: Everything worked
  • I did not test "Moments"
  • I can update the docs, if you want to. But let's clarify first, what exactly is wanted and should be included. I marked the points that I think should be included with [DOCS-MISSMATCH]

@Kafkalasch
Copy link
Author

For those bugs that also exist in production -> shall i create bug tickets for those as well?

@lastzero
Copy link
Member

lastzero commented Aug 1, 2022

Note the cover images are cached.

@lastzero
Copy link
Member

lastzero commented Aug 1, 2022

The idea of the private flag is that no naket or otherwise private pictures of you randomly pop up when you, for example, open the calendar. For manually managed albums this is different as you intentionally added the private pictures, if any. So it should NOT surprise you when you open the album in front of someone else. Theresa will test this all again when we are back from vacation. Please don't open individual bug tickets for this meanwhile. Thank you!

@Kafkalasch
Copy link
Author

Ah, ok, thats fine, if thats intentional.

I guess then only the images in places are a bug

lastzero added a commit that referenced this issue Aug 2, 2022
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

lastzero commented Aug 2, 2022

Started another preview build with a fix for hiding private pictures in Places.

@lastzero lastzero added the please-test Ready for acceptance test label Aug 3, 2022
@graciousgrey
Copy link
Member

Docs have been updated: https://docs.photoprism.app/user-guide/organize/private/

I found two minor issues:

  1. If all photos of a person are marked as private --> The person is still shown with a crop. The full image is not visible.

  2. If all photos of an autogenerated album are marked as private --> The whole album is hidden. If Private is disabled in settings the hidden album does not re-appear. It does only re-appear in case at least one photo from it is marked as public again

lastzero added a commit that referenced this issue Aug 31, 2022
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Aug 31, 2022
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

  1. If all photos of a person are marked as private --> The person is still shown with a crop. The full image is not visible.

This was an intentional decision to improve query performance and to avoid displaying no cover at all in this case.

I've added a code comment so we can revisit this issue later when it becomes important and we have time to take care of it.

  1. If all photos of an autogenerated album are marked as private --> The whole album is hidden. If Private is disabled in settings the hidden album does not re-appear. It does only re-appear in case at least one photo from it is marked as public again

These issues should be fixed with the above commits. Also, the "private" switch has been removed from the photo editing dialog when the feature is disabled.

I'm starting a new preview build for testing soon.

@graciousgrey
Copy link
Member

If all photos of an autogenerated album are marked as private --> The whole album is hidden. If Private is disabled in settings the hidden album does not re-appear. It does only re-appear in case at least one photo from it is marked as public again

This is fixed for mariadb now. Please note that when you turn the private flag back on you need to restart PhotoPrism in order to hide previously restored autogenerated albums again.

Won't be fixed at this time for sqlite.

@lastzero lastzero removed the help wanted Well suited for external contributors! label Sep 1, 2022
@graciousgrey graciousgrey added released Available in the stable release and removed please-test Ready for acceptance test labels Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs 📚 Write, improve, or review documentation enhancement Refactoring, improvement or maintenance task released Available in the stable release
Projects
Status: Release 🌈
Development

No branches or pull requests

3 participants