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 new field nextUpdateTime to feed #2993

Merged
merged 1 commit into from
Dec 22, 2024
Merged

Add new field nextUpdateTime to feed #2993

merged 1 commit into from
Dec 22, 2024

Conversation

Grotax
Copy link
Member

@Grotax Grotax commented Dec 19, 2024

Summary

This is a first step to modernize the update cycle, the idea is to not fetch feeds on a fixed schedule.
feed-io provides a function to calculate the timestamp when an update would make sense.
This timestamp gets stored in the feed model now and can be used in a future PR to decide if we update a feed or not.

This is of course with feeds in mind that work like "normal" news feeds there might be special use case feeds that do not work well with this approach. But News does not claim to be the perfect solution for everything it is a feed reader and server api that helps users to consume feeds from different sources.

This also adds a new field to the API: nextUpdateTime contains a timestamp which indicates when the next update is due

{
    "feeds": [
        {
            "added": 1734689445,
            "faviconLink": "http://localhost:8090/logo.png",
            "folderId": null,
            "id": 39,
            "items": [],
            "lastUpdateError": null,
            "link": "https://nextcloud.com/",
            "nextUpdateTime": 2071387335,
            "ordering": 0,
            "pinned": false,
            "title": "Nextcloud",
            "unreadCount": 10,
            "updateErrorCount": 0,
            "url": "http://localhost:8090/Nextcloud.rss"
        }
    ],
    "newestItemId": 190,
    "starredCount": 0
}

Checklist

lib/Db/Feed.php Outdated Show resolved Hide resolved
lib/Db/Feed.php Outdated Show resolved Hide resolved
@Grotax
Copy link
Member Author

Grotax commented Dec 19, 2024

@SMillerDev any idea what do about the failing tests? I tested it on my machine and the tests are fine. My system is a 64bit Manjaro though and my jq seems to know how to handle the output.

I can imagine that it fails because of 4090814535 which is bigger than max 32bit int.
It is a unique case of course since the feed we serve here in the test is far in the future.

Maybe if I print the jq version in actions I can get more information.

It worked before as string of course since those can be much longer

@Grotax Grotax force-pushed the updater/nextUpdateTime branch 2 times, most recently from ff32b7a to a63877e Compare December 20, 2024 10:21
@Grotax Grotax marked this pull request as ready for review December 20, 2024 10:32
@Grotax Grotax requested a review from SMillerDev December 20, 2024 10:34
@Grotax
Copy link
Member Author

Grotax commented Dec 20, 2024

I simply changed the date of the test to some years earlier, idk why jq in this environment can't handle 64bit numbers in theory it should be able to...

As a logical consequence at some point 32 bit clients or libraries that struggle with big numbers will have issues with this but I think they should be able to fix that.

@Grotax Grotax added enhancement API Impact API/Backend code labels Dec 20, 2024
@Grotax Grotax force-pushed the updater/nextUpdateTime branch 2 times, most recently from 59660f3 to ed9fef2 Compare December 22, 2024 09:54
@Grotax Grotax force-pushed the updater/nextUpdateTime branch from ed9fef2 to 37b7c2b Compare December 22, 2024 10:07
Copy link
Contributor

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

Thanks @Grotax, just one tweak.

lib/Db/Feed.php Outdated Show resolved Hide resolved
Co-authored-by: Sean Molenaar <[email protected]>
Signed-off-by: Benjamin Brahmer <[email protected]>
@Grotax Grotax force-pushed the updater/nextUpdateTime branch from bee8366 to 59b975f Compare December 22, 2024 11:21
@Grotax Grotax enabled auto-merge (rebase) December 22, 2024 11:22
@Grotax Grotax merged commit 02ffadb into master Dec 22, 2024
23 of 24 checks passed
@Grotax Grotax deleted the updater/nextUpdateTime branch December 22, 2024 11:24
Grotax added a commit that referenced this pull request Dec 30, 2024
Changed
- API add new field to Feed that indicates when the next update will be done "nextUpdateTime" (#2993)
- Change logic to update feed only if the nextUpdateTime has been reached (#2999)
- Add setting to disable the usage of nextUpdateTime (#2999)

Fixed
- `TypeError: this.$refs.actions.$refs.menuButton is undefined` when tabbing through feeds and folders (#3004)

Signed-off-by: Benjamin Brahmer <[email protected]>
@Grotax Grotax mentioned this pull request Dec 30, 2024
Grotax added a commit that referenced this pull request Dec 30, 2024
Changed
- API add new field to Feed that indicates when the next update will be done "nextUpdateTime" (#2993)
- Change logic to update feed only if the nextUpdateTime has been reached (#2999)
- Add setting to disable the usage of nextUpdateTime (#2999)

Fixed
- `TypeError: this.$refs.actions.$refs.menuButton is undefined` when tabbing through feeds and folders (#3004)

Signed-off-by: Benjamin Brahmer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Impact API/Backend code enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants