feat: ask for license acceptance by imobachgs · Pull Request #1922 · agama-project/agama

imobachgs · 2025-01-20T07:11:01Z

Problem

Agama does not show the license of any product and does not ask the user to accept it. This is a requirement for beta 1, and it needs to be implemented as soon as possible.

Solution

Extend Agama to allow the specification of a license per product. The license should be accepted on the product selection page. The solution does not cover all cases (e.g., openSUSE licenses are not there yet) and the technical details might change soon. But it is expected to be good enough for beta 1.

The licenses are included in /usr/share/agama/eula with one directory per license. The directory name is used as the license ID (license.beta and license.final) and it contains the translations.

Licenses

The licenses come from the https://build.suse.de/package/show/SUSE:SLE-15-SP7:GA/skelcd package.

Testing

Added a new unit test
Tested manually

Screenshots

Click to show/hide some screenshots

Be aware below screenshots were taken using mock data for emulating a license in a product and illustrating the new interface controls and behavior.

Product without license	Product with license

License accepted for a product not selected yet	License accepted for a product previously selected

License dialog

Made adjustments to support an acceptable layout for actions on the product selection page. This change is expected to be temporary until the migration to PatternFly 6 is completed, at which point the entire page will be revamped.

Still pending retrieving the license to display it in the dialog open when user clicks on the license link.

Apart from displaying only available languages, still pending interface improvements and unit tests.

coveralls · 2025-01-20T07:15:37Z

Pull Request Test Coverage Report for Build 12866228169

Details

8 of 21 (38.1%) changed or added relevant lines in 6 files are covered.
124 unchanged lines in 6 files lost coverage.
Overall coverage increased (+0.005%) to 70.966%

Changes Missing Coverage	Changed/Added Lines	%
rust/agama-lib/src/product/client.rs	2	0.0%
service/lib/agama/dbus/software/product.rb	2	0.0%
rust/agama-server/src/software/web.rs	9	0.0%

Files with Coverage Reduction	New Missed Lines	%
service/service/lib/agama/software/product_builder.rb	1	96.0%
service/service/lib/agama/software/product.rb	1	97.62%
service/service/lib/agama/software/manager.rb	9	74.58%
service/service/lib/agama/dbus/software/product.rb	11	84.26%
rust/agama-lib/src/product/client.rs	25	0.0%
rust/agama-server/src/software/web.rs	77	0.0%

Totals
Change from base Build 12836679478:	0.005%
Covered Lines:	17188
Relevant Lines:	24220

💛 - Coveralls

They were there just for testing purposes.

jreidinger · 2025-01-20T13:22:00Z

live/root/usr/share/agama/eula/license.beta/license.cs.txt

+Jiné.  Uplatnění Konvence OSN o smlouvách v mezinárodním obchodě se 
+zbožím je tímto výslovně vyloučeno.
+
+©2013 SUSE LLC nebo její přidružené společnosti.  Všechna práva 


license copyright looks quite old :)

Yes, indeed.

jreidinger · 2025-01-20T13:24:34Z

rust/agama-server/src/software/license.rs

+
+        let body: String = std::fs::read_to_string(license_path).ok()?;
+
+        Some(LicenseContent {


I would like to see here at least a bit of logging, like candidates and found licese so we know from logs why given license is shown.

IMHO that's too much logging just for retrieving a license (the logic is pretty straightforward). But I can live with that :-)

jreidinger · 2025-01-20T13:30:38Z

rust/agama-server/src/software/license.rs

+    /// The language is inferred from the file name (e.g., "es-ES" for license.es_ES.txt").
+    fn language_tag_from_file(name: &str) -> Option<LanguageTag> {
+        if !name.starts_with("license") {
+            return None;


this basically means there is some mess in our directory. Maybe we should also log here.

jreidinger · 2025-01-20T13:31:04Z

rust/agama-server/src/software/web.rs


+    let mut licenses_repo = LicensesRepo::default();
+    if let Err(error) = licenses_repo.read() {
+        tracing::error!("Could not read the licenses repository: {:?}", error);


jreidinger · 2025-01-20T13:32:28Z

rust/agama-server/src/software/web.rs

    let software_issues = issues_router(&dbus, DBUS_SERVICE, DBUS_PATH).await?;
    let product_issues = issues_router(&dbus, DBUS_SERVICE, DBUS_PRODUCT_PATH).await?;

+    let mut licenses_repo = LicensesRepo::default();


so if read below failed it will act as NulObject? And not display any license?

It considers there repository is empty. We could add some logging.

mvidner

For traceability, we should mention where we took the license texts from

in this commit message 'feat(live): add licenses to the live image '
or in the PR description (which will get committed too)

jreidinger · 2025-01-20T13:44:01Z

web/src/components/product/ProductSelectionPage.tsx

  const { products, selectedProduct } = useProduct({ suspense: true });
  const [nextProduct, setNextProduct] = useState(selectedProduct);
+  // FIXME: should not be accepted by default first selectedProduct is accepted
+  // because it's a singleProduct iso.


this can be really issue with s390 SLES iso as it is single product. So I would ask in advance

In a single product ISO, if there is a license, the product will not be auto-selected. So we should be safe.

imobachgs · 2025-01-20T14:46:08Z

For traceability, we should mention where we took the license texts from
* in this commit message 'feat(live): add licenses to the live image '

* or in the PR description (which will get committed too)

Good point. I am adding it to the PR description. Thanks!

Prepare for releasing Agama 12: * #1858 * #1887 * #1890 * #1892 * #1893 * #1894 * #1896 * #1898 * #1899 * #1900 * #1901 * #1906 * #1908 * #1909 * #1910 * #1911 * #1912 * #1914 * #1915 * #1917 * #1919 * #1920 * #1921 * #1922 * #1923 * #1924 * #1926 * #1927 * #1928 * #1929 * #1930 * #1931 * #1932 * #1933 * #1934 * #1935 * #1936 * #1937 * #1938 * #1939 * #1942 * #1943 * #1945 * #1948 * #1949 * #1952 * #1953 * #1954 * #1955 * #1957 * #1958 * #1959 * #1961 * #1963 * #1964 * #1967 * #1969 * #1970 * #1971 * #1972 * #1973 * #1974 * #1975 * #1976 * #1977 * #1979 * #1980 * #1981 * #1982 * #1984 * #1986 * #1987 * #1988 * #1990 * #1991 * #1992 * #1993 * #1995 * #1996 * #1997 * #1999 * #2000 * #2001 * #2002 * #2003 * #2004 * #2005 * #2006 * #2007 * #2008 * #2009 * #2010 * #2011 * #2012 * #2013 * #2014 * #2015 * #2016 * #2017 * #2019 * #2021 * #2022 * #2025 * #2027 * #2029 * #2030 * #2031 * #2033 * #2034 * #2035 * #2036 * #2037 * #2038 * #2039 * #2040 * #2045 * #2046 * #2050 * #2053 * #2054 * #2055 * #2056 * #2058 * #2060 * #2061 * #2062 * #2063 * #2064 * #2066 * #2067 * #2068 * #2069 * #2070 * #2071 * #2072 * #2073 * #2075 * #2076 * #2080 * #2082 * #2083

imobachgs and others added 16 commits January 17, 2025 12:15

feat(rust): add an HTTP API to get licenses information

9249a0e

feat(service): add a license_id to the products

b5a2524

feat(rust): expose the license_id in the list of products

1ece607

fix(rust): use camel case for the license ID

24fedc6

fix(service): do not crash when exporting a nil license_id

bb6da7a

test: set license id values just for testing

ac24f3a

feat(live): add licenses to the live image

025992d

doc: improve license module documentation

3049292

fix(rust): fix language tag parsing

4330e12

refactor(web): allow not justified Page.Actions

11e6800

Made adjustments to support an acceptable layout for actions on the product selection page. This change is expected to be temporary until the migration to PatternFly 6 is completed, at which point the entire page will be revamped.

feat(web): show license acceptance in product selection

e5eb11f

Still pending retrieving the license to display it in the dialog open when user clicks on the license link.

feat(web): allow displaying a product license

9046792

Apart from displaying only available languages, still pending interface improvements and unit tests.

feat(web): add basic unit tests for license dialog

6ea77a9

fix(web): please ESLint

e721698

refactor(rust): better error reporting in the license module

587f4c9

fix(rust): better error reporting software::web

40043da

dgdavid and others added 4 commits January 20, 2025 07:37

fix(web): allow license dialog adjust its size

441167e

fix(web): adjust license dialog title

edf69f5

fix(service): auto-select single product if there is no license

3a7ec98

refactor: rename license_id to license

72de380

imobachgs marked this pull request as ready for review January 20, 2025 10:22

imobachgs added 4 commits January 20, 2025 10:39

doc: update changes files

6a092fc

Merge branch 'master' into eula-support

24ecaa9

Merge branch 'master' into eula-support

c885aa1

fix(products): remove Leap and Slowroll licenses

32d028f

They were there just for testing purposes.

jreidinger reviewed Jan 20, 2025

View reviewed changes

mvidner reviewed Jan 20, 2025

View reviewed changes

jreidinger reviewed Jan 20, 2025

View reviewed changes

jreidinger approved these changes Jan 20, 2025

View reviewed changes

chore(rust): add more logging to license module

3381eed

jreidinger approved these changes Jan 20, 2025

View reviewed changes

imobachgs merged commit 31823f4 into master Jan 20, 2025
10 checks passed

imobachgs deleted the eula-support branch January 20, 2025 14:58

imobachgs mentioned this pull request Feb 26, 2025

Update to release 12 #2084

Merged


		let body: String = std::fs::read_to_string(license_path).ok()?;

		Some(LicenseContent {

Conversation

imobachgs commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Licenses

Testing

Screenshots

Uh oh!

coveralls commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 12866228169

Details

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvidner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imobachgs commented Jan 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

imobachgs commented Jan 20, 2025 •

edited

Loading

coveralls commented Jan 20, 2025 •

edited

Loading