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

Removing picture from album does not update the album cover #575

Closed
make-github-pseudonymous-again opened this issue Oct 30, 2020 · 15 comments
Assignees
Labels
bug Something isn't working released Available in the stable release
Milestone

Comments

@make-github-pseudonymous-again

If one removes from an album the picture used by its cover, the cover does not get updated to use a different picture from the album. Bug or feature?

@graciousgrey
Copy link
Member

That's because it is cached. It will disappear later.

@make-github-pseudonymous-again
Copy link
Author

Ok. At what level is it cached? If I open a shared album with deleted cover picture in an incognito/private tab with disabled cache the deleted picture still shows up as a cover. This happens even if the shared link is created after the deletion. There does not seem to be a way to manually change the cover picture. I could recreate the album but that defeats the purpose... I create the album first by selecting all pictures in some time range, then I delete the ones I do not want. If one of the is the cover picture then the cover picture stays there. This is bad if that picture is not meant to be shared for whatever reason.

@lastzero
Copy link
Member

lastzero commented Nov 2, 2020

On the server in memory. Expires after one hour or so. Cache should be cleared if album items change, but wasn't a top priority so far.

@make-github-pseudonymous-again
Copy link
Author

On the server in memory. Expires after one hour or so. Cache should be cleared if album items change, but wasn't a top priority so far.

It has been more than two weeks than the cover picture was removed from the album but it still shows up in a shared link open in incognito/private tab with disabled cache. I am using NGINX as proxy with the following configuration:

proxy_redirect off;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header Host $http_host;

proxy_http_version 1.1;
proxy_set_header Upgrade $http_upgrade;
proxy_set_header Connection "upgrade";
proxy_set_header X-Forwarded-Proto $scheme;

client_max_body_size 30M;
proxy_buffering off;
location / {
    auth_basic 'PLEASE IDENTIFY';
    auth_basic_user_file /etc/nginx/photoprism.example.tld/.htpasswd;
    proxy_pass http://localhost:2342;
}

location ~ ^/(?:s|static|api)/.* { # Disable authentication for shares.
    auth_basic off;
    proxy_pass http://localhost:2342;
}

I get a 200 OK response for the tile_500 request:

Request URL: https://photoprism.example.tld/api/v1/albums/<blurb>/t/public/tile_500
Request Method: GET
Status Code: 200 OK
Remote Address: <IP ADDRESS>:443
Referrer Policy: no-referrer-when-downgrade
Accept-Ranges: bytes
Connection: keep-alive
Content-Length: 112458
Content-Type: image/jpeg
Date: Mon, 02 Nov 2020 06:07:08 GMT
Last-Modified: Mon, 12 Oct 2020 01:23:45 GMT
Server: nginx
Accept: image/avif,image/webp,image/apng,image/*,*/*;q=0.8
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=0.9
Connection: keep-alive
Host: photoprism.example.tld
Referer: https://photoprism.example.tld/s/<blurb>
Sec-Fetch-Dest: image
Sec-Fetch-Mode: no-cors
Sec-Fetch-Site: same-origin
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.111 Safari/537.36

@graciousgrey graciousgrey reopened this Nov 2, 2020
@lastzero
Copy link
Member

lastzero commented Nov 2, 2020

@lastzero
Copy link
Member

lastzero commented Nov 8, 2020

@aureooms Could you take a look at our code? If you think this is a bug, I might be able to fix it next week. In any case, it should work to clear the cache when an album changes - just wasn't a priority so far. Last week was crazy busy again, so I didn't get to much.

@lastzero lastzero self-assigned this Nov 8, 2020
@lastzero lastzero added the bug Something isn't working label Nov 8, 2020
@lastzero lastzero added this to the MVP milestone Nov 8, 2020
@make-github-pseudonymous-again
Copy link
Author

Also the cover picture does not update when I restart the server. I am 100% sure the picture used for the cover is not part of the album (it was included at album creation but was removed later). I will look at the code.

@lastzero
Copy link
Member

Maybe there is an issue with the db query then: We keep the database row when a photo was removed, maybe the "hidden" flag is ignored?

@make-github-pseudonymous-again
Copy link
Author

Here is what I think is happening:

@make-github-pseudonymous-again
Copy link
Author

Maybe there is an issue with the db query then: We keep the database row when a photo was removed, maybe the "hidden" flag is ignored?

Yes!

@make-github-pseudonymous-again
Copy link
Author

Can you point to where hidden is used in order to filter which photos to load in the UI? I was not able to find it.

@lpnueg4
Copy link

lpnueg4 commented Nov 13, 2020

Why not add a button to set album cover ? so this problem is gone

@lastzero
Copy link
Member

@lpnueg4 It's on our todo, just didn't have time for this yet. Showing a default cover is a must, selecting a cover is optional. See #604.

lastzero added a commit that referenced this issue Nov 21, 2020
@lastzero lastzero added the please-test Ready for acceptance test label Nov 21, 2020
@lastzero
Copy link
Member

Starting a new Docker build for testing soon.

@make-github-pseudonymous-again
Copy link
Author

The changeset in 668025c looks good to me. Clearing the cache when the album cover query response changes would be sufficient. Clearing on each update is safe.

@lastzero lastzero added released Available in the stable release and removed please-test Ready for acceptance test labels Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released Available in the stable release
Projects
Status: Release 🌈
Development

No branches or pull requests

4 participants