Skip to content

Conversation

@teclator
Copy link
Contributor

@teclator teclator commented Jun 2, 2025

Changes added by #2402 but targeting rc1.

dgdavid and others added 30 commits June 2, 2025 20:09
Extends PF/Switch to include a label and description, providing
additional context for toggles.
This component renders brief notes or clarifications with an optional
icon, using the <strong> HTML element to emphasize the text. It is
intended for non-<Alert> annotations that still require emphasis.
This introduces a UI warning that should be shown only when all available
network connections are configured for installation use only.

Currently, the warning is always rendered; future commits should implement
the proper logic to show it conditionally.
Introduce the AllConnectionsStatusAlert component to display a contextual
summary of how network connections are configured (installation-only,
persistent, or mixed).

The alert supports optional bulk actions to adjust all connections and
is conditionally rendered based on connection count and mode.

It replaces the static warning from the previous commit, but similar to
it is always rendered in mixed mode until future commits implement the
proper logic to show it conditionally depending on the real connections
status.
This adds a toggle allowing users to mark a network connection as
either transient (used only during installation) or persistent
(available in the installed system).

Also includes minor related fixes:
- Adjusted flex direction in SwitchEnhanced
- Added missing exports in components/core
A tiny bug detected by unit tests added in previous commit.
Also add missing unit tests for the UI control responsible for toggling
the "keep" flag on a connection using this mutation.
Although its "bulk actions" are still pending and not functional at all.
Allows expressions like `if (!isEmpty(variable)) doSomething()`
to work when `variable` is a boolean (e.g., `true`).

An example use case can be found in `types/network.ts`,
which was adapted to use the updated function.
Replaced the previous AllConnectionsStatusAlert component with a simpler
NoPersistentConnectionsAlert that displays a warning only when all
network connections are set to be transient (not persisted in the
installed system).

Removed unused complexity such as mixed-mode detection and global
actions, which may be revisited in future iterations.
teclator and others added 10 commits June 3, 2025 09:41
The nested hash string values are surrounded by quotes
when using `.to_string()` but not with `try_into()`.
)

## Problem

When a connection is created bind to an interface renamed with the
kernel ifname kernel cmdline argument, it could be problematic as the
parameter is not kept after the installation.

- https://bugzilla.suse.com/show_bug.cgi?id=1241969
- https://bugzilla.suse.com/show_bug.cgi?id=1237327

## Solution

Create a systemd network link file for renaming the interface after the
installation

## Testing

- *Added unit test for copying the link systemd network link files**
- *Tested manually*

![image](https://github.com/user-attachments/assets/834a45ba-7d7c-4f4a-829a-84fb7426965d)
@teclator teclator marked this pull request as ready for review June 3, 2025 09:19
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

We need these fixes in RC1. But I would like to have a closer look when working on the master branch version.

ieee_8021x_config: Default::default(),
autoconnect: true,
state: Default::default(),
keep: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

master: Should we consider using copy? For me it sounds inconsistent having keep and copy.

Copy link
Contributor Author

@teclator teclator Jun 4, 2025

Choose a reason for hiding this comment

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

what about: persistent ... at the end it is about persist or not to disk using the enum NMSettingsAddConnection2Flags or enum NMSettingsUpdate2Flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean, this is the connection one, so, for me having it a persist or persistent looks better while for the general state it is about copying or not copying the persistent configuration.

State(state): State<NetworkServiceState>,
Path(id): Path<String>,
) -> Result<impl IntoResponse, NetworkError> {
if id == "all" {
Copy link
Contributor

Choose a reason for hiding this comment

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

master: IMHO there is too much logic in this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree

@imobachgs imobachgs merged commit 8e10aff into rc1 Jun 3, 2025
10 of 11 checks passed
@imobachgs imobachgs deleted the network-persistent-connections-handling-rc1 branch June 3, 2025 15:34
@imobachgs imobachgs mentioned this pull request Jun 30, 2025
imobachgs added a commit that referenced this pull request Jun 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants