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

Add support for refreshing feed icons #2123

Closed
wants to merge 9 commits into from

Conversation

rystaf
Copy link
Contributor

@rystaf rystaf commented Oct 9, 2023

Refetch feed icon if the icon url included in the feed changes or if feed is refreshed manually.

Resolves #1555 #1805


Do you follow the guidelines?

@rystaf
Copy link
Contributor Author

rystaf commented Oct 20, 2023

I removed the RefreshIcon option I added to the feeds. This was to enable an automatic check for icon changes. Now instead of just checking everytime, I'm keeping track of the icon_url in the feed table and triggering a download if it's changed.
I'm also trying out removing the feed_icons table entirely and storing the icon_id directly in the feeds table

@rystaf rystaf marked this pull request as ready for review October 20, 2023 21:32
@rystaf rystaf marked this pull request as draft October 25, 2023 20:09
@rystaf rystaf force-pushed the refresh-icon branch 5 times, most recently from 261d48b to feba42b Compare October 26, 2023 20:28
@rystaf rystaf marked this pull request as ready for review October 29, 2023 17:28
@mattxtaz
Copy link

I removed the RefreshIcon option I added to the feeds. This was to enable an automatic check for icon changes. Now instead of just checking everytime, I'm keeping track of the icon_url in the feed table and triggering a download if it's changed.

What about if the site changes the favicon image but does not change the URL? Or can you just delete the URL completely and let Miniflux automatically detect it again? I want a way to make it refresh the icon even if the URL hasn’t changed

@rystaf
Copy link
Contributor Author

rystaf commented Nov 13, 2023

@mattxtaz with this PR, clicking the "Refresh" link on the feed page would remove the feed icon and trigger miniflux to redownload it.

@rystaf rystaf force-pushed the refresh-icon branch 2 times, most recently from 85ed199 to 3e4c65a Compare January 2, 2024 18:09
Copy link
Member

@fguillot fguillot left a comment

Choose a reason for hiding this comment

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

How do you backfill the icon_url field? If I understand correctly, this field is updated only when a new icon is created or when someone force refresh the feed manually.

internal/reader/handler/handler.go Outdated Show resolved Hide resolved
internal/reader/handler/handler.go Outdated Show resolved Hide resolved
internal/storage/icon.go Show resolved Hide resolved
@rystaf
Copy link
Contributor Author

rystaf commented Jan 12, 2024

How do you backfill the icon_url field? If I understand correctly, this field is updated only when a new icon is created or when someone force refresh the feed manually.

The icon_url will match what ever is included in the feed. It gets updated when the feed is updated.

@fguillot
Copy link
Member

fguillot commented Feb 1, 2024

How do you backfill the icon_url field? If I understand correctly, this field is updated only when a new icon is created or when someone force refresh the feed manually.

The icon_url will match what ever is included in the feed. It gets updated when the feed is updated.

What about feeds that don't specify an icon URL explicitly in their feed?

The logs will have this message below but that can leads to confusion because the icon_url is always empty in the DB:

level=DEBUG msg="Feed icon not modified" user_id=1 feed_id=1 feed_icon_url=""

Tested with few feeds:

miniflux2=# select feed_url, icon_url from feeds;
                   feed_url                   |               icon_url
----------------------------------------------+---------------------------------------
 https://www.lemonde.fr/rss/une.xml           |
 https://linuxfr.org/news.atom                |
 https://blog.alexellis.io/rss/               | https://blog.alexellis.io/favicon.png
 https://miniflux.app/feed.xml                |
 https://github.com/miniflux/v2/releases.atom |
(5 rows)

@rystaf
Copy link
Contributor Author

rystaf commented Feb 1, 2024

icon urls that were discovered by fetching the feed's website are not included here since checking for changes would require refetching the feed's website on each update.

I updated the log message to read "feed icon url not modified" and only if an icon_url is set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Favicon is not updated when Site URL changes
3 participants