Add update checks for new Dangerzone versions #466

apyrgio · 2023-07-04T17:40:25Z

This is a PR that brings notification updates for new Dangerzone versions. It follows most of the workflow in issue #189, but uses notification bubbles instead of pop-ups, as the main method of grabbing the user's attention.

This PR is mostly complete, but there are some minor issues that still persist:

There are some medium priority FIXME/TODO comments in the code. These can be fixed in a subsequent PR, since this one has grown too large.
The test coverage can be improved, especially for the Qt part of the code. Again, this can take place in a subsequent PR.

Closes #189

deeplow · 2023-07-05T06:40:53Z

Some tests are failing on Windows/MacOS, but the same thing applies to Add "change document selection" button #439

Turns out the recursion issue was caused by parallel tests in Docker. I solved the issue on the other PR by completely removing parallel tests (they have caused issues in the past as well). See this diff.

deeplow

Extra things to think about:

on linux the check for updates will appear disabled by default. We should mark it as checked and say that it's updated via the package manager.
the hamburger menu probably needs to be a bit bigger and equally spaced from each margin
the "new version available" option in the menu should somehow be highlighted (see how Tor Browser shows it).

dangerzone/settings.py

dangerzone/gui/updater.py

dangerzone/gui/main_window.py

tests/test_updater.py

tests/__init__.py

apyrgio · 2023-07-11T12:50:49Z

Hey, thanks a lot for the detailed review :-).

Extra things to think about:

* on linux the check for updates will appear disabled by default. We should mark it as checked and say that it's updated via the package manager.

I think it's still worth having this option in two cases:

If the user has installed a .deb / .rpm (e.g., because they don't want to use our repos.
If the user has installed Dangerzone through an unofficial package (think AUR), which may not be fully maintained.

* the hamburger menu probably needs to be a bit bigger and equally spaced from each margin

Definitely, I agree.

* the "new version available" option in the menu should somehow be highlighted (see how Tor Browser shows it).

Yeap, I agree as well.

deeplow · 2023-07-11T14:37:51Z

I think it's still worth having this option in two cases:

If the user has installed a .deb / .rpm (e.g., because they don't want to use our repos.

If the user has installed Dangerzone through an unofficial package (think AUR), which may not be fully maintained.

OK! You mean having different behaviors on RPM vs DEB versus unnoficial ones, right?

apyrgio · 2023-07-12T19:17:59Z

I think it's still worth having this option in two cases:

If the user has installed a .deb / .rpm (e.g., because they don't want to use our repos.

If the user has installed Dangerzone through an unofficial package (think AUR), which may not be fully maintained.

OK! You mean having different behaviors on RPM vs DEB versus unnoficial ones, right?

Even for the official RPMs / DEBs, if the user just downloads and installs them manually, they will not learn about updates. Hopefully, this will be a rare situation...

apyrgio · 2023-07-17T16:01:18Z

@deeplow Reminder, I have re-implemented your commit for the cooldown period, because I had made some changes to the code and couldn't rebase them on top of your commit. You'll see that the logic and the tests are a bit different in places, so please take a cursory look.

dangerzone/gui/updater.py

dangerzone/gui/main_window.py

dangerzone/gui/updater.py

Makefile

deeplow · 2023-07-20T10:53:13Z

I have addded equal padding on the top-right margins and changed the icon:

How does this look?

deeplow · 2023-07-20T11:51:53Z

I'm pushing a commit to center the dialog because it looks better:

Before	After

Run tests sequentially, because in subsequent commits we will add Qt tests that do not play nice when `pytest` creates new processes [1]. Also, remove the pytest wrapper, whose main task was to decide if tests can run in parallel [2]. [1]: https://bugreports.qt.io/projects/PYSIDE/issues/PYSIDE-2393 [2]: #217

Pass the X11 socket of the Linux CI runners to the container where our CI tests run, with the `-g` flag of `dev_scripts/env.py`. By having a working X11 socket, we can run GUI tests. Prior to this fix, we would encounter this error: tests/gui/test_main_window.py::test_change_document_button qt.qpa.xcb: could not connect to display qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found. This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem. Available platform plugins are: xcb, offscreen, wayland-egl, wayland, eglfs, vnc, minimalegl, vkkhrdisplay, linuxfb, minimal. Another alternative we considered was to use the `QT_QPA_PLATFORM=offscreen` environment variable. This alternative works, but it's less close to the end-user's environment, so we decided in favor of the approach above.

Add some settings prefixed with `"updater_"`, which will be used for updates later on.

Add the following two features in the Settings class: 1. Add a way to save the settings, if the contents of a key have changed. 2. Add a way to get all the updater settings, by getting fetching the keys that start with `"updater_"`.

Get the default settings of Dangezone for the current version, without having to instantiate the Settings class. Note that instantiating the Settings class also writes the settings to the underlying `settings.json` file, and there are cases where we don't want this behavior.

Add a new Python module called "updater", which contains the logic for prompting the user to enable updates, and checking our GitHub releases for new updates. This class has some light dependency to Qt functionality, since it needs to: * Show a prompt to the user, * Run update checks asynchronously in a Qt thread, * Provide the main window with the result of the update check Refs #189

Add a Pytest fixture that returns an UpdaterThread instance which has its own unique settings directory. Note that the UpdaterThread instance needs to be slightly nerfed, so that it doesn't rely on Qt functionality or any isolation providers.

Add three status icon for the hamburger menu: * hamburger_menu.svg: The typical hamburger menu. Taken from https://commons.wikimedia.org/wiki/File:Hamburger_icon.svg, which is in the public domain. * hamburger_menu_update_error.svg: A hamburger menu with a red notification bubble on the top right corner. * hamburger_menu_update_success.svg: A hamburger menu with a green notification bubble on the top right corner.

Factor out some parts of the Alert class into a more generic dialog class. This class will be used for a new type of dialog that we will introduce in a subsequent commit. Note that this commit does not alter the functionality of the Alert class.

Add a Qt widget called "CollapsibleBox", in order to build sections that you can hide/show with a single click. There is no native widget for this functionality, so we borrow some code from a StackOverflow user: https://stackoverflow.com/a/52617714

Add a dialog that we will show for update-related tasks. This dialog has a different layout than the Alert class: it has a message, followed by a widget that the user chooses (can be a text box or collapsible element), and then one last message.

deeplow

Good to merge. The remaining TODOs were agreed to tb be tackled later as not to further delay is rather large PR.

Add a hamburger button in the main window of Dangerzone, that will be the entry point for update information. Whenever a new update is released, users will see a green notification bubble. If an update error happens, they will see a red notification bubble. In the hamburger menu, users have the option to enable or disable update checks. Depending on the update check status, users will see in a pop-up dialog more info about the new update or the error. Closes #189

Add a very rudimentary test for GUI update logic. Refs #290

deeplow self-requested a review July 5, 2023 05:47

deeplow requested changes Jul 10, 2023

View reviewed changes

deeplow force-pushed the 189-updates branch from 14fa151 to 1fd9348 Compare July 12, 2023 16:41

apyrgio force-pushed the 189-updates branch from 1fd9348 to 89f4dd0 Compare July 17, 2023 15:51

apyrgio force-pushed the 189-updates branch from 751441a to 20abdbf Compare July 17, 2023 16:49

deeplow reviewed Jul 18, 2023

View reviewed changes

dangerzone/gui/updater.py Outdated Show resolved Hide resolved

deeplow reviewed Jul 19, 2023

View reviewed changes

dangerzone/gui/main_window.py Outdated Show resolved Hide resolved

deeplow reviewed Jul 19, 2023

View reviewed changes

dangerzone/gui/main_window.py Outdated Show resolved Hide resolved

deeplow reviewed Jul 19, 2023

View reviewed changes

dangerzone/gui/main_window.py Outdated Show resolved Hide resolved

deeplow reviewed Jul 19, 2023

View reviewed changes

dangerzone/gui/updater.py Outdated Show resolved Hide resolved

apyrgio force-pushed the 189-updates branch from b9207b6 to 014c504 Compare July 19, 2023 13:22

apyrgio marked this pull request as ready for review July 20, 2023 07:12

deeplow reviewed Jul 20, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

apyrgio force-pushed the 189-updates branch from 16482fb to 230bed5 Compare July 24, 2023 11:17

apyrgio added 9 commits July 24, 2023 14:22

Add default settings for Dangerzone updater

2df459b

Add some settings prefixed with `"updater_"`, which will be used for updates later on.

Make it easier to get and save updater settings

266addb

Add the following two features in the Settings class: 1. Add a way to save the settings, if the contents of a key have changed. 2. Add a way to get all the updater settings, by getting fetching the keys that start with `"updater_"`.

tests: Add a Pytest fixture for Updater

f6e9458

Add a Pytest fixture that returns an UpdaterThread instance which has its own unique settings directory. Note that the UpdaterThread instance needs to be slightly nerfed, so that it doesn't rely on Qt functionality or any isolation providers.

tests: Add tests for update logic

47171a2

apyrgio added 3 commits July 24, 2023 14:22

Allow more types of dialogs

20a25f1

Factor out some parts of the Alert class into a more generic dialog class. This class will be used for a new type of dialog that we will introduce in a subsequent commit. Note that this commit does not alter the functionality of the Alert class.

gui: Add Update Dialog

58c5fc8

Add a dialog that we will show for update-related tasks. This dialog has a different layout than the Alert class: it has a message, followed by a widget that the user chooses (can be a text box or collapsible element), and then one last message.

deeplow approved these changes Jul 24, 2023

View reviewed changes

apyrgio added 3 commits July 24, 2023 16:49

Add pytest-qt test dependency

ca81b4a

tests: Add Qt test for updates

47b3371

Add a very rudimentary test for GUI update logic. Refs #290

apyrgio force-pushed the 189-updates branch from 230bed5 to 47b3371 Compare July 24, 2023 13:54

apyrgio merged commit 47b3371 into main Jul 24, 2023

apyrgio mentioned this pull request Jul 25, 2023

Add full test coverage for update notifications #484

Merged

apyrgio deleted the 189-updates branch July 28, 2023 08:05

deeplow mentioned this pull request Aug 2, 2023

0.4.2 build on ubuntu 22.04 can't import PySide2.QtSvg #500

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add update checks for new Dangerzone versions #466

Add update checks for new Dangerzone versions #466

apyrgio commented Jul 4, 2023 •

edited

Loading

deeplow commented Jul 5, 2023

deeplow left a comment

apyrgio commented Jul 11, 2023

deeplow commented Jul 11, 2023

apyrgio commented Jul 12, 2023

apyrgio commented Jul 17, 2023

deeplow commented Jul 20, 2023

deeplow commented Jul 20, 2023

deeplow left a comment

Add update checks for new Dangerzone versions #466

Add update checks for new Dangerzone versions #466

Conversation

apyrgio commented Jul 4, 2023 • edited Loading

deeplow commented Jul 5, 2023

deeplow left a comment

Choose a reason for hiding this comment

apyrgio commented Jul 11, 2023

deeplow commented Jul 11, 2023

apyrgio commented Jul 12, 2023

apyrgio commented Jul 17, 2023

deeplow commented Jul 20, 2023

deeplow commented Jul 20, 2023

deeplow left a comment

Choose a reason for hiding this comment

apyrgio commented Jul 4, 2023 •

edited

Loading