diff --git a/console_conf/controllers/chooser.py b/console_conf/controllers/chooser.py index db0e76b5..8cb4e707 100644 --- a/console_conf/controllers/chooser.py +++ b/console_conf/controllers/chooser.py @@ -26,6 +26,7 @@ log = logging.getLogger("console_conf.controllers.chooser") class RecoveryChooserBaseController(BaseController): + def __init__(self, app): super().__init__(app) self.model = app.base_model @@ -34,25 +35,57 @@ class RecoveryChooserBaseController(BaseController): # exit without taking any action self.app.exit() + def back(self): + self.app.prev_sceen() + class RecoveryChooserController(RecoveryChooserBaseController): + + def __init__(self, app): + super().__init__(app) + self._model_view, self._all_view = self._make_views() + # one of the current views + self._current_view = None + def start_ui(self): + # current view is preserved, so going back comes back to the right + # screen + if self._current_view is None: + # right off the start, default to all-systems view + self._current_view = self._all_view + # unless we can show the current model + if self._model_view is not None: + self._current_view = self._model_view + + self.ui.set_body(self._current_view) + + def _make_views(self): + current_view = None if self.model.current and self.model.current.actions: # only when we have a current system and it has actions available - more = len(self.model.systems) > 1 - view = ChooserCurrentSystemView(self, self.model.current, - has_more=more) - else: - view = ChooserView(self, self.model.systems) - self.ui.set_body(view) + current_view = ChooserCurrentSystemView(self, + self.model.current, + has_more=more) + + all_view = ChooserView(self, self.model.systems) + return current_view, all_view def select(self, system, action): self.model.select(system, action) self.app.next_screen() def more_options(self): - self.ui.set_body(ChooserView(self, self.model.systems)) + self._current_view = self._all_view + self.ui.set_body(self._all_view) + + def back(self): + if self._current_view == self._all_view and \ + self._model_view is not None: + # back in the all-systems screen goes back to the current model + # screen, provided we have one + self._current_view = self._model_view + self.ui.set_body(self._current_view) class RecoveryChooserConfirmController(RecoveryChooserBaseController): @@ -65,3 +98,7 @@ class RecoveryChooserConfirmController(RecoveryChooserBaseController): # output the choice self.app.respond(self.model.selection) self.app.exit() + + def back(self): + self.model.unselect() + self.app.prev_screen() diff --git a/console_conf/controllers/tests/test_chooser.py b/console_conf/controllers/tests/test_chooser.py index 42b9e0ab..d38e63e8 100644 --- a/console_conf/controllers/tests/test_chooser.py +++ b/console_conf/controllers/tests/test_chooser.py @@ -34,14 +34,18 @@ class MockedApplication: opts = None -def make_app(): +def make_app(model=None): app = MockedApplication() app.ui = mock.Mock() - app.base_model = mock.Mock() + if model is not None: + app.base_model = model + else: + app.base_model = mock.Mock() app.context = Context.new(app) app.exit = mock.Mock() app.respond = mock.Mock() app.next_screen = mock.Mock() + app.prev_screen = mock.Mock() return app @@ -92,12 +96,15 @@ def make_model(): class TestChooserConfirmController(unittest.TestCase): - def test_abort(self): + def test_back(self): app = make_app() c = RecoveryChooserConfirmController(app) - c.cancel() + c.model = mock.Mock(selection='selection') + c.back() app.respond.assert_not_called() - app.exit.assert_called() + app.exit.assert_not_called() + app.prev_screen.assert_called() + c.model.unselect.assert_called() def test_confirm(self): app = make_app() @@ -119,17 +126,9 @@ class TestChooserConfirmController(unittest.TestCase): class TestChooserController(unittest.TestCase): - def test_abort(self): - app = make_app() - c = RecoveryChooserController(app) - c.cancel() - app.respond.assert_not_called() - app.exit.assert_called() - def test_select(self): - app = make_app() + app = make_app(model=make_model()) c = RecoveryChooserController(app) - c.model = make_model() c.select(c.model.systems[0], c.model.systems[0].actions[0]) exp = SelectedSystemAction(system=c.model.systems[0], @@ -140,50 +139,84 @@ class TestChooserController(unittest.TestCase): app.respond.assert_not_called() app.exit.assert_not_called() - @mock.patch('console_conf.controllers.chooser.ChooserCurrentSystemView') - @mock.patch('console_conf.controllers.chooser.ChooserView') + @mock.patch('console_conf.controllers.chooser.ChooserCurrentSystemView', + return_value='current') + @mock.patch('console_conf.controllers.chooser.ChooserView', + return_value='all') def test_current_ui_first(self, cv, ccsv): - app = make_app() + app = make_app(model=make_model()) c = RecoveryChooserController(app) - c.model = make_model() - - c.start_ui() + c.ui.start_ui = mock.Mock() # current system view is constructed ccsv.assert_called_with(c, c.model.current, has_more=True) - # but the all systems view is not - cv.assert_not_called() - ccsv.reset_mock() + # as well as all systems view + cv.assert_called_with(c, c.model.systems) + c.start_ui() + c.ui.set_body.assert_called_with('current') # user selects more options and the view is replaced c.more_options() - # we get the all systems view now - cv.assert_called_with(c, c.model.systems) - # and the current system view was not constructed - ccsv.assert_not_called() + c.ui.set_body.assert_called_with('all') - @mock.patch('console_conf.controllers.chooser.ChooserCurrentSystemView') - @mock.patch('console_conf.controllers.chooser.ChooserView') - def test_only_one_and_current(self, cv, ccsv): - app = make_app() + @mock.patch('console_conf.controllers.chooser.ChooserCurrentSystemView', + return_value='current') + @mock.patch('console_conf.controllers.chooser.ChooserView', + return_value='all') + def test_current_current_all_there_and_back(self, cv, ccsv): + app = make_app(model=make_model()) c = RecoveryChooserController(app) - c.model = RecoverySystemsModel.from_systems([model2_current]) + c.ui.start_ui = mock.Mock() + # sanity + ccsv.assert_called_with(c, c.model.current, has_more=True) + cv.assert_called_with(c, c.model.systems) c.start_ui() - # current system view is constructed - ccsv.assert_called_with(c, c.model.current, has_more=False) - # but the all systems view is not - cv.assert_not_called() + c.ui.set_body.assert_called_with('current') + # user selects more options and the view is replaced + c.more_options() + c.ui.set_body.assert_called_with('all') + # go back now + c.back() + c.ui.set_body.assert_called_with('current') + # nothing + c.back() + c.ui.set_body.not_called() - @mock.patch('console_conf.controllers.chooser.ChooserCurrentSystemView') - @mock.patch('console_conf.controllers.chooser.ChooserView') - def test_all_systems_first_no_current(self, cv, ccsv): - app = make_app() + @mock.patch('console_conf.controllers.chooser.ChooserCurrentSystemView', + return_value='current') + @mock.patch('console_conf.controllers.chooser.ChooserView', + return_value='all') + def test_only_one_and_current(self, cv, ccsv): + model = RecoverySystemsModel.from_systems([model2_current]) + app = make_app(model=model) c = RecoveryChooserController(app) - c.model = RecoverySystemsModel.from_systems([model1_non_current]) + c.ui.start_ui = mock.Mock() + # both views are constructed + ccsv.assert_called_with(c, c.model.current, has_more=False) + cv.assert_called_with(c, c.model.systems) + + c.start_ui() + c.ui.set_body.assert_called_with('current') + # going back does nothing + c.back() + c.ui.set_body.not_called() + + @mock.patch('console_conf.controllers.chooser.ChooserCurrentSystemView', + return_value='current') + @mock.patch('console_conf.controllers.chooser.ChooserView', + return_value='all') + def test_all_systems_first_no_current(self, cv, ccsv): + model = RecoverySystemsModel.from_systems([model1_non_current]) + app = make_app(model=model) + c = RecoveryChooserController(app) + c.ui.start_ui = mock.Mock() + # sanity self.assertIsNone(c.model.current) - c.start_ui() - # we get the all systems view now + # we get the all-systems view now cv.assert_called() - # current system view is not constructed + # current system view is not constructed at all ccsv.assert_not_called() + + c.start_ui() + c.ui.set_body.assert_called_with('all') diff --git a/console_conf/controllers/welcome.py b/console_conf/controllers/welcome.py index fe709e84..8144e2fc 100644 --- a/console_conf/controllers/welcome.py +++ b/console_conf/controllers/welcome.py @@ -35,4 +35,9 @@ class WelcomeController(BaseController): class RecoveryChooserWelcomeController(WelcomeController): + welcome_view = ChooserWelcomeView + + def __init__(self, app): + super().__init__(app) + self.model = app.base_model diff --git a/console_conf/models/systems.py b/console_conf/models/systems.py index 759c376e..501cc109 100644 --- a/console_conf/models/systems.py +++ b/console_conf/models/systems.py @@ -95,6 +95,9 @@ class RecoverySystemsModel: def select(self, system, action): self._selection = SelectedSystemAction(system=system, action=action) + def unselect(self): + self._selection = None + @property def selection(self): return self._selection diff --git a/console_conf/models/tests/test_systems.py b/console_conf/models/tests/test_systems.py index 597004b7..ba94663d 100644 --- a/console_conf/models/tests/test_systems.py +++ b/console_conf/models/tests/test_systems.py @@ -209,6 +209,8 @@ class RecoverySystemsModelTests(unittest.TestCase): SelectedSystemAction( system=model.systems[1], action=model.systems[1].actions[0])) + model.unselect() + self.assertIsNone(model.selection) def test_to_response_stream(self): raw = json.dumps(self.reference) diff --git a/console_conf/ui/views/chooser.py b/console_conf/ui/views/chooser.py index da799035..886398d4 100644 --- a/console_conf/ui/views/chooser.py +++ b/console_conf/ui/views/chooser.py @@ -26,8 +26,8 @@ from urwid import ( ) from subiquitycore.ui.buttons import ( danger_btn, - reset_btn, forward_btn, + back_btn, ) from subiquitycore.ui.actionmenu import ( Action, @@ -47,23 +47,34 @@ from subiquitycore.view import BaseView log = logging.getLogger("console_conf.views.chooser") -class ChooserCurrentSystemView(BaseView): +class ChooserBaseView(BaseView): title = "Ubuntu Core" - def __init__(self, controller, current, has_more=False): - fmt = "Select one of available actions for \"{}\" by \"{}\"{}." - maybe_more = " or view all available systems" if has_more else "" - excerpt = fmt.format(current.model.display_name, - current.brand.display_name, - maybe_more) + def __init__(self, current, scr): + super().__init__(scr) + if current is not None: + self.title = current.model.display_name + + +def by_preferred_action_type(action): + """Order action entries by having the 'run' mode first, then 'recover', then + 'install', the rest is ordered alphabetically.""" + priority = {"run": 0, "recover": 1, "install": 2} + return (priority.get(action.mode, 100), action.title.lower()) + + +class ChooserCurrentSystemView(ChooserBaseView): + excerpt = "Select action:" + + def __init__(self, controller, current, has_more=False): self.controller = controller - log.debug('current system: %s', current) log.debug('more systems available: %s', has_more) + log.debug('current system: %s', current) actions = [] - for action in current.actions: - actions.append(forward_btn(label=action.title, + for action in sorted(current.actions, key=by_preferred_action_type): + actions.append(forward_btn(label=action.title.capitalize(), on_press=self._current_system_action, user_arg=(current, action))) @@ -75,16 +86,11 @@ class ChooserCurrentSystemView(BaseView): lb = ListBox(actions) - buttons = [ - reset_btn("ABORT", on_press=self.abort), - ] - - super().__init__(screen( - lb, - buttons=button_pile(buttons), - focus_buttons=False, - narrow_rows=True, - excerpt=excerpt)) + super().__init__(current, + screen( + lb, + narrow_rows=True, + excerpt=self.excerpt)) def _current_system_action(self, sender, arg): current, action = arg @@ -93,12 +99,11 @@ class ChooserCurrentSystemView(BaseView): def _more_options(self, sender): self.controller.more_options() - def abort(self, result): - self.controller.cancel() + def back(self, result): + self.controller.back() -class ChooserView(BaseView): - title = "Ubuntu Core" +class ChooserView(ChooserBaseView): excerpt = ("Select one of available recovery systems and a desired " "action to execute.") @@ -123,8 +128,8 @@ class ChooserView(BaseView): for s in systems: actions = [] log.debug('actions: %s', s.actions) - for act in s.actions: - actions.append(Action(label=act.title, + for act in sorted(s.actions, key=by_preferred_action_type): + actions.append(Action(label=act.title.capitalize(), value=act, enabled=True)) menu = ActionMenu(actions) @@ -133,7 +138,7 @@ class ChooserView(BaseView): Text(s.label), Text(s.model.display_name), Text(s.brand.display_name), - Text("(current)" if s.current else ""), + Text("(installed)" if s.current else ""), menu, ], menu) trows.append(srow) @@ -144,56 +149,68 @@ class ChooserView(BaseView): Pile([heading_table, systems_table]), ] - buttons = [ - reset_btn("ABORT", on_press=self.abort), - ] + buttons = [] + if controller.model.current is not None: + # back to options of current system + buttons.append(back_btn("BACK", on_press=self.back)) - super().__init__(screen( - rows=rows, - buttons=button_pile(buttons), - focus_buttons=False, - excerpt=self.excerpt)) + super().__init__(controller.model.current, + screen( + rows=rows, + buttons=button_pile(buttons), + focus_buttons=False, + excerpt=self.excerpt)) def _system_action(self, sender, action, system): self.controller.select(system, action) - def abort(self, result): - self.controller.cancel() + def back(self, result): + self.controller.back() -class ChooserConfirmView(BaseView): - title = "Ubuntu Core" - excerpt = ("Summary of the selected action.") +class ChooserConfirmView(ChooserBaseView): + + canned_summary = { + "run": "Continue running the system without any changes.", + "recover": ("You have requested to reboot the system into recovery " + "mode."), + "install": ("You are about to {action_lower} the system version " + "{version} for {model} from {publisher}.\n\n" + "This will remove all existing user data on the " + "device.\n\n" + "The system will reboot in the process."), + } + default_summary = ("You are about to execute action \"{action}\" using " + "system version {version} for device {model} from " + "{publisher}.\n\n" + "Make sure you understand the consequences of " + "performing this action.") def __init__(self, controller, selection): self.controller = controller buttons = [ danger_btn("CONFIRM", on_press=self.confirm), - reset_btn("ABORT", on_press=self.abort), - ] - using_summary = "System seed of device {} version {} from {}".format( - selection.system.model.display_name, - selection.system.label, - selection.system.brand.display_name - ) - summary = [ - TableRow([Text("Action:"), Color.info_error(Text( - selection.action.title))]), - TableRow([Text("Using:"), Text(using_summary)]), + back_btn("BACK", on_press=self.back), ] + fmt = self.canned_summary.get(selection.action.mode, + self.default_summary) + summary = fmt.format(action=selection.action.title, + action_lower=selection.action.title.lower(), + model=selection.system.model.display_name, + publisher=selection.system.brand.display_name, + version=selection.system.label) rows = [ - Pile([Text("")]), - Pile([TablePile(summary)]) + Text(summary), ] - super().__init__(screen( - rows=rows, - buttons=button_pile(buttons), - focus_buttons=False, - excerpt=self.excerpt)) - - def abort(self, result): - self.controller.cancel() + super().__init__(controller.model.current, + screen( + rows=rows, + buttons=button_pile(buttons), + focus_buttons=False)) def confirm(self, result): self.controller.confirm() + + def back(self, result): + self.controller.back() diff --git a/console_conf/ui/views/tests/__init__.py b/console_conf/ui/views/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/console_conf/ui/views/tests/test_chooser.py b/console_conf/ui/views/tests/test_chooser.py new file mode 100644 index 00000000..3b7b8037 --- /dev/null +++ b/console_conf/ui/views/tests/test_chooser.py @@ -0,0 +1,43 @@ +# Copyright 2020 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +import unittest + +from console_conf.models.systems import ( + SystemAction, + ) +from console_conf.ui.views.chooser import ( + by_preferred_action_type, + ) + + +class TestSorting(unittest.TestCase): + + def test_simple(self): + data = [ + SystemAction(title="b-other", mode="unknown"), + SystemAction(title="reinstall", mode="install"), + SystemAction(title="run normally", mode="run"), + SystemAction(title="a-other", mode="some-other"), + SystemAction(title="recover", mode="recover"), + ] + exp = [ + SystemAction(title="run normally", mode="run"), + SystemAction(title="recover", mode="recover"), + SystemAction(title="reinstall", mode="install"), + SystemAction(title="a-other", mode="some-other"), + SystemAction(title="b-other", mode="unknown"), + ] + self.assertSequenceEqual(sorted(data, key=by_preferred_action_type), + exp) diff --git a/console_conf/ui/views/welcome.py b/console_conf/ui/views/welcome.py index 16227135..09d37bf9 100644 --- a/console_conf/ui/views/welcome.py +++ b/console_conf/ui/views/welcome.py @@ -51,6 +51,13 @@ class WelcomeView(BaseView): class ChooserWelcomeView(WelcomeView): excerpt = ( - "System recovery triggered. Proceed to select one of available " - "systems and execute a recovery action." + "System mode selection triggered. Proceed to select one of available " + "systems and actions." ) + + def __init__(self, controller): + super().__init__(controller) + + current = controller.model.current + if current is not None: + self.title = current.model.display_name diff --git a/subiquitycore/core.py b/subiquitycore/core.py index ba1fec21..3a0e493c 100644 --- a/subiquitycore/core.py +++ b/subiquitycore/core.py @@ -310,7 +310,7 @@ class Application: # "Identity", # "InstallProgress", # ] - # The 'next_screen' and 'prev-screen' methods move through the list of + # The 'next_screen' and 'prev_screen' methods move through the list of # controllers in order, calling the start_ui method on the controller # instance.