From 7a36ee23f5c299828cb21212fcf749e89113711e Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 26 Jul 2022 16:47:32 -0700 Subject: [PATCH] app: Drop optimization that skips setup process When an app does not implement module setup() method, trying to get setup version automatically results in App being updated to latest version. This optimization seems hardly used and does not work when setup() is moved to App from module level. Remove it. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/app.py | 10 +--------- plinth/modules/first_boot/middleware.py | 8 ++++++++ plinth/tests/test_app.py | 11 ----------- 3 files changed, 9 insertions(+), 20 deletions(-) diff --git a/plinth/app.py b/plinth/app.py index df90dc2aa..63f0b4e4d 100644 --- a/plinth/app.py +++ b/plinth/app.py @@ -51,6 +51,7 @@ class App: class SetupState(enum.Enum): """Various states of app being setup.""" + NEEDS_SETUP = 'needs-setup' NEEDS_UPDATE = 'needs-update' UP_TO_DATE = 'up-to-date' @@ -139,15 +140,6 @@ class App: if current_version and self.info.version <= current_version: return self.SetupState.UP_TO_DATE - # If an app needs installing/updating but no setup method is available, - # then automatically set version. - # - # Minor violation of 'get' only discipline for convenience. - module = sys.modules[self.__module__] - if not hasattr(module, 'setup'): - self.set_setup_version(self.info.version) - return self.SetupState.UP_TO_DATE - if not current_version: return self.SetupState.NEEDS_SETUP diff --git a/plinth/modules/first_boot/middleware.py b/plinth/modules/first_boot/middleware.py index 9135bf62e..2ba122b4a 100644 --- a/plinth/modules/first_boot/middleware.py +++ b/plinth/modules/first_boot/middleware.py @@ -11,6 +11,7 @@ from django.http.response import HttpResponseRedirect from django.urls import reverse from django.utils.deprecation import MiddlewareMixin +from plinth import setup from plinth.modules import first_boot from plinth.utils import is_user_admin @@ -33,6 +34,13 @@ class FirstBootMiddleware(MiddlewareMixin): if user_requests_help: return + # Don't interfere with first setup progress page. When first setup is + # still running, no apps may have provided the first boot steps. This + # will result in first boot wizard getting marked as completed + # prematurely. + if setup.is_first_setup_running: + return + firstboot_completed = first_boot.is_completed() user_requests_firstboot = first_boot.is_firstboot_url(request.path) diff --git a/plinth/tests/test_app.py b/plinth/tests/test_app.py index 3ce28e5fc..2369f0eb7 100644 --- a/plinth/tests/test_app.py +++ b/plinth/tests/test_app.py @@ -141,22 +141,11 @@ def test_setup_state(): """Test retrieving the current setup state of the app.""" app = AppSetupTest() app.info.version = 3 - module = sys.modules[__name__] app.set_setup_version(3) assert app.get_setup_state() == App.SetupState.UP_TO_DATE assert not app.needs_setup() - app.set_setup_version(0) - try: - delattr(module, 'setup') - except AttributeError: - pass - assert app.get_setup_state() == App.SetupState.UP_TO_DATE - assert not app.needs_setup() - assert app.get_setup_version() == 3 - - setattr(module, 'setup', True) app.set_setup_version(0) assert app.get_setup_state() == App.SetupState.NEEDS_SETUP assert app.needs_setup()