From 881869ea92076deb275bc157d10c318449fae0e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 24 Jan 2025 09:30:13 +0000 Subject: [PATCH 1/5] feat: introduce a new "finish" phase --- rust/agama-lib/src/manager.rs | 3 +++ service/lib/agama/dbus/clients/manager.rb | 2 ++ service/lib/agama/dbus/manager.rb | 5 ++++- service/lib/agama/installation_phase.rb | 18 +++++++++++++++ service/lib/agama/manager.rb | 1 + .../test/agama/dbus/clients/manager_test.rb | 8 +++++++ service/test/agama/dbus/manager_test.rb | 7 +++++- service/test/agama/installation_phase_test.rb | 22 +++++++++++++++++++ service/test/agama/manager_test.rb | 7 +++--- 9 files changed, 68 insertions(+), 5 deletions(-) diff --git a/rust/agama-lib/src/manager.rs b/rust/agama-lib/src/manager.rs index d59e79c1f7..6eaf80bf43 100644 --- a/rust/agama-lib/src/manager.rs +++ b/rust/agama-lib/src/manager.rs @@ -67,6 +67,8 @@ pub enum InstallationPhase { Config, /// Installation phase. Install, + /// Finished installation phase. + Finish, } impl TryFrom for InstallationPhase { @@ -77,6 +79,7 @@ impl TryFrom for InstallationPhase { 0 => Ok(Self::Startup), 1 => Ok(Self::Config), 2 => Ok(Self::Install), + 3 => Ok(Self::Finish), _ => Err(ServiceError::UnknownInstallationPhase(value)), } } diff --git a/service/lib/agama/dbus/clients/manager.rb b/service/lib/agama/dbus/clients/manager.rb index b766683c57..2ef82a0cd5 100644 --- a/service/lib/agama/dbus/clients/manager.rb +++ b/service/lib/agama/dbus/clients/manager.rb @@ -63,6 +63,8 @@ def current_installation_phase InstallationPhase::CONFIG when DBus::Manager::INSTALL_PHASE InstallationPhase::INSTALL + when DBus::Manager::FINISH_PHASE + InstallationPhase::FINISH end end diff --git a/service/lib/agama/dbus/manager.rb b/service/lib/agama/dbus/manager.rb index 19a69c359c..09f9d5dcb0 100644 --- a/service/lib/agama/dbus/manager.rb +++ b/service/lib/agama/dbus/manager.rb @@ -58,6 +58,7 @@ def initialize(backend, logger) STARTUP_PHASE = 0 CONFIG_PHASE = 1 INSTALL_PHASE = 2 + FINISH_PHASE = 3 dbus_interface MANAGER_INTERFACE do dbus_method(:Probe, "") { config_phase } @@ -111,7 +112,8 @@ def installation_phases [ { "id" => STARTUP_PHASE, "label" => "startup" }, { "id" => CONFIG_PHASE, "label" => "config" }, - { "id" => INSTALL_PHASE, "label" => "install" } + { "id" => INSTALL_PHASE, "label" => "install" }, + { "id" => FINISH_PHASE, "label" => "finish" } ] end @@ -122,6 +124,7 @@ def current_installation_phase return STARTUP_PHASE if backend.installation_phase.startup? return CONFIG_PHASE if backend.installation_phase.config? return INSTALL_PHASE if backend.installation_phase.install? + return FINISH_PHASE if backend.installation_phase.finish? end # States whether installation runs on iguana diff --git a/service/lib/agama/installation_phase.rb b/service/lib/agama/installation_phase.rb index dbc650d532..68184c6ef3 100644 --- a/service/lib/agama/installation_phase.rb +++ b/service/lib/agama/installation_phase.rb @@ -27,6 +27,7 @@ class InstallationPhase STARTUP = "startup" CONFIG = "config" INSTALL = "install" + FINISH = "finish" def initialize @on_change_callbacks = [] @@ -53,6 +54,13 @@ def install? value == INSTALL end + # Whether the current installation phase value is finish + # + # @return [Boolean] + def finish? + value == FINISH + end + # Sets the installation phase value to startup # # @note Callbacks are called. @@ -83,6 +91,16 @@ def install self end + # Sets the installation phase value to finish + # + # @note Callbacks are called. + # + # @return [self] + def finish + change_to(FINISH) + self + end + # Registers callbacks to be called when the installation phase value changes # # @param block [Proc] diff --git a/service/lib/agama/manager.rb b/service/lib/agama/manager.rb index 69ef425dd8..6bae4e6bf6 100644 --- a/service/lib/agama/manager.rb +++ b/service/lib/agama/manager.rb @@ -144,6 +144,7 @@ def install_phase logger.error "Installation error: #{e.inspect}. Backtrace: #{e.backtrace}" ensure service_status.idle + installation_phase.finish finish_progress end # rubocop:enable Metrics/AbcSize diff --git a/service/test/agama/dbus/clients/manager_test.rb b/service/test/agama/dbus/clients/manager_test.rb index 9c91faa6b5..9b3ff7e488 100644 --- a/service/test/agama/dbus/clients/manager_test.rb +++ b/service/test/agama/dbus/clients/manager_test.rb @@ -99,6 +99,14 @@ expect(subject.current_installation_phase).to eq(Agama::InstallationPhase::INSTALL) end end + + context "when the current phase is finish" do + let(:current_phase) { Agama::DBus::Manager::FINISH_PHASE } + + it "returns the install phase value" do + expect(subject.current_installation_phase).to eq(Agama::InstallationPhase::FINISH) + end + end end include_examples "service status" diff --git a/service/test/agama/dbus/manager_test.rb b/service/test/agama/dbus/manager_test.rb index d0b62ae234..2567a6d33b 100644 --- a/service/test/agama/dbus/manager_test.rb +++ b/service/test/agama/dbus/manager_test.rb @@ -157,7 +157,7 @@ describe "#installation_phases" do it "includes all possible values for the installation phase" do labels = subject.installation_phases.map { |i| i["label"] } - expect(labels).to contain_exactly("startup", "config", "install") + expect(labels).to contain_exactly("startup", "config", "install", "finish") end it "associates 'startup' with the id 0" do @@ -174,6 +174,11 @@ install = subject.installation_phases.find { |i| i["label"] == "install" } expect(install["id"]).to eq(described_class::INSTALL_PHASE) end + + it "associates 'finish' with the id 3" do + install = subject.installation_phases.find { |i| i["label"] == "finish" } + expect(install["id"]).to eq(described_class::FINISH_PHASE) + end end describe "#current_installation_phase" do diff --git a/service/test/agama/installation_phase_test.rb b/service/test/agama/installation_phase_test.rb index 11825cae20..8eb08c427e 100644 --- a/service/test/agama/installation_phase_test.rb +++ b/service/test/agama/installation_phase_test.rb @@ -95,6 +95,28 @@ end end + describe "finish?" do + context "if the installation phase is finish" do + before do + subject.finish + end + + it "returns true" do + expect(subject.finish?).to eq(true) + end + end + + context "if the installation phase is not finish" do + before do + subject.config + end + + it "returns false" do + expect(subject.finish?).to eq(false) + end + end + end + describe "#startup" do it "sets the installation phase to startup" do subject.startup diff --git a/service/test/agama/manager_test.rb b/service/test/agama/manager_test.rb index e37a9034e9..e2b4952e97 100644 --- a/service/test/agama/manager_test.rb +++ b/service/test/agama/manager_test.rb @@ -20,7 +20,7 @@ # find current contact information at www.suse.com. require_relative "../test_helper" -require_relative "./with_progress_examples" +require_relative "with_progress_examples" require "agama/manager" require "agama/config" require "agama/issue" @@ -115,9 +115,10 @@ end describe "#install_phase" do - it "sets the installation phase to install" do + it "sets the installation phase to install and later to finish" do + expect(subject.installation_phase).to receive(:install) subject.install_phase - expect(subject.installation_phase.install?).to eq(true) + expect(subject.installation_phase.finish?).to eq(true) end it "calls #propose on proxy and software modules" do From 07166a0a9168e88e1bb105ce84a4ecf5fba55803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 24 Jan 2025 09:31:14 +0000 Subject: [PATCH 2/5] fix(web): use the "finish" in InstallationFinished component --- web/src/App.test.tsx | 8 +++----- web/src/App.tsx | 4 ++-- web/src/components/core/InstallationFinished.test.tsx | 2 +- web/src/components/core/InstallationFinished.tsx | 8 ++------ web/src/types/status.ts | 1 + 5 files changed, 9 insertions(+), 14 deletions(-) diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index 5984c42a9e..ca54efabb3 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -203,10 +203,9 @@ describe("App", () => { }); }); - describe("on the busy installation phase", () => { + describe("on the installation phase", () => { beforeEach(() => { mockClientStatus.phase = InstallationPhase.Install; - mockClientStatus.isBusy = true; mockSelectedProduct = tumbleweed; }); @@ -216,10 +215,9 @@ describe("App", () => { }); }); - describe("on the idle installation phase", () => { + describe("on the idle finish phase", () => { beforeEach(() => { - mockClientStatus.phase = InstallationPhase.Install; - mockClientStatus.isBusy = false; + mockClientStatus.phase = InstallationPhase.Finish; mockSelectedProduct = tumbleweed; }); diff --git a/web/src/App.tsx b/web/src/App.tsx index 06b45ba55d..dc3b01e317 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -55,11 +55,11 @@ function App() { const Content = () => { if (error) return ; - if (phase === InstallationPhase.Install && isBusy) { + if (phase === InstallationPhase.Install) { return ; } - if (phase === InstallationPhase.Install && !isBusy) { + if (phase === InstallationPhase.Finish) { return ; } diff --git a/web/src/components/core/InstallationFinished.test.tsx b/web/src/components/core/InstallationFinished.test.tsx index c96ed26e37..91432a6ed8 100644 --- a/web/src/components/core/InstallationFinished.test.tsx +++ b/web/src/components/core/InstallationFinished.test.tsx @@ -29,7 +29,7 @@ import { Encryption } from "~/api/storage/types/config"; jest.mock("~/queries/status", () => ({ ...jest.requireActual("~/queries/status"), - useInstallerStatus: () => ({ isBusy: false, useIguana: false, phase: 2, canInstall: false }), + useInstallerStatus: () => ({ isBusy: false, useIguana: false, phase: 3, canInstall: false }), })); type storageConfigType = "guided" | "raw"; diff --git a/web/src/components/core/InstallationFinished.tsx b/web/src/components/core/InstallationFinished.tsx index 3a609043a6..060bc8bf2f 100644 --- a/web/src/components/core/InstallationFinished.tsx +++ b/web/src/components/core/InstallationFinished.tsx @@ -100,17 +100,13 @@ function usingTpm(config): boolean { } function InstallationFinished() { - const { phase, isBusy, useIguana } = useInstallerStatus({ suspense: true }); + const { phase, useIguana } = useInstallerStatus({ suspense: true }); const config = useConfig(); - if (phase !== InstallationPhase.Install) { + if (phase !== InstallationPhase.Finish) { return ; } - if (isBusy) { - return ; - } - return (
diff --git a/web/src/types/status.ts b/web/src/types/status.ts index 1aa07ace86..7c598f69b2 100644 --- a/web/src/types/status.ts +++ b/web/src/types/status.ts @@ -27,6 +27,7 @@ enum InstallationPhase { Startup = 0, Config = 1, Install = 2, + Finish = 3, } /* From 56eadf1baed739801303aba47a9d29bf7800500a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 24 Jan 2025 09:36:20 +0000 Subject: [PATCH 3/5] docs: update changes files --- rust/package/agama.changes | 6 ++++++ service/package/rubygem-agama-yast.changes | 6 ++++++ web/package/agama-web-ui.changes | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/rust/package/agama.changes b/rust/package/agama.changes index f40bf411c1..495c35efdb 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Fri Jan 24 09:33:31 UTC 2025 - Imobach Gonzalez Sosa + +- Introduce a new installation phase "finish" + (gh#agama-project/agama#1616). + ------------------------------------------------------------------- Fri Jan 24 06:43:05 UTC 2025 - Imobach Gonzalez Sosa diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index 5a4202b075..20c4d68ee7 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Fri Jan 24 09:33:27 UTC 2025 - Imobach Gonzalez Sosa + +- Introduce a new installation phase "finish" + (gh#agama-project/agama#1616). + ------------------------------------------------------------------- Fri Jan 24 06:42:00 UTC 2025 - Imobach Gonzalez Sosa diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index 3fd2a18a68..d5a49a67c5 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Fri Jan 24 09:34:24 UTC 2025 - Imobach Gonzalez Sosa + +- Use the "finish" to decide whether to navigate to the + congratulations screen (gh#agama-project/agama#1616). + ------------------------------------------------------------------- Fri Jan 24 06:43:52 UTC 2025 - Imobach Gonzalez Sosa From d26394a08277649129452b0aa73914ce7edae64d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 24 Jan 2025 11:51:02 +0000 Subject: [PATCH 4/5] fix(web): remove isBusy from InstallationProgress --- .../core/InstallationProgress.test.tsx | 18 ++---------------- .../components/core/InstallationProgress.tsx | 6 +----- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/web/src/components/core/InstallationProgress.test.tsx b/web/src/components/core/InstallationProgress.test.tsx index 46bccbb010..12bfdf4727 100644 --- a/web/src/components/core/InstallationProgress.test.tsx +++ b/web/src/components/core/InstallationProgress.test.tsx @@ -27,7 +27,6 @@ import { InstallationPhase } from "~/types/status"; import InstallationProgress from "./InstallationProgress"; import { ROOT } from "~/routes/paths"; -let isBusy = false; let phase = InstallationPhase.Install; const mockInstallerStatusChanges = jest.fn(); @@ -35,7 +34,7 @@ jest.mock("~/components/core/ProgressReport", () => () =>
ProgressReport Mo jest.mock("~/queries/status", () => ({ ...jest.requireActual("~/queries/status"), - useInstallerStatus: () => ({ isBusy, phase }), + useInstallerStatus: () => ({ phase }), useInstallerStatusChanges: () => mockInstallerStatusChanges(), })); @@ -56,10 +55,9 @@ describe("InstallationProgress", () => { }); }); - describe("when installer in the installation phase and busy", () => { + describe("when installer in the installation phase", () => { beforeEach(() => { phase = InstallationPhase.Install; - isBusy = true; }); it("renders progress report", () => { @@ -67,16 +65,4 @@ describe("InstallationProgress", () => { screen.getByText("ProgressReport Mock"); }); }); - - describe("when installer in the installation phase but not busy", () => { - beforeEach(() => { - phase = InstallationPhase.Install; - isBusy = false; - }); - - it("redirect to installation finished path", async () => { - installerRender(); - await screen.findByText(`Navigating to ${ROOT.installationFinished}`); - }); - }); }); diff --git a/web/src/components/core/InstallationProgress.tsx b/web/src/components/core/InstallationProgress.tsx index 53e38685d4..6197d8720d 100644 --- a/web/src/components/core/InstallationProgress.tsx +++ b/web/src/components/core/InstallationProgress.tsx @@ -29,17 +29,13 @@ import { Navigate } from "react-router-dom"; import { useInstallerStatus, useInstallerStatusChanges } from "~/queries/status"; function InstallationProgress() { - const { isBusy, phase } = useInstallerStatus({ suspense: true }); + const { phase } = useInstallerStatus({ suspense: true }); useInstallerStatusChanges(); if (phase !== InstallationPhase.Install) { return ; } - if (!isBusy) { - return ; - } - return ; } From f9fba4faceece595dc5c41262011a2ff072aef90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 24 Jan 2025 11:51:47 +0000 Subject: [PATCH 5/5] chore(web): update from code review --- web/src/App.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index ca54efabb3..2715aac301 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -215,7 +215,7 @@ describe("App", () => { }); }); - describe("on the idle finish phase", () => { + describe("on the finish phase", () => { beforeEach(() => { mockClientStatus.phase = InstallationPhase.Finish; mockSelectedProduct = tumbleweed;