From 61c8f2a6add12c99e87af9f7d0390e41803dacdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Sat, 1 Mar 2025 15:50:32 +0100 Subject: [PATCH] Replace the `updater_check` setting by `updater_check_all` This new setting triggers the same user prompts, but the actual meaning of it differs, since users will now be accepting to upgrade the container image rather than just checking for new releases. Changing the name of the setting will trigger this prompt for all users, effectively ensuring they want their image to be automatically upgraded. --- dangerzone/gui/main_window.py | 4 ++-- dangerzone/gui/updater.py | 2 +- dangerzone/settings.py | 2 +- dangerzone/updater/releases.py | 4 ++-- docs/developer/updates.md | 38 ++++++---------------------------- tests/gui/test_main_window.py | 16 +++++++------- tests/gui/test_updater.py | 28 ++++++++++++------------- 7 files changed, 34 insertions(+), 60 deletions(-) diff --git a/dangerzone/gui/main_window.py b/dangerzone/gui/main_window.py index a17cb1a..5924aae 100644 --- a/dangerzone/gui/main_window.py +++ b/dangerzone/gui/main_window.py @@ -163,7 +163,7 @@ class MainWindow(QtWidgets.QMainWindow): self.toggle_updates_action.triggered.connect(self.toggle_updates_triggered) self.toggle_updates_action.setCheckable(True) self.toggle_updates_action.setChecked( - bool(self.dangerzone.settings.get("updater_check")) + bool(self.dangerzone.settings.get("updater_check_all")) ) # Add the "Exit" action @@ -281,7 +281,7 @@ class MainWindow(QtWidgets.QMainWindow): def toggle_updates_triggered(self) -> None: """Change the underlying update check settings based on the user's choice.""" check = self.toggle_updates_action.isChecked() - self.dangerzone.settings.set("updater_check", check) + self.dangerzone.settings.set("updater_check_all", check) self.dangerzone.settings.save() def handle_docker_desktop_version_check( diff --git a/dangerzone/gui/updater.py b/dangerzone/gui/updater.py index f4dae0b..ece2619 100644 --- a/dangerzone/gui/updater.py +++ b/dangerzone/gui/updater.py @@ -106,7 +106,7 @@ class UpdaterThread(QtCore.QThread): should_check = self.prompt_for_checks() if should_check is not None: self.dangerzone.settings.set( - "updater_check", should_check, autosave=True + "updater_check_all", should_check, autosave=True ) return bool(should_check) diff --git a/dangerzone/settings.py b/dangerzone/settings.py index 619b458..8613ce7 100644 --- a/dangerzone/settings.py +++ b/dangerzone/settings.py @@ -33,7 +33,7 @@ class Settings: "open": True, "open_app": None, "safe_extension": SAFE_EXTENSION, - "updater_check": None, + "updater_check_all": None, "updater_last_check": None, # last check in UNIX epoch (secs since 1970) # FIXME: How to invalidate those if they change upstream? "updater_latest_version": get_version(), diff --git a/dangerzone/updater/releases.py b/dangerzone/updater/releases.py index 82cb05f..7fc842b 100644 --- a/dangerzone/updater/releases.py +++ b/dangerzone/updater/releases.py @@ -114,14 +114,14 @@ def should_check_for_releases(settings: Settings) -> bool: * Thus we will always show to the user the cached info about the new version, and won't make a new update check. """ - check = settings.get("updater_check") + check = settings.get("updater_check_all") log.debug("Checking platform type") # TODO: Disable updates for Homebrew installations. if platform.system() == "Linux" and not getattr(sys, "dangerzone_dev", False): log.debug("Running on Linux, disabling updates") if not check: # if not overidden by user - settings.set("updater_check", False, autosave=True) + settings.set("updater_check_all", False, autosave=True) return False log.debug("Checking if first run of Dangerzone") diff --git a/docs/developer/updates.md b/docs/developer/updates.md index e5e1197..27cbc60 100644 --- a/docs/developer/updates.md +++ b/docs/developer/updates.md @@ -11,8 +11,7 @@ https://github.com/freedomofpress/dangerzone/wiki/Updates ## Design overview -This feature introduces a hamburger icon that will be visible across almost all -of the Dangerzone windows. This will be used to notify the users about updates. +A hamburger icon is visible across almost all of the Dangerzone windows, and is used to notify the users when there are new releases. ### First run @@ -21,8 +20,7 @@ _We detect it's the first time Dangerzone runs because the Add the following keys in our `settings.json` file. -* `"updater_check": None`: Whether to check for updates or not. `None` means - that the user has not decided yet, and is the default. +* `"updater_check_all": True`: Whether or not to check and apply independent container updates and check for new releases. * `"updater_last_check": None`: The last time we checked for updates (in seconds from Unix epoch). None means that we haven't checked yet. * `"updater_latest_version": "0.4.2"`: The latest version that the Dangerzone @@ -32,43 +30,19 @@ Add the following keys in our `settings.json` file. * `"updater_errors: 0`: The number of update check errors that we have encountered in a row. -Note: - -* If on Linux, make `"updater_check": False`, since we normally have - other update channels for these platforms. +Previously, `"updater_check"` was used to determine if we should check for new releases, and has been replaced by `"updater_check_all"` when adding support for independent container updates. ### Second run _We detect it's the second time Dangerzone runs because -`settings["updater_check"] is not None and settings["updater_last_check"] is +`settings["updater_check_all"] is not None and settings["updater_last_check"] is None`._ -Before starting up the main window, show this window: - -* Title: Dangerzone Updater -* Body: - - > Do you want Dangerzone to automatically check for updates? - > - > If you accept, Dangerzone will check the latest releases page in github.com - > on startup. Otherwise it will make no network requests and won't inform you - > about new releases. - > - > If you prefer another way of getting notified about new releases, we suggest adding - > to your RSS reader our [Mastodon feed](https://fosstodon.org/@dangerzone.rss). For more information - > about updates, check [this webpage](https://github.com/freedomofpress/dangerzone/wiki/Updates). - -* Buttons: - - Check Automaticaly: Store `settings["updater_check"] = True` - - Don't Check: Store `settings["updater_check"] = False` - -Note: -* Users will be able to change their choice from the hamburger menu, which will - contain an entry called "Check for updates", that users can check and uncheck. +Before starting up the main window, the user is prompted if they want to enable update checks. ### Subsequent runs -_We perform the following only if `settings["updater_check"] == True`._ +_We perform the following only if `settings["updater_check_all"] == True`._ 1. Spawn a new thread so that we don't block the main window. 2. Check if we have cached information about a release (version and changelog). diff --git a/tests/gui/test_main_window.py b/tests/gui/test_main_window.py index 75a11d3..4422af1 100644 --- a/tests/gui/test_main_window.py +++ b/tests/gui/test_main_window.py @@ -97,7 +97,7 @@ def test_default_menu( updater: UpdaterThread, ) -> None: """Check that the default menu entries are in order.""" - updater.dangerzone.settings.set("updater_check", True) + updater.dangerzone.settings.set("updater_check_all", True) window = MainWindow(updater.dangerzone) menu_actions = window.hamburger_button.menu().actions() @@ -115,7 +115,7 @@ def test_default_menu( toggle_updates_action.trigger() assert not toggle_updates_action.isChecked() - assert updater.dangerzone.settings.get("updater_check") is False + assert updater.dangerzone.settings.get("updater_check_all") is False def test_no_update( @@ -128,12 +128,12 @@ def test_no_update( # Check that when no update is detected, e.g., due to update cooldown, an empty # report is received that does not affect the menu entries. curtime = int(time.time()) - updater.dangerzone.settings.set("updater_check", True) + updater.dangerzone.settings.set("updater_check_all", True) updater.dangerzone.settings.set("updater_errors", 9) updater.dangerzone.settings.set("updater_last_check", curtime) expected_settings = default_updater_settings() - expected_settings["updater_check"] = True + expected_settings["updater_check_all"] = True expected_settings["updater_errors"] = 0 # errors must be cleared expected_settings["updater_last_check"] = curtime @@ -166,7 +166,7 @@ def test_update_detected( ) -> None: """Test that a newly detected version leads to a notification to the user.""" - qt_updater.dangerzone.settings.set("updater_check", True) + qt_updater.dangerzone.settings.set("updater_check_all", True) qt_updater.dangerzone.settings.set("updater_last_check", 0) qt_updater.dangerzone.settings.set("updater_errors", 9) @@ -198,7 +198,7 @@ def test_update_detected( # Check that the settings have been updated properly. expected_settings = default_updater_settings() - expected_settings["updater_check"] = True + expected_settings["updater_check_all"] = True expected_settings["updater_last_check"] = qt_updater.dangerzone.settings.get( "updater_last_check" ) @@ -279,7 +279,7 @@ def test_update_error( ) -> None: """Test that an error during an update check leads to a notification to the user.""" # Test 1 - Check that the first error does not notify the user. - qt_updater.dangerzone.settings.set("updater_check", True) + qt_updater.dangerzone.settings.set("updater_check_all", True) qt_updater.dangerzone.settings.set("updater_last_check", 0) qt_updater.dangerzone.settings.set("updater_errors", 0) @@ -306,7 +306,7 @@ def test_update_error( # Check that the settings have been updated properly. expected_settings = default_updater_settings() - expected_settings["updater_check"] = True + expected_settings["updater_check_all"] = True expected_settings["updater_last_check"] = qt_updater.dangerzone.settings.get( "updater_last_check" ) diff --git a/tests/gui/test_updater.py b/tests/gui/test_updater.py index 114dac5..a0ee4ad 100644 --- a/tests/gui/test_updater.py +++ b/tests/gui/test_updater.py @@ -106,7 +106,7 @@ def test_post_0_4_2_settings( def test_linux_no_check(updater: UpdaterThread, monkeypatch: MonkeyPatch) -> None: """Ensure that Dangerzone on Linux does not make any update check.""" expected_settings = default_updater_settings() - expected_settings["updater_check"] = False + expected_settings["updater_check_all"] = False expected_settings["updater_last_check"] = None # XXX: Simulate Dangerzone installed via package manager. @@ -124,7 +124,7 @@ def test_user_prompts(updater: UpdaterThread, mocker: MockerFixture) -> None: # When Dangerzone runs for the first time, users should not be asked to enable # updates. expected_settings = default_updater_settings() - expected_settings["updater_check"] = None + expected_settings["updater_check_all"] = None expected_settings["updater_last_check"] = 0 assert updater.should_check_for_updates() is False assert settings.get_updater_settings() == expected_settings @@ -139,14 +139,14 @@ def test_user_prompts(updater: UpdaterThread, mocker: MockerFixture) -> None: # Check disabling update checks. prompt_mock().launch.return_value = False # type: ignore [attr-defined] - expected_settings["updater_check"] = False + expected_settings["updater_check_all"] = False assert updater.should_check_for_updates() is False assert settings.get_updater_settings() == expected_settings - # Reset the "updater_check" field and check enabling update checks. - settings.set("updater_check", None) + # Reset the "updater_check_all" field and check enabling update checks. + settings.set("updater_check_all", None) prompt_mock().launch.return_value = True # type: ignore [attr-defined] - expected_settings["updater_check"] = True + expected_settings["updater_check_all"] = True assert updater.should_check_for_updates() is True assert settings.get_updater_settings() == expected_settings @@ -156,7 +156,7 @@ def test_user_prompts(updater: UpdaterThread, mocker: MockerFixture) -> None: # checks. prompt_mock().side_effect = RuntimeError("Should not be called") # type: ignore [attr-defined] for check in [True, False]: - settings.set("updater_check", check) + settings.set("updater_check_all", check) assert updater.should_check_for_updates() == check @@ -211,7 +211,7 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - """Make sure Dangerzone only checks for updates every X hours""" settings = updater.dangerzone.settings - settings.set("updater_check", True) + settings.set("updater_check_all", True) settings.set("updater_last_check", 0) # Mock some functions before the tests start @@ -393,7 +393,7 @@ def test_update_check_prompt( # Test 2 - Check that when the user chooses to enable update checks, we # store that decision in the settings. - settings.set("updater_check", None, autosave=True) + settings.set("updater_check_all", None, autosave=True) def click_ok() -> None: dialog = qt_updater.dangerzone.app.activeWindow() @@ -403,11 +403,11 @@ def test_update_check_prompt( res = qt_updater.should_check_for_updates() assert res is True - assert settings.get("updater_check") is True + assert settings.get("updater_check_all") is True # Test 3 - Same as the previous test, but check that clicking on cancel stores the # opposite decision. - settings.set("updater_check", None) # type: ignore [unreachable] + settings.set("updater_check_all", None) # type: ignore [unreachable] def click_cancel() -> None: dialog = qt_updater.dangerzone.app.activeWindow() @@ -417,11 +417,11 @@ def test_update_check_prompt( res = qt_updater.should_check_for_updates() assert res is False - assert settings.get("updater_check") is False + assert settings.get("updater_check_all") is False # Test 4 - Same as the previous test, but check that clicking on "X" does not store # any decision. - settings.set("updater_check", None, autosave=True) + settings.set("updater_check_all", None, autosave=True) def click_x() -> None: dialog = qt_updater.dangerzone.app.activeWindow() @@ -431,4 +431,4 @@ def test_update_check_prompt( res = qt_updater.should_check_for_updates() assert res is False - assert settings.get("updater_check") is None + assert settings.get("updater_check_all") is None