From bdc3bbcf78df0055b2ee9d6d95fe67977d31b15c Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 18 Jul 2024 09:44:49 +0200 Subject: [PATCH 01/26] initial version of report patching --- service/lib/agama/autoyast/converter.rb | 2 + service/lib/agama/autoyast/report_patching.rb | 106 ++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 service/lib/agama/autoyast/report_patching.rb diff --git a/service/lib/agama/autoyast/converter.rb b/service/lib/agama/autoyast/converter.rb index e49fd97ce1..0dfcadaa96 100755 --- a/service/lib/agama/autoyast/converter.rb +++ b/service/lib/agama/autoyast/converter.rb @@ -33,6 +33,8 @@ require "fileutils" require "pathname" +require "agama/autoyast/report_patching" + # :nodoc: module Agama module AutoYaST diff --git a/service/lib/agama/autoyast/report_patching.rb b/service/lib/agama/autoyast/report_patching.rb new file mode 100644 index 0000000000..609554b202 --- /dev/null +++ b/service/lib/agama/autoyast/report_patching.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# 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 General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +# Goal of this file is monkey patch Popup and Report functionality to not try to use UI +# and instead adapt it for agama needs. +# Do not directly require agama ruby files to be able to keep autoyast converter and agama +# independent. +# TODO: what to do if it runs without agama? Just print question to stderr? + +require "yaml" +require "yast/execute" + +module Yast2 + class Popup + class << self + # Keep in sync with real Yast2::Popup + def show(message, details: "", headline: "", timeout: 0, focus: nil, buttons: :ok, richtext: false, style: :notice) + # at first construct agama question to display. + # NOTE: timeout is not supported. + # FIXME: what to do with richtext? + text = message + text += "\n\n" + details unless details.empty? + options = generate_options(buttons) + question = { + # TODO: id for newly created question is ignored, but maybe it will be better to not have to specify it at all? + id: 0, + class: "autoyast.popup", + text: text, + options: generate_options(buttons), + default_option: focus || options.first, + data: {} + } + data = { generic: question }.to_yaml + answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, stdout: :capture) + answer = YAML.load(answer_yaml) + answer["generic"]["answer"].to_sym + end + + private + + def generate_options(buttons) + case buttons + when :ok + [:ok] + when :continue_cancel + [:continue, :cancel] + when :yes_no + [:yes, :no] + when Hash + buttons.keys + else + raise ArgumentError, "Invalid value #{buttons.inspect} for buttons" + end + end + end + end +end + +# needed to ask for GPG encrypted autoyast profiles +# TODO: encrypt agama profile? is it needed? +module UI + class PasswordDialog + def new(label, confirm: false) + @label = label + # NOTE: implement confirm if needed + end + + def run + # at first construct agama question to display. + text = @label + question = { + # TODO: id for newly created question is ignored, but maybe it will be better to not have to specify it at all? + id: 0, + class: "autoyast.password", + text: text, + options: ["ok", "cancel"], + default_option: "cancel", + data: {} + } + data = { generic: question, with_password: {} }.to_yaml + answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, stdout: :capture) + answer = YAML.load(answer_yaml) + result = answer["generic"]["answer"].to_sym + return nil if result == "cancel" + answer["with_password"]["password"] + end + end +end From fb7603689d3b65a2f91cbdcbae75de45d87042b8 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 18 Jul 2024 14:11:31 +0200 Subject: [PATCH 02/26] add readme for agama-yast with hint how to deploy modified gem --- service/README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 service/README.md diff --git a/service/README.md b/service/README.md new file mode 100644 index 0000000000..63261976aa --- /dev/null +++ b/service/README.md @@ -0,0 +1,21 @@ +# Agama YaST + +According to [Agama's architecture](../doc/architecture.md) this project implements the following components: + +* The *Agama YaST*, the layer build on top of YaST functionality. + +## Testing Changes + +The easiest way to test changes done to ruby code on agama liveCD is to build +gem with modified sources with `gem build agama-yast`. Then copy resulting file +to agama live ISO. There do this sequence of commands: + +```sh +# ensure that only modified sources are installed +gem uninstall agama-yast +# install modified sources including proper binary names +gem install --no-doc --no-format-executable +``` + +If change modifies also dbus parts, then restart related dbus services. + From ddd18bfd756c4941a4af07429ef9ff866086c9f5 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 18 Jul 2024 14:20:54 +0200 Subject: [PATCH 03/26] fix file loading --- service/lib/agama/autoyast/report_patching.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/lib/agama/autoyast/report_patching.rb b/service/lib/agama/autoyast/report_patching.rb index 609554b202..62a845415b 100644 --- a/service/lib/agama/autoyast/report_patching.rb +++ b/service/lib/agama/autoyast/report_patching.rb @@ -26,7 +26,7 @@ # TODO: what to do if it runs without agama? Just print question to stderr? require "yaml" -require "yast/execute" +require "yast2/execute" module Yast2 class Popup From d2a7391aca6136920a62f6f8e0765eea01be8910 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 18 Jul 2024 14:27:17 +0200 Subject: [PATCH 04/26] make rubocop happy --- service/lib/agama/autoyast/bond_reader.rb | 0 .../lib/agama/autoyast/connections_reader.rb | 0 service/lib/agama/autoyast/l10n_reader.rb | 0 service/lib/agama/autoyast/network_reader.rb | 0 service/lib/agama/autoyast/product_reader.rb | 0 service/lib/agama/autoyast/report_patching.rb | 28 ++++++++++++++----- service/lib/agama/autoyast/root_reader.rb | 1 - service/lib/agama/autoyast/software_reader.rb | 1 - service/lib/agama/autoyast/storage_reader.rb | 1 - service/lib/agama/autoyast/user_reader.rb | 1 - service/lib/agama/autoyast/wireless_reader.rb | 1 - 11 files changed, 21 insertions(+), 12 deletions(-) mode change 100644 => 100755 service/lib/agama/autoyast/bond_reader.rb mode change 100644 => 100755 service/lib/agama/autoyast/connections_reader.rb mode change 100644 => 100755 service/lib/agama/autoyast/l10n_reader.rb mode change 100644 => 100755 service/lib/agama/autoyast/network_reader.rb mode change 100644 => 100755 service/lib/agama/autoyast/product_reader.rb mode change 100644 => 100755 service/lib/agama/autoyast/root_reader.rb mode change 100644 => 100755 service/lib/agama/autoyast/software_reader.rb mode change 100644 => 100755 service/lib/agama/autoyast/storage_reader.rb mode change 100644 => 100755 service/lib/agama/autoyast/user_reader.rb mode change 100644 => 100755 service/lib/agama/autoyast/wireless_reader.rb diff --git a/service/lib/agama/autoyast/bond_reader.rb b/service/lib/agama/autoyast/bond_reader.rb old mode 100644 new mode 100755 diff --git a/service/lib/agama/autoyast/connections_reader.rb b/service/lib/agama/autoyast/connections_reader.rb old mode 100644 new mode 100755 diff --git a/service/lib/agama/autoyast/l10n_reader.rb b/service/lib/agama/autoyast/l10n_reader.rb old mode 100644 new mode 100755 diff --git a/service/lib/agama/autoyast/network_reader.rb b/service/lib/agama/autoyast/network_reader.rb old mode 100644 new mode 100755 diff --git a/service/lib/agama/autoyast/product_reader.rb b/service/lib/agama/autoyast/product_reader.rb old mode 100644 new mode 100755 diff --git a/service/lib/agama/autoyast/report_patching.rb b/service/lib/agama/autoyast/report_patching.rb index 62a845415b..dbfd7fef28 100644 --- a/service/lib/agama/autoyast/report_patching.rb +++ b/service/lib/agama/autoyast/report_patching.rb @@ -28,11 +28,16 @@ require "yaml" require "yast2/execute" +# :nodoc: +# rubocop:disable Metrics/ParameterLists +# rubocop:disable Lint/UnusedMethodArgument module Yast2 + # :nodoc: class Popup class << self # Keep in sync with real Yast2::Popup - def show(message, details: "", headline: "", timeout: 0, focus: nil, buttons: :ok, richtext: false, style: :notice) + def show(message, details: "", headline: "", timeout: 0, focus: nil, + buttons: :ok, richtext: false, style: :notice) # at first construct agama question to display. # NOTE: timeout is not supported. # FIXME: what to do with richtext? @@ -40,7 +45,8 @@ def show(message, details: "", headline: "", timeout: 0, focus: nil, buttons: :o text += "\n\n" + details unless details.empty? options = generate_options(buttons) question = { - # TODO: id for newly created question is ignored, but maybe it will be better to not have to specify it at all? + # TODO: id for newly created question is ignored, but maybe it will + # be better to not have to specify it at all? id: 0, class: "autoyast.popup", text: text, @@ -49,8 +55,9 @@ def show(message, details: "", headline: "", timeout: 0, focus: nil, buttons: :o data: {} } data = { generic: question }.to_yaml - answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, stdout: :capture) - answer = YAML.load(answer_yaml) + answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", + stdin: data, stdout: :capture) + answer = YAML.safe_load(answer_yaml) answer["generic"]["answer"].to_sym end @@ -77,6 +84,7 @@ def generate_options(buttons) # needed to ask for GPG encrypted autoyast profiles # TODO: encrypt agama profile? is it needed? module UI + # :nodoc: class PasswordDialog def new(label, confirm: false) @label = label @@ -87,7 +95,8 @@ def run # at first construct agama question to display. text = @label question = { - # TODO: id for newly created question is ignored, but maybe it will be better to not have to specify it at all? + # TODO: id for newly created question is ignored, but maybe it will + # be better to not have to specify it at all? id: 0, class: "autoyast.password", text: text, @@ -96,11 +105,16 @@ def run data: {} } data = { generic: question, with_password: {} }.to_yaml - answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, stdout: :capture) - answer = YAML.load(answer_yaml) + answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, +stdout: :capture) + answer = YAML.safe_load(answer_yaml) result = answer["generic"]["answer"].to_sym return nil if result == "cancel" + answer["with_password"]["password"] end end end + +# rubocop:enable Metrics/ParameterLists +# rubocop:enable Lint/UnusedMethodArgument diff --git a/service/lib/agama/autoyast/root_reader.rb b/service/lib/agama/autoyast/root_reader.rb old mode 100644 new mode 100755 index 2b009069f5..1598cc5f92 --- a/service/lib/agama/autoyast/root_reader.rb +++ b/service/lib/agama/autoyast/root_reader.rb @@ -1,4 +1,3 @@ -#!/usr/bin/env ruby # frozen_string_literal: true # Copyright (c) [2024] SUSE LLC diff --git a/service/lib/agama/autoyast/software_reader.rb b/service/lib/agama/autoyast/software_reader.rb old mode 100644 new mode 100755 index b4edc774e1..213c7b4c48 --- a/service/lib/agama/autoyast/software_reader.rb +++ b/service/lib/agama/autoyast/software_reader.rb @@ -1,4 +1,3 @@ -#!/usr/bin/env ruby # frozen_string_literal: true # Copyright (c) [2024] SUSE LLC diff --git a/service/lib/agama/autoyast/storage_reader.rb b/service/lib/agama/autoyast/storage_reader.rb old mode 100644 new mode 100755 index 96a00e975c..7e0b543a9f --- a/service/lib/agama/autoyast/storage_reader.rb +++ b/service/lib/agama/autoyast/storage_reader.rb @@ -1,4 +1,3 @@ -#!/usr/bin/env ruby # frozen_string_literal: true # Copyright (c) [2024] SUSE LLC diff --git a/service/lib/agama/autoyast/user_reader.rb b/service/lib/agama/autoyast/user_reader.rb old mode 100644 new mode 100755 index ad850ff550..1df0ec05b3 --- a/service/lib/agama/autoyast/user_reader.rb +++ b/service/lib/agama/autoyast/user_reader.rb @@ -1,4 +1,3 @@ -#!/usr/bin/env ruby # frozen_string_literal: true # Copyright (c) [2024] SUSE LLC diff --git a/service/lib/agama/autoyast/wireless_reader.rb b/service/lib/agama/autoyast/wireless_reader.rb old mode 100644 new mode 100755 index 3a273eec68..9c4bb5f3e5 --- a/service/lib/agama/autoyast/wireless_reader.rb +++ b/service/lib/agama/autoyast/wireless_reader.rb @@ -1,4 +1,3 @@ -#!/usr/bin/env ruby # frozen_string_literal: true # Copyright (c) [2024] SUSE LLC From 747ee38749cc2167ec2f4ebd9b8141c73c7bfc82 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 18 Jul 2024 16:24:36 +0200 Subject: [PATCH 05/26] fix returning correct question and add tracing to see which kind of question is created --- rust/agama-server/src/questions/web.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rust/agama-server/src/questions/web.rs b/rust/agama-server/src/questions/web.rs index 3bae35d0e6..d1c1077eef 100644 --- a/rust/agama-server/src/questions/web.rs +++ b/rust/agama-server/src/questions/web.rs @@ -61,6 +61,7 @@ impl<'a> QuestionsClient<'a> { .map(|(k, v)| (k.as_str(), v.as_str())) .collect(); let path = if question.with_password.is_some() { + tracing::info!("creating question with password"); self.questions_proxy .new_with_password( &generic.class, @@ -71,6 +72,7 @@ impl<'a> QuestionsClient<'a> { ) .await? } else { + tracing::info!("creating generic question"); self.questions_proxy .new_question( &generic.class, @@ -89,7 +91,8 @@ impl<'a> QuestionsClient<'a> { return Err(ServiceError::UnsuccessfulAction(msg)); }; // TODO: better error if path does not contain id res.generic.id = id_cap["id"].parse::().unwrap(); - Ok(question) + tracing::info!("new question gets id {}", res.generic.id); + Ok(res) } pub async fn questions(&self) -> Result, ServiceError> { From 4a7ece494b5c98ad023d14cc6f959c93125ce9c1 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 18 Jul 2024 16:29:52 +0200 Subject: [PATCH 06/26] clarify format of questions and answers --- rust/agama-cli/src/questions.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rust/agama-cli/src/questions.rs b/rust/agama-cli/src/questions.rs index 51513b2a76..7dfe1ae709 100644 --- a/rust/agama-cli/src/questions.rs +++ b/rust/agama-cli/src/questions.rs @@ -3,6 +3,7 @@ use agama_lib::questions::http_client::HTTPClient; use agama_lib::{connection, error::ServiceError}; use clap::{Args, Subcommand, ValueEnum}; +// TODO: use for answers also JSON to be consistent #[derive(Subcommand, Debug)] pub enum QuestionsCommands { /// Set the mode for answering questions. @@ -19,9 +20,9 @@ pub enum QuestionsCommands { /// Path to a file containing the answers in YAML format. path: String, }, - /// prints list of questions that is waiting for answer in YAML format + /// prints list of questions that is waiting for answer in JSON format List, - /// Ask question from stdin in YAML format and print answer when it is answered. + /// Ask question from stdin in JSON format and print answer when it is answered. Ask, } From c5494e2a8940c5c165b2972cc2ac67f2f804d3ed Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 18 Jul 2024 14:54:19 +0200 Subject: [PATCH 07/26] fix superclass --- service/lib/agama/autoyast/report_patching.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/service/lib/agama/autoyast/report_patching.rb b/service/lib/agama/autoyast/report_patching.rb index dbfd7fef28..965e8bb09f 100644 --- a/service/lib/agama/autoyast/report_patching.rb +++ b/service/lib/agama/autoyast/report_patching.rb @@ -26,7 +26,9 @@ # TODO: what to do if it runs without agama? Just print question to stderr? require "yaml" +require "yast" require "yast2/execute" +require "ui/dialog" # :nodoc: # rubocop:disable Metrics/ParameterLists @@ -85,7 +87,7 @@ def generate_options(buttons) # TODO: encrypt agama profile? is it needed? module UI # :nodoc: - class PasswordDialog + class PasswordDialog < Dialog def new(label, confirm: false) @label = label # NOTE: implement confirm if needed From f1dc311dba6c14e1e39cd2bb7cec66dcfba2ce92 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 18 Jul 2024 16:28:33 +0200 Subject: [PATCH 08/26] fixes from testing --- service/lib/agama/autoyast/report_patching.rb | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/service/lib/agama/autoyast/report_patching.rb b/service/lib/agama/autoyast/report_patching.rb index 965e8bb09f..a102ac1cba 100644 --- a/service/lib/agama/autoyast/report_patching.rb +++ b/service/lib/agama/autoyast/report_patching.rb @@ -25,7 +25,7 @@ # independent. # TODO: what to do if it runs without agama? Just print question to stderr? -require "yaml" +require "json" require "yast" require "yast2/execute" require "ui/dialog" @@ -56,10 +56,10 @@ def show(message, details: "", headline: "", timeout: 0, focus: nil, default_option: focus || options.first, data: {} } - data = { generic: question }.to_yaml - answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", + data = { generic: question }.to_json + answer_json = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, stdout: :capture) - answer = YAML.safe_load(answer_yaml) + answer = JSON.parse!(answer_json) answer["generic"]["answer"].to_sym end @@ -99,17 +99,17 @@ def run question = { # TODO: id for newly created question is ignored, but maybe it will # be better to not have to specify it at all? - id: 0, - class: "autoyast.password", - text: text, - options: ["ok", "cancel"], - default_option: "cancel", - data: {} + "id" => 0, + "class" => "autoyast.password", + "text" => text, + "options" => ["ok", "cancel"], + "defaultOption" => "cancel", + "data" => {} } - data = { generic: question, with_password: {} }.to_yaml - answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, + data = { "generic" => question, "withPassword" => {} }.to_json + answer_json = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, stdout: :capture) - answer = YAML.safe_load(answer_yaml) + answer = JSON.parse!(answer_json) result = answer["generic"]["answer"].to_sym return nil if result == "cancel" From d92deef4ded5eb6f400e9bad54f080ff392cd229 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Fri, 19 Jul 2024 10:30:13 +0200 Subject: [PATCH 09/26] fix getting answer --- rust/agama-lib/src/error.rs | 2 ++ rust/agama-server/src/questions/web.rs | 43 +++++++++++++------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/rust/agama-lib/src/error.rs b/rust/agama-lib/src/error.rs index f40278ed7e..b5809c73b7 100644 --- a/rust/agama-lib/src/error.rs +++ b/rust/agama-lib/src/error.rs @@ -37,6 +37,8 @@ pub enum ServiceError { UnsuccessfulAction(String), #[error("Unknown installation phase: {0}")] UnknownInstallationPhase(u32), + #[error("Question with id {0} does not exist")] + QuestionNotExist(u32), #[error("Backend call failed with status {0} and text '{1}'")] BackendError(u16, String), #[error("You are not logged in. Please use: agama auth login")] diff --git a/rust/agama-server/src/questions/web.rs b/rust/agama-server/src/questions/web.rs index d1c1077eef..6fd1788cdf 100644 --- a/rust/agama-server/src/questions/web.rs +++ b/rust/agama-server/src/questions/web.rs @@ -7,6 +7,7 @@ use crate::{error::Error, web::Event}; use agama_lib::{ + dbus::{extract_id_from_path, get_property}, error::ServiceError, proxies::{GenericQuestionProxy, QuestionWithPasswordProxy, Questions1Proxy}, questions::model::{Answer, GenericQuestion, PasswordAnswer, Question, QuestionWithPassword}, @@ -19,7 +20,6 @@ use axum::{ routing::{delete, get}, Json, Router, }; -use regex::Regex; use std::{collections::HashMap, pin::Pin}; use tokio_stream::{Stream, StreamExt}; use zbus::{ @@ -84,13 +84,7 @@ impl<'a> QuestionsClient<'a> { .await? }; let mut res = question.clone(); - // we are sure that regexp is correct, so use unwrap - let id_matcher = Regex::new(r"/(?\d+)$").unwrap(); - let Some(id_cap) = id_matcher.captures(path.as_str()) else { - let msg = format!("Failed to get ID for new question: {}", path.as_str()).to_string(); - return Err(ServiceError::UnsuccessfulAction(msg)); - }; // TODO: better error if path does not contain id - res.generic.id = id_cap["id"].parse::().unwrap(); + res.generic.id = extract_id_from_path(&path)?; tracing::info!("new question gets id {}", res.generic.id); Ok(res) } @@ -167,24 +161,29 @@ impl<'a> QuestionsClient<'a> { ObjectPath::try_from(format!("/org/opensuse/Agama1/Questions/{}", id)) .context("Failed to create dbus path")?, ); + let objects = self.objects_proxy.get_managed_objects().await?; + let password_interface = OwnedInterfaceName::from( + InterfaceName::from_static_str("org.opensuse.Agama1.Questions.WithPassword") + .context("Failed to create interface name for question with password")?, + ); let mut result = Answer::default(); - let dbus_password_res = QuestionWithPasswordProxy::builder(&self.connection) - .path(&question_path)? - .cache_properties(zbus::CacheProperties::No) - .build() - .await; - if let Ok(dbus_password) = dbus_password_res { + let question = objects + .get(&question_path) + .ok_or(ServiceError::QuestionNotExist(id))?; + + if let Some(password_iface) = question.get(&password_interface) { result.with_password = Some(PasswordAnswer { - password: dbus_password.password().await?, + password: get_property(password_iface, "password")?, }); } - - let dbus_generic = GenericQuestionProxy::builder(&self.connection) - .path(&question_path)? - .cache_properties(zbus::CacheProperties::No) - .build() - .await?; - let answer = dbus_generic.answer().await?; + let generic_interface = OwnedInterfaceName::from( + InterfaceName::from_static_str("org.opensuse.Agama1.Questions.Generic") + .context("Failed to create interface name for generic question")?, + ); + let generic_iface = question + .get(&generic_interface) + .context("Question does not have generic interface")?; + let answer: String = get_property(generic_iface, "answer")?; if answer.is_empty() { Ok(None) } else { From 4d8c0ced453af80cda904aa3bfcdad18d7866ed5 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Fri, 19 Jul 2024 22:01:21 +0200 Subject: [PATCH 10/26] use correct property names --- rust/agama-server/src/questions/web.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/agama-server/src/questions/web.rs b/rust/agama-server/src/questions/web.rs index 6fd1788cdf..7615c20911 100644 --- a/rust/agama-server/src/questions/web.rs +++ b/rust/agama-server/src/questions/web.rs @@ -173,7 +173,7 @@ impl<'a> QuestionsClient<'a> { if let Some(password_iface) = question.get(&password_interface) { result.with_password = Some(PasswordAnswer { - password: get_property(password_iface, "password")?, + password: get_property(password_iface, "Password")?, }); } let generic_interface = OwnedInterfaceName::from( @@ -183,7 +183,7 @@ impl<'a> QuestionsClient<'a> { let generic_iface = question .get(&generic_interface) .context("Question does not have generic interface")?; - let answer: String = get_property(generic_iface, "answer")?; + let answer: String = get_property(generic_iface, "Answer")?; if answer.is_empty() { Ok(None) } else { From 7e550a7575436eb8e65fcf6a4ba6823b2e53b98b Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Sun, 21 Jul 2024 23:13:33 +0200 Subject: [PATCH 11/26] fix questions list to list only unanswered ones --- rust/agama-server/src/questions/web.rs | 68 +++++++++++++------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/rust/agama-server/src/questions/web.rs b/rust/agama-server/src/questions/web.rs index 7615c20911..63400e4070 100644 --- a/rust/agama-server/src/questions/web.rs +++ b/rust/agama-server/src/questions/web.rs @@ -25,7 +25,7 @@ use tokio_stream::{Stream, StreamExt}; use zbus::{ fdo::ObjectManagerProxy, names::{InterfaceName, OwnedInterfaceName}, - zvariant::{ObjectPath, OwnedObjectPath}, + zvariant::{ObjectPath, OwnedObjectPath, OwnedValue}, }; // TODO: move to lib or maybe not and just have in lib client for http API? @@ -34,6 +34,8 @@ struct QuestionsClient<'a> { connection: zbus::Connection, objects_proxy: ObjectManagerProxy<'a>, questions_proxy: Questions1Proxy<'a>, + generic_interface: OwnedInterfaceName, + with_password_interface: OwnedInterfaceName, } impl<'a> QuestionsClient<'a> { @@ -48,6 +50,14 @@ impl<'a> QuestionsClient<'a> { .destination("org.opensuse.Agama1")? .build() .await?, + generic_interface: InterfaceName::from_str_unchecked( + "org.opensuse.Agama1.Questions.Generic", + ) + .into(), + with_password_interface: InterfaceName::from_str_unchecked( + "org.opensuse.Agama1.Questions.WithPassword", + ) + .into(), }) } @@ -96,37 +106,39 @@ impl<'a> QuestionsClient<'a> { .await .context("failed to get managed object with Object Manager")?; let mut result: Vec = Vec::with_capacity(objects.len()); - let password_interface = OwnedInterfaceName::from( - InterfaceName::from_static_str("org.opensuse.Agama1.Questions.WithPassword") - .context("Failed to create interface name for question with password")?, - ); - for (path, interfaces_hash) in objects.iter() { - if interfaces_hash.contains_key(&password_interface) { - result.push(self.build_question_with_password(path).await?) - } else { - result.push(self.build_generic_question(path).await?) + + for (_path, interfaces_hash) in objects.iter() { + let generic_properties = interfaces_hash + .get(&self.generic_interface) + .context("Failed to create interface name for generic question")?; + // skip if question is already answered + let answer: String = get_property(generic_properties, "Answer")?; + if !answer.is_empty() { + continue; } + let mut question = self.build_generic_question(generic_properties)?; + + if interfaces_hash.contains_key(&self.with_password_interface) { + question.with_password = Some(QuestionWithPassword {}); + } + + result.push(question); } Ok(result) } - async fn build_generic_question( + fn build_generic_question( &self, - path: &OwnedObjectPath, + properties: &HashMap, ) -> Result { - let dbus_question = GenericQuestionProxy::builder(&self.connection) - .path(path)? - .cache_properties(zbus::CacheProperties::No) - .build() - .await?; let result = Question { generic: GenericQuestion { - id: dbus_question.id().await?, - class: dbus_question.class().await?, - text: dbus_question.text().await?, - options: dbus_question.options().await?, - default_option: dbus_question.default_option().await?, - data: dbus_question.data().await?, + id: get_property(properties, "Id")?, + class: get_property(properties, "Class")?, + text: get_property(properties, "Text")?, + options: get_property(properties, "Options")?, + default_option: get_property(properties, "DefaultOption")?, + data: get_property(properties, "Data")?, }, with_password: None, }; @@ -134,16 +146,6 @@ impl<'a> QuestionsClient<'a> { Ok(result) } - async fn build_question_with_password( - &self, - path: &OwnedObjectPath, - ) -> Result { - let mut result = self.build_generic_question(path).await?; - result.with_password = Some(QuestionWithPassword {}); - - Ok(result) - } - pub async fn delete(&self, id: u32) -> Result<(), ServiceError> { let question_path = ObjectPath::from( ObjectPath::try_from(format!("/org/opensuse/Agama1/Questions/{}", id)) From 9deacdb04be6d0e47c2ddad8a761a2165723ed36 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Sun, 21 Jul 2024 23:14:06 +0200 Subject: [PATCH 12/26] implement general Question with Dialog component --- web/src/client/questions.js | 2 +- .../questions/QuestionWithPassword.jsx | 69 +++++++++++++++++++ web/src/components/questions/Questions.jsx | 7 +- web/src/components/questions/index.js | 1 + 4 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 web/src/components/questions/QuestionWithPassword.jsx diff --git a/web/src/client/questions.js b/web/src/client/questions.js index 1999d39ba8..d2e822c238 100644 --- a/web/src/client/questions.js +++ b/web/src/client/questions.js @@ -150,4 +150,4 @@ class QuestionsClient { } } -export { QuestionsClient }; +export { QUESTION_TYPES, QuestionsClient }; diff --git a/web/src/components/questions/QuestionWithPassword.jsx b/web/src/components/questions/QuestionWithPassword.jsx new file mode 100644 index 0000000000..984f3c5918 --- /dev/null +++ b/web/src/components/questions/QuestionWithPassword.jsx @@ -0,0 +1,69 @@ +/* + * Copyright (c) [2022] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * 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 General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React, { useState } from "react"; +import { Alert, Form, FormGroup, Text } from "@patternfly/react-core"; +import { Icon } from "~/components/layout"; +import { PasswordInput, Popup } from "~/components/core"; +import { QuestionActions } from "~/components/questions"; +import { _ } from "~/i18n"; + + +export default function QuestionWithPassword({ question, answerCallback }) { + const [password, setPassword] = useState(question.password || ""); + const defaultAction = question.defaultOption; + + const actionCallback = (option) => { + question.password = password; + question.answer = option; + answerCallback(question); + }; + + return ( + } + > + {question.text} +
+ {/* TRANSLATORS: field label */} + + setPassword(value)} + /> + +
+ + + + +
+ ); +} \ No newline at end of file diff --git a/web/src/components/questions/Questions.jsx b/web/src/components/questions/Questions.jsx index 14be33362d..b2f786ef41 100644 --- a/web/src/components/questions/Questions.jsx +++ b/web/src/components/questions/Questions.jsx @@ -22,8 +22,9 @@ import React, { useCallback, useEffect, useState } from "react"; import { useInstallerClient } from "~/context/installer"; import { useCancellablePromise } from "~/utils"; +import { QUESTION_TYPES } from "~/client/questions"; -import { GenericQuestion, LuksActivationQuestion } from "~/components/questions"; +import { GenericQuestion, QuestionWithPassword, LuksActivationQuestion } from "~/components/questions"; export default function Questions() { const client = useInstallerClient(); @@ -73,6 +74,10 @@ export default function Questions() { // Renders the first pending question const [currentQuestion] = pendingQuestions; let QuestionComponent = GenericQuestion; + // show specialized popup for question which need password + if (currentQuestion.type === QUESTION_TYPES.withPassword) { + QuestionComponent = QuestionWithPassword; + } // show specialized popup for luks activation question // more can follow as it will be needed if (currentQuestion.class === "storage.luks_activation") { diff --git a/web/src/components/questions/index.js b/web/src/components/questions/index.js index 2e9b51dc81..15cc7dee28 100644 --- a/web/src/components/questions/index.js +++ b/web/src/components/questions/index.js @@ -21,5 +21,6 @@ export { default as QuestionActions } from "./QuestionActions"; export { default as GenericQuestion } from "./GenericQuestion"; +export { default as QuestionWithPassword } from "./QuestionWithPassword"; export { default as LuksActivationQuestion } from "./LuksActivationQuestion"; export { default as Questions } from "./Questions"; From 62621564ca50897ae637b7fceb0851805e7217fd Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 22 Jul 2024 15:28:18 +0200 Subject: [PATCH 13/26] fix path for deleting question --- rust/agama-lib/src/questions/http_client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/agama-lib/src/questions/http_client.rs b/rust/agama-lib/src/questions/http_client.rs index e498fce21e..0be917d658 100644 --- a/rust/agama-lib/src/questions/http_client.rs +++ b/rust/agama-lib/src/questions/http_client.rs @@ -63,7 +63,7 @@ impl HTTPClient { } pub async fn delete_question(&self, question_id: u32) -> Result<(), ServiceError> { - let path = format!("/questions/{}/answer", question_id); + let path = format!("/questions/{}", question_id); self.client.delete(path.as_str()).await } } From 78d997c30979cadece596d3ab2514b8716c174cb Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 22 Jul 2024 15:50:31 +0200 Subject: [PATCH 14/26] fix parsing of result --- service/lib/agama/autoyast/report_patching.rb | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/service/lib/agama/autoyast/report_patching.rb b/service/lib/agama/autoyast/report_patching.rb index a102ac1cba..36f4194862 100644 --- a/service/lib/agama/autoyast/report_patching.rb +++ b/service/lib/agama/autoyast/report_patching.rb @@ -49,17 +49,17 @@ def show(message, details: "", headline: "", timeout: 0, focus: nil, question = { # TODO: id for newly created question is ignored, but maybe it will # be better to not have to specify it at all? - id: 0, - class: "autoyast.popup", - text: text, - options: generate_options(buttons), - default_option: focus || options.first, - data: {} + id: 0, + class: "autoyast.popup", + text: text, + options: generate_options(buttons), + defaultOption: focus || options.first, + data: {} } data = { generic: question }.to_json - answer_json = Yast::Execute.locally!("agama", "questions", "ask", + answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, stdout: :capture) - answer = JSON.parse!(answer_json) + answer = JSON.parse!(answer_yaml) answer["generic"]["answer"].to_sym end @@ -106,14 +106,14 @@ def run "defaultOption" => "cancel", "data" => {} } - data = { "generic" => question, "withPassword" => {} }.to_json - answer_json = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, + data = { generic: question, withPassword: {} }.to_json + answer_yaml = Yast::Execute.locally!("agama", "questions", "ask", stdin: data, stdout: :capture) - answer = JSON.parse!(answer_json) + answer = JSON.parse!(answer_yaml) result = answer["generic"]["answer"].to_sym return nil if result == "cancel" - answer["with_password"]["password"] + answer["withPassword"]["password"] end end end From 7879a7c938ced061e6e075b6c7e974af4a8a9885 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 22 Jul 2024 15:50:56 +0200 Subject: [PATCH 15/26] Apply suggestions from code review Co-authored-by: Martin Vidner --- service/lib/agama/autoyast/report_patching.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/lib/agama/autoyast/report_patching.rb b/service/lib/agama/autoyast/report_patching.rb index 36f4194862..0ecfcb3edf 100644 --- a/service/lib/agama/autoyast/report_patching.rb +++ b/service/lib/agama/autoyast/report_patching.rb @@ -111,7 +111,7 @@ def run stdout: :capture) answer = JSON.parse!(answer_yaml) result = answer["generic"]["answer"].to_sym - return nil if result == "cancel" + return nil if result == :cancel answer["withPassword"]["password"] end From b7e1a9e660bb5c3099469bac9f1eeb920083ad72 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 22 Jul 2024 17:16:00 +0200 Subject: [PATCH 16/26] do not require id for newly created question --- rust/agama-cli/src/questions.rs | 7 +++++-- rust/agama-lib/src/questions/model.rs | 3 ++- rust/agama-server/src/questions/web.rs | 6 +++--- service/lib/agama/autoyast/report_patching.rb | 6 ------ 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/rust/agama-cli/src/questions.rs b/rust/agama-cli/src/questions.rs index 7dfe1ae709..fa2ef1d34b 100644 --- a/rust/agama-cli/src/questions.rs +++ b/rust/agama-cli/src/questions.rs @@ -72,11 +72,14 @@ async fn ask_question() -> Result<(), ServiceError> { let question = serde_json::from_reader(std::io::stdin())?; let created_question = client.create_question(&question).await?; - let answer = client.get_answer(created_question.generic.id).await?; + let Some(id) = created_question.generic.id else { + return Err(ServiceError::QuestionNotExist(0)); + }; + let answer = client.get_answer(id).await?; let answer_json = serde_json::to_string_pretty(&answer).map_err(Into::::into)?; println!("{}", answer_json); - client.delete_question(created_question.generic.id).await?; + client.delete_question(id).await?; Ok(()) } diff --git a/rust/agama-lib/src/questions/model.rs b/rust/agama-lib/src/questions/model.rs index 6d1a87d72f..df35c0763f 100644 --- a/rust/agama-lib/src/questions/model.rs +++ b/rust/agama-lib/src/questions/model.rs @@ -19,7 +19,8 @@ pub struct Question { #[derive(Clone, Serialize, Deserialize, utoipa::ToSchema)] #[serde(rename_all = "camelCase")] pub struct GenericQuestion { - pub id: u32, + /// id is optional as newly created questions does not have it assigned + pub id: Option, pub class: String, pub text: String, pub options: Vec, diff --git a/rust/agama-server/src/questions/web.rs b/rust/agama-server/src/questions/web.rs index 63400e4070..f7054193ee 100644 --- a/rust/agama-server/src/questions/web.rs +++ b/rust/agama-server/src/questions/web.rs @@ -94,8 +94,8 @@ impl<'a> QuestionsClient<'a> { .await? }; let mut res = question.clone(); - res.generic.id = extract_id_from_path(&path)?; - tracing::info!("new question gets id {}", res.generic.id); + res.generic.id = Some(extract_id_from_path(&path)?); + tracing::info!("new question gets id {:?}", res.generic.id); Ok(res) } @@ -133,7 +133,7 @@ impl<'a> QuestionsClient<'a> { ) -> Result { let result = Question { generic: GenericQuestion { - id: get_property(properties, "Id")?, + id: Some(get_property(properties, "Id")?), class: get_property(properties, "Class")?, text: get_property(properties, "Text")?, options: get_property(properties, "Options")?, diff --git a/service/lib/agama/autoyast/report_patching.rb b/service/lib/agama/autoyast/report_patching.rb index 0ecfcb3edf..79f37943f2 100644 --- a/service/lib/agama/autoyast/report_patching.rb +++ b/service/lib/agama/autoyast/report_patching.rb @@ -47,9 +47,6 @@ def show(message, details: "", headline: "", timeout: 0, focus: nil, text += "\n\n" + details unless details.empty? options = generate_options(buttons) question = { - # TODO: id for newly created question is ignored, but maybe it will - # be better to not have to specify it at all? - id: 0, class: "autoyast.popup", text: text, options: generate_options(buttons), @@ -97,9 +94,6 @@ def run # at first construct agama question to display. text = @label question = { - # TODO: id for newly created question is ignored, but maybe it will - # be better to not have to specify it at all? - "id" => 0, "class" => "autoyast.password", "text" => text, "options" => ["ok", "cancel"], From 85a71598e6b2bb2e322166f20d4b917f093f0ff3 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 22 Jul 2024 17:29:09 +0200 Subject: [PATCH 17/26] changes --- rust/package/agama.changes | 7 +++++++ service/package/rubygem-agama-yast.changes | 6 ++++++ web/package/agama-web-ui.changes | 6 ++++++ 3 files changed, 19 insertions(+) diff --git a/rust/package/agama.changes b/rust/package/agama.changes index 970368fce3..422153939d 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Mon Jul 22 15:27:44 UTC 2024 - Josef Reidinger + +- Fix `agama questions list` to list only unaswered questions and + improve its performance + (gh#openSUSE/agama#1476) + ------------------------------------------------------------------- Wed Jul 17 11:15:33 UTC 2024 - Jorik Cronenberg diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index 5defe09148..35da1c5e61 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Jul 22 15:26:48 UTC 2024 - Josef Reidinger + +- Autoyast convert script: Use agama questions to report errors + and ask when encrypted profile is used (gh#openSUSE/agama#1476) + ------------------------------------------------------------------- Fri Jul 12 11:03:14 UTC 2024 - Imobach Gonzalez Sosa diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index 9bbd0fac95..2b3de5b9e8 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Jul 22 15:28:42 UTC 2024 - Josef Reidinger + +- Add support for generic questions with password + (gh#openSUSE/agama#1476) + ------------------------------------------------------------------- Wed Jul 17 09:52:36 UTC 2024 - Imobach Gonzalez Sosa From c44f3af735d946594c917daca5545efbd24d0f7f Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 23 Jul 2024 11:03:26 +0200 Subject: [PATCH 18/26] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Imobach González Sosa Co-authored-by: David Díaz <1691872+dgdavid@users.noreply.github.com> --- rust/agama-cli/src/questions.rs | 4 ++-- rust/agama-server/src/questions/web.rs | 4 ++-- service/README.md | 8 ++++---- service/package/rubygem-agama-yast.changes | 2 +- web/src/components/questions/QuestionWithPassword.jsx | 3 +-- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/rust/agama-cli/src/questions.rs b/rust/agama-cli/src/questions.rs index fa2ef1d34b..484e0438e5 100644 --- a/rust/agama-cli/src/questions.rs +++ b/rust/agama-cli/src/questions.rs @@ -20,9 +20,9 @@ pub enum QuestionsCommands { /// Path to a file containing the answers in YAML format. path: String, }, - /// prints list of questions that is waiting for answer in JSON format + /// Prints the list of questions that are waiting for an answer in JSON format List, - /// Ask question from stdin in JSON format and print answer when it is answered. + /// Reads a question definition in JSON from stdin and prints the response when it is answered. Ask, } diff --git a/rust/agama-server/src/questions/web.rs b/rust/agama-server/src/questions/web.rs index f7054193ee..792ad7453c 100644 --- a/rust/agama-server/src/questions/web.rs +++ b/rust/agama-server/src/questions/web.rs @@ -71,7 +71,7 @@ impl<'a> QuestionsClient<'a> { .map(|(k, v)| (k.as_str(), v.as_str())) .collect(); let path = if question.with_password.is_some() { - tracing::info!("creating question with password"); + tracing::info!("creating a question with password"); self.questions_proxy .new_with_password( &generic.class, @@ -82,7 +82,7 @@ impl<'a> QuestionsClient<'a> { ) .await? } else { - tracing::info!("creating generic question"); + tracing::info!("creating a generic question"); self.questions_proxy .new_question( &generic.class, diff --git a/service/README.md b/service/README.md index 63261976aa..2db0a7d6d6 100644 --- a/service/README.md +++ b/service/README.md @@ -6,9 +6,9 @@ According to [Agama's architecture](../doc/architecture.md) this project impleme ## Testing Changes -The easiest way to test changes done to ruby code on agama liveCD is to build -gem with modified sources with `gem build agama-yast`. Then copy resulting file -to agama live ISO. There do this sequence of commands: +The easiest way to test changes done to Ruby code on Agama live media is to build +the gem with modified sources with `gem build agama-yast`. Then copy the resulting file +to Agama live image. Then run this sequence of commands: ```sh # ensure that only modified sources are installed @@ -17,5 +17,5 @@ gem uninstall agama-yast gem install --no-doc --no-format-executable ``` -If change modifies also dbus parts, then restart related dbus services. +If the changes modify the D-Bus part, then restart related D-Bus services. diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index 35da1c5e61..5df57f925c 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,7 +1,7 @@ ------------------------------------------------------------------- Mon Jul 22 15:26:48 UTC 2024 - Josef Reidinger -- Autoyast convert script: Use agama questions to report errors +- AutoYaST convert script: use Agama questions to report errors and ask when encrypted profile is used (gh#openSUSE/agama#1476) ------------------------------------------------------------------- diff --git a/web/src/components/questions/QuestionWithPassword.jsx b/web/src/components/questions/QuestionWithPassword.jsx index 984f3c5918..582be2b07a 100644 --- a/web/src/components/questions/QuestionWithPassword.jsx +++ b/web/src/components/questions/QuestionWithPassword.jsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2024] SUSE LLC * * All Rights Reserved. * @@ -41,7 +41,6 @@ export default function QuestionWithPassword({ question, answerCallback }) { } > {question.text} From 2e9d7e7839df2cb9185bdb366738e4b73f809537 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 23 Jul 2024 14:05:19 +0200 Subject: [PATCH 19/26] introduce internal error --- rust/agama-cli/src/questions.rs | 8 +++----- rust/agama-lib/src/error.rs | 3 +++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/rust/agama-cli/src/questions.rs b/rust/agama-cli/src/questions.rs index 484e0438e5..d6861bd402 100644 --- a/rust/agama-cli/src/questions.rs +++ b/rust/agama-cli/src/questions.rs @@ -57,12 +57,10 @@ async fn set_answers(proxy: Questions1Proxy<'_>, path: String) -> Result<(), Ser async fn list_questions() -> Result<(), ServiceError> { let client = HTTPClient::new().await?; let questions = client.list_questions().await?; - // FIXME: that conversion to anyhow error is nasty, but we do not expect issue - // when questions are already read from json // FIXME: if performance is bad, we can skip converting json from http to struct and then // serialize it, but it won't be pretty string let questions_json = - serde_json::to_string_pretty(&questions).map_err(Into::::into)?; + serde_json::to_string_pretty(&questions).map_err(|e| ServiceError::InternalError(e.to_string()))?; println!("{}", questions_json); Ok(()) } @@ -73,10 +71,10 @@ async fn ask_question() -> Result<(), ServiceError> { let created_question = client.create_question(&question).await?; let Some(id) = created_question.generic.id else { - return Err(ServiceError::QuestionNotExist(0)); + return Err(ServiceError::InternalError("Created question does not get id".to_string())); }; let answer = client.get_answer(id).await?; - let answer_json = serde_json::to_string_pretty(&answer).map_err(Into::::into)?; + let answer_json = serde_json::to_string_pretty(&answer).map_err(|e| ServiceError::InternalError(e.to_string()))?; println!("{}", answer_json); client.delete_question(id).await?; diff --git a/rust/agama-lib/src/error.rs b/rust/agama-lib/src/error.rs index b5809c73b7..ed67f8f50f 100644 --- a/rust/agama-lib/src/error.rs +++ b/rust/agama-lib/src/error.rs @@ -43,6 +43,9 @@ pub enum ServiceError { BackendError(u16, String), #[error("You are not logged in. Please use: agama auth login")] NotAuthenticated, + // Specific error when something does not work as expected, but it is not user fault + #[error("Internal error. Please report a bug and attach logs. Details: {0}")] + InternalError(String), } #[derive(Error, Debug)] From 2d7fff4040545a8316c6c6ad42fb0aba73fad4a8 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 23 Jul 2024 14:06:09 +0200 Subject: [PATCH 20/26] remove duplicite popup now --- service/lib/yast2/popup.rb | 74 -------------------------------------- 1 file changed, 74 deletions(-) delete mode 100644 service/lib/yast2/popup.rb diff --git a/service/lib/yast2/popup.rb b/service/lib/yast2/popup.rb deleted file mode 100644 index ec1af2a5b3..0000000000 --- a/service/lib/yast2/popup.rb +++ /dev/null @@ -1,74 +0,0 @@ -# frozen_string_literal: true - -# -# Copyright (c) [2024] SUSE LLC -# -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or modify it -# under the terms of version 2 of the GNU General Public License as published -# by the Free Software Foundation. -# -# 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 General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, contact SUSE LLC. -# -# To contact SUSE LLC about this file by physical or electronic mail, you may -# find current contact information at www.suse.com. - -require "yast" -require "agama/dbus/clients/questions" - -module Yast2 - # Replacement to the Yast2::Popup class to work with Agama. - class Popup - class << self - # rubocop:disable Metrics/ParameterLists - # rubocop:disable Lint/UnusedMethodArgument - def show(message, details: "", headline: "", timeout: 0, focus: nil, buttons: :ok, - richtext: false, style: :notice) - - question = Agama::Question.new( - qclass: "popup", - text: message, - options: generate_options(buttons), - default_option: focus - ) - questions_client.ask(question) - end - - private - - # FIXME: inject the logger - def logger - @logger = Logger.new($stdout) - end - - def generate_options(buttons) - case buttons - when :ok - [:ok] - when :continue_cancel - [:continue, :cancel] - when :yes_no - [:yes, :no] - else - raise ArgumentError, "Invalid value #{buttons.inspect} for buttons" - end - end - - # Returns the client to ask questions - # - # @return [Agama::DBus::Clients::Questions] - def questions_client - @questions_client ||= Agama::DBus::Clients::Questions.new(logger: logger) - end - end - end -end -# rubocop:enable Metrics/ParameterLists -# rubocop:enable Lint/UnusedMethodArgument From 20ea50a9b8872c9c5fa1bfaa89bb854767425db1 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 23 Jul 2024 14:26:04 +0200 Subject: [PATCH 21/26] fix formatting --- .../questions/QuestionWithPassword.jsx | 3 +-- web/src/components/questions/Questions.jsx | 6 ++++- web/src/languages.json | 22 +++++++++---------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/web/src/components/questions/QuestionWithPassword.jsx b/web/src/components/questions/QuestionWithPassword.jsx index 582be2b07a..14e534ec44 100644 --- a/web/src/components/questions/QuestionWithPassword.jsx +++ b/web/src/components/questions/QuestionWithPassword.jsx @@ -26,7 +26,6 @@ import { PasswordInput, Popup } from "~/components/core"; import { QuestionActions } from "~/components/questions"; import { _ } from "~/i18n"; - export default function QuestionWithPassword({ question, answerCallback }) { const [password, setPassword] = useState(question.password || ""); const defaultAction = question.defaultOption; @@ -65,4 +64,4 @@ export default function QuestionWithPassword({ question, answerCallback }) { ); -} \ No newline at end of file +} diff --git a/web/src/components/questions/Questions.jsx b/web/src/components/questions/Questions.jsx index b2f786ef41..4280a4cf6b 100644 --- a/web/src/components/questions/Questions.jsx +++ b/web/src/components/questions/Questions.jsx @@ -24,7 +24,11 @@ import { useInstallerClient } from "~/context/installer"; import { useCancellablePromise } from "~/utils"; import { QUESTION_TYPES } from "~/client/questions"; -import { GenericQuestion, QuestionWithPassword, LuksActivationQuestion } from "~/components/questions"; +import { + GenericQuestion, + QuestionWithPassword, + LuksActivationQuestion, +} from "~/components/questions"; export default function Questions() { const client = useInstallerClient(); diff --git a/web/src/languages.json b/web/src/languages.json index 8c64959024..d406f186b0 100644 --- a/web/src/languages.json +++ b/web/src/languages.json @@ -1,12 +1,12 @@ { - "ca-es": "Català", - "de-de": "Deutsch", - "en-us": "English", - "es-es": "Español", - "ja-jp": "日本語", - "nb-NO": "Norsk bokmål", - "pt-BR": "Português", - "ru-ru": "Русский", - "sv-se": "Svenska", - "zh-Hans": "中文" -} \ No newline at end of file + "ca-es": "Català", + "de-de": "Deutsch", + "en-us": "English", + "es-es": "Español", + "ja-jp": "日本語", + "nb-NO": "Norsk bokmål", + "pt-BR": "Português", + "ru-ru": "Русский", + "sv-se": "Svenska", + "zh-Hans": "中文" +} From 097f14f49b5128b9e8a6b52f14c5c1d5b70b9351 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 23 Jul 2024 14:26:34 +0200 Subject: [PATCH 22/26] add test for question with password component --- .../questions/QuestionWithPassword.test.jsx | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 web/src/components/questions/QuestionWithPassword.test.jsx diff --git a/web/src/components/questions/QuestionWithPassword.test.jsx b/web/src/components/questions/QuestionWithPassword.test.jsx new file mode 100644 index 0000000000..03603fedda --- /dev/null +++ b/web/src/components/questions/QuestionWithPassword.test.jsx @@ -0,0 +1,71 @@ +/* + * Copyright (c) [2022] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * 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 General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen } from "@testing-library/react"; +import { installerRender } from "~/test-utils"; +import { QuestionWithPassword } from "~/components/questions"; + +let question; +const answerFn = jest.fn(); + +const renderQuestion = () => + installerRender(); + +describe("QuestionWithPassword", () => { + beforeEach(() => { + question = { + id: 1, + class: "question.password", + text: "Random question. Will you provide random password?", + options: ["ok", "cancel"], + defaultOption: "cancel", + data: {}, + }; + }); + + it("renders the question text", async () => { + renderQuestion(); + + await screen.findByText(question.text); + }); + + it("contains a textinput for entering the password", async () => { + renderQuestion(); + + const passwordInput = await screen.findByLabelText("Password"); + expect(passwordInput).not.toBeNull(); + }); + + describe("when the user selects one of the options", () => { + it("calls the callback after setting both, answer and password", async () => { + const { user } = renderQuestion(); + + const passwordInput = await screen.findByLabelText("Password"); + await user.type(passwordInput, "notSecret"); + const skipButton = await screen.findByRole("button", { name: /Ok/ }); + await user.click(skipButton); + + expect(question).toEqual(expect.objectContaining({ password: "notSecret", answer: "ok" })); + expect(answerFn).toHaveBeenCalledWith(question); + }); + }); +}); From 33f7c75ab329693f1365689ac7d3994cf34f498e Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 23 Jul 2024 14:29:01 +0200 Subject: [PATCH 23/26] relax condition of products test to not fail when we change product --- service/test/agama/software/manager_test.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/service/test/agama/software/manager_test.rb b/service/test/agama/software/manager_test.rb index 8643608c20..d090f027ce 100644 --- a/service/test/agama/software/manager_test.rb +++ b/service/test/agama/software/manager_test.rb @@ -251,11 +251,7 @@ it "returns the list of known products" do products = subject.products expect(products).to all(be_a(Agama::Software::Product)) - expect(products).to contain_exactly( - an_object_having_attributes(id: "Tumbleweed"), - an_object_having_attributes(id: "MicroOS"), - an_object_having_attributes(id: "Leap_16.0") - ) + expect(products).to_not be_empty end end From 8bc9b1b2b7b34a65d6efd392e2a8946e30f08459 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 23 Jul 2024 14:39:47 +0200 Subject: [PATCH 24/26] fix formatting --- rust/agama-cli/src/questions.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/rust/agama-cli/src/questions.rs b/rust/agama-cli/src/questions.rs index d6861bd402..6d0d9c05a8 100644 --- a/rust/agama-cli/src/questions.rs +++ b/rust/agama-cli/src/questions.rs @@ -59,8 +59,8 @@ async fn list_questions() -> Result<(), ServiceError> { let questions = client.list_questions().await?; // FIXME: if performance is bad, we can skip converting json from http to struct and then // serialize it, but it won't be pretty string - let questions_json = - serde_json::to_string_pretty(&questions).map_err(|e| ServiceError::InternalError(e.to_string()))?; + let questions_json = serde_json::to_string_pretty(&questions) + .map_err(|e| ServiceError::InternalError(e.to_string()))?; println!("{}", questions_json); Ok(()) } @@ -71,10 +71,13 @@ async fn ask_question() -> Result<(), ServiceError> { let created_question = client.create_question(&question).await?; let Some(id) = created_question.generic.id else { - return Err(ServiceError::InternalError("Created question does not get id".to_string())); + return Err(ServiceError::InternalError( + "Created question does not get id".to_string(), + )); }; let answer = client.get_answer(id).await?; - let answer_json = serde_json::to_string_pretty(&answer).map_err(|e| ServiceError::InternalError(e.to_string()))?; + let answer_json = serde_json::to_string_pretty(&answer) + .map_err(|e| ServiceError::InternalError(e.to_string()))?; println!("{}", answer_json); client.delete_question(id).await?; From fc249f873021ffc2d2f7bb478493d5b4f3e2bc31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 23 Jul 2024 14:28:33 +0100 Subject: [PATCH 25/26] test(web): simplify QuestionWithPassword tests --- .../questions/QuestionWithPassword.test.jsx | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/web/src/components/questions/QuestionWithPassword.test.jsx b/web/src/components/questions/QuestionWithPassword.test.jsx index 03603fedda..1bbc50ef51 100644 --- a/web/src/components/questions/QuestionWithPassword.test.jsx +++ b/web/src/components/questions/QuestionWithPassword.test.jsx @@ -21,14 +21,14 @@ import React from "react"; import { screen } from "@testing-library/react"; -import { installerRender } from "~/test-utils"; +import { plainRender } from "~/test-utils"; import { QuestionWithPassword } from "~/components/questions"; let question; const answerFn = jest.fn(); const renderQuestion = () => - installerRender(); + plainRender(); describe("QuestionWithPassword", () => { beforeEach(() => { @@ -42,17 +42,10 @@ describe("QuestionWithPassword", () => { }; }); - it("renders the question text", async () => { + it("renders the question text", () => { renderQuestion(); - await screen.findByText(question.text); - }); - - it("contains a textinput for entering the password", async () => { - renderQuestion(); - - const passwordInput = await screen.findByLabelText("Password"); - expect(passwordInput).not.toBeNull(); + screen.queryByText(question.text); }); describe("when the user selects one of the options", () => { From 5cd74da4b72167513841a9c6a8c9b40c935dd3ac Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 23 Jul 2024 18:09:57 +0200 Subject: [PATCH 26/26] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Imobach González Sosa Co-authored-by: David Díaz <1691872+dgdavid@users.noreply.github.com> --- web/src/components/questions/QuestionWithPassword.test.jsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/src/components/questions/QuestionWithPassword.test.jsx b/web/src/components/questions/QuestionWithPassword.test.jsx index 1bbc50ef51..c9831d72d0 100644 --- a/web/src/components/questions/QuestionWithPassword.test.jsx +++ b/web/src/components/questions/QuestionWithPassword.test.jsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2024] SUSE LLC * * All Rights Reserved. * @@ -48,8 +48,8 @@ describe("QuestionWithPassword", () => { screen.queryByText(question.text); }); - describe("when the user selects one of the options", () => { - it("calls the callback after setting both, answer and password", async () => { + describe("when the user enters the password", () => { + it("calls the callback", async () => { const { user } = renderQuestion(); const passwordInput = await screen.findByLabelText("Password");