Skip to content

Pre-installation checks#299

Merged
imobachgs merged 32 commits intomasterfrom
pre-install-checks
Nov 14, 2022
Merged

Pre-installation checks#299
imobachgs merged 32 commits intomasterfrom
pre-install-checks

Conversation

@imobachgs
Copy link
Contributor

@imobachgs imobachgs commented Nov 11, 2022

Problem

D-Installer does not perform any pre-installation check. So if you click the "Install" button, it will blindly try to install the system. It does not matter whether you have provided all the required information.

See #285 or #177 as examples.

Solution

This is the first version of a mechanism to report the issues that might block the installation. We might add some additional information to these validation errors in the future (severity, scope, etc.), but we need to start with something simple enough.

The implementation has two parts:

  • A new org.opensuse.DInstaller.Validation1 that exposes the validation errors. The D-Bus layer takes care of keeping this information in sync, so the only thing that the backend classes need to provide is a #validate method (like Manager#validate or Users#validate).
  • Support at UI level to display these validation errors:
    • In the users section, the errors are shown after the user clicks the Install button.
    • In storage, the errors are always visible.

Demo

User and storage checks

demo-d-installer-checks.webm

Multiple errors

When there are multiple errors, they are shown in a pop-over. Of course, in the example below, the second is a testing error. Bear in mind that, by now, this situation should not happen.

multiple-errors-description

TODO

  • Improve how errors are presented
  • Document the Validation1 interface
  • Extract the InstallButton component to a separate file
  • Fix unit tests
  • Rename IsValid to just Valid.

@coveralls
Copy link

coveralls commented Nov 11, 2022

Coverage Status

Coverage increased (+0.07%) to 74.747% when pulling 93b99e8 on pre-install-checks into 1e3f516 on master.

@imobachgs imobachgs marked this pull request as ready for review November 14, 2022 05:51
@imobachgs imobachgs linked an issue Nov 14, 2022 that may be closed by this pull request
Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only minor things. For the UI part ... I did my best to understand it :), but it looks ok. Great work!

Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@imobachgs imobachgs merged commit 30bd6dd into master Nov 14, 2022
@imobachgs imobachgs deleted the pre-install-checks branch November 14, 2022 15:40
This was referenced Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants