Skip to content

Conversation

@leolost2605
Copy link
Member

@leolost2605 leolost2605 commented Aug 3, 2025

This PR streamlines the refresh flow. We now do everything in the update manager which will automatically start the timeout if we're less than 24 hours from the last update or else refresh immediately.
We also now only refresh after the network comes available if we tried to refresh while the network wasn't available.

This doesn't include but prepares for more behavioral changes proposed in #2214
Also prepares for some cleanup of the refresh action enabled logic which is left to another PR

cancel_updates was removed because it was only used to cancel updates

  1. when disabling automatic updates which isn't necessary anymore since the user now sees that the apps are updating and can cancel the update manually if they wish to
  2. when the app window was closed which I don't get anyways why that was done? This could falsely cancel ongoing cache refreshes and automatic updates

@leolost2605 leolost2605 changed the title Refresh less often and streamline refresh flow Streamline refresh flow Aug 3, 2025
@leolost2605 leolost2605 marked this pull request as ready for review August 3, 2025 15:26
@leolost2605 leolost2605 requested a review from a team August 3, 2025 15:36
@zeebok
Copy link
Contributor

zeebok commented Aug 4, 2025

can cancel the update manually if they wish to

This actually making me wonder why there isn't a "Cancel All" button to help instead of having to click Cancel for every app that is waiting to update

@leolost2605
Copy link
Member Author

you actually don't have to click cancel for every app, you only have to click cancel for the app that is currently updating which will automatically drop out of the update loop. you can't even click cancel for the other apps since they aren't updating yet and their buttons should be insensitive or do you observe some different behavior?

@zeebok
Copy link
Contributor

zeebok commented Aug 4, 2025

I must be remembering old behavior! Just confirmed you are right. Sweet!

} else {
update_manager.cancel_updates (true);
// TODO: Follow up: think about this? Only update all and handle in manager?
update_manager.refresh.begin ();
Copy link
Member

Choose a reason for hiding this comment

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

Yeah I agree we should just listen for the gsettings change in update manager

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

This all makes sense to me. I think we probably will want to replace those timeouts with a systemd timer like we did here: elementary/settings-daemon#200

I just have a couple of minor comments but nothing blocking. Happy to merge this. Nice cleanup :)

@leolost2605 leolost2605 merged commit 24132e2 into main Aug 9, 2025
4 checks passed
@leolost2605 leolost2605 deleted the leolost/less-refresh branch August 9, 2025 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants