Skip to content

feat(web): allow editing the hostname#2147

Merged
dgdavid merged 22 commits intomasterfrom
hostname-ui
Mar 17, 2025
Merged

feat(web): allow editing the hostname#2147
dgdavid merged 22 commits intomasterfrom
hostname-ui

Conversation

@dgdavid
Copy link
Contributor

@dgdavid dgdavid commented Mar 11, 2025

Problem

Currently, users cannot edit the hostname through the web user interface.

Solution

Add a page that allows users to change the hostname directly from the web UI. Additionally, warn users at the registration form about hostname implications with a link to the section for changing it.

Tests

  • Tested manually
  • Added unit tests

Notes

  • This PR includes some cherry-picks from lvm-form branch to make forms looks a bit better. The motivation to do so is: a) LVM PR is taking longer than expected; b) this PR is no longer against a temporary branch but against master
  • Since updating the registration form was initially needed, this PR also takes the opportunity to improve the form. That's why Some commits are purely related to the registration form
  • A new "system" namespace has been introduced and the code place there. That's because the hostname entry will be moved to a "System" page when time permits. See comment feat(web): allow editing the hostname #2147 (comment) to know more

Screenshots

Not using static hostname Using static hostname
localhost_8080_ (8) localhost_8080_ (10)

Related to https://trello.com/c/bDToEZ43 (protected link)

dgdavid added 5 commits March 11, 2025 13:40
Removed unnecessary spacing and section regions to streamline the form.
These elements were redundant in simple pages where there is no need to
distinguish between different sections or content.
This change is a step toward having forms display only "required" inputs.
While not yet documented in developer guidelines, it has been discussed
and agreed that forms should show only required inputs or inputs that will
always have a value. Optional fields should be explicitly requested by
the user, effectively making them required once displayed.

In the registration form, this applies to the email input: if the user
does not select "Provide email address," the email input field is neither
rendered nor validated. However, if selected, it is displayed and
validated accordingly.
The idea is to make it easier for users to set a hostname when
registering a product, as the product will be registered with the
specified hostname. Instead of adding warnings elsewhere, it’s more
helpful to let users set the hostname directly from the registration
form too, which will be prefilled with the current hostname value.

NOTE: This commit only updates the interface. The logic for setting the
hostname is not yet implemented. Tests are intentionally broken to
prevent overlooking it.
This commit adds only the basic interface components. Logic for setting
the hostname and its options still pending to be implemented.
Alert users that changing the hostname of an already registered product
will not update the hostname used during registration.
@dgdavid dgdavid requested a review from teclator March 11, 2025 17:57
@dgdavid
Copy link
Contributor Author

dgdavid commented Mar 12, 2025

After separate discussions with @teclator and @imobachgs, the following changes are planned:

  • The Enable DHCP option will be removed for now (see Hostname handling #2142 and Hostname handling #2142 (comment))
  • Removing the above option makes having a dedicated page for setting the hostname a bit overkill.
  • While the hostname field at registration form could be helpful for users, it might give the false impression it is something that can be updated later. To avoid confusion, it will be replaced with useful information about the hostname being part of the registration and a link to the page where it can be managed.
  • The hostname could be actually tied to a "System" page, which doesn’t exist yet. We have agree to plan combine the hostname and localization settings (language, keyboard, timezone) into a new "System" page. Additionally, we’re considering merging such a page and the current Overview to make the later really useful. However, this will need careful planning for a future PBI.

Thus, below changes will be performed for considering this PR ready

  • Add a "Hostname" menu entry that links to a simple hostname page. Such an entry will eventually evolve into the future "System" page.
  • Replace the hostname input in the registration form with helpful information and a link to previous page.

dgdavid added 5 commits March 12, 2025 22:53
On the registration page, the hostname input is replaced with an alert
to provide helpful information to users about hostname implications
during registration, without giving the impression that it can be
updated later.
The option to enable DHCP hostname is removed since it will not be
implemented by now.

This commit also includes additional unit tests, which are intentionally
broken to serve as a reminder to update them once the actual hooks are
implemented.
@dgdavid dgdavid changed the title feat(web): allow editing the hostname and its options feat(web): allow editing the hostname Mar 12, 2025
@dgdavid dgdavid marked this pull request as ready for review March 14, 2025 11:56
@dgdavid

This comment was marked as outdated.

@dgdavid

This comment was marked as outdated.

@dgdavid

This comment was marked as outdated.

@dgdavid

This comment was marked as outdated.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 13856030764

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on hostname-ui at 72.338%

Totals Coverage Status
Change from base Build 13837957018: 72.3%
Covered Lines: 20021
Relevant Lines: 27677

💛 - Coveralls

Base automatically changed from after-release-beta2 to master March 14, 2025 15:25
@teclator

This comment was marked as outdated.

@dgdavid

This comment was marked as resolved.

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.

The code looks good. However, the wording might be improved.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please, FIXIT 😉

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 need help for fix it. I do not have an idea of a good text for helping users there. I'm a bit lost with transient and static stuff. So, I hope @teclator (or anyone else) can help me with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After a chat with @teclator I realize that actually it was desired to allow the user unset the static hostname too. Thus, the form has changed a bit and this explanation now belongs to a checkbox in which a final sentence has been added. Unless somebody suggest something better.

dgdavid and others added 2 commits March 14, 2025 21:36
Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>
For using real useHostname hook and display the proper hostname value to
users.
@dgdavid dgdavid requested a review from imobachgs March 14, 2025 22:06
Introduces a custom alert at the top of the hostname page to provide
users with both the current hostname value and the mode (permanent or
temporary). The goal is to ensure users not only know the current
hostname but also understand the meaning and implications of each mode,
helping them make an informed choice.

While there’s room for improvement, further refinements will be
addressed in future updates when moving toward a system page. This will
help unblock the current work and enable users to set the hostname via
the web interface as soon as possible.
@dgdavid
Copy link
Contributor Author

dgdavid commented Mar 17, 2025

@teclator, re-ready for a review.

@imobachgs ^^^ (just in case this still opens when you are back. Hope it will not).

dgdavid added 5 commits March 17, 2025 13:01
Override PatternFly styles to ensure that a FormGroup aligns to the
start of its flex container instead of stretching to fill all available
space by default.

https://developer.mozilla.org/en-US/docs/Web/CSS/justify-content#normal
Among others (out of the scope of the feature this commit belongs),
storage/EncryptionSettingsPage form no longer requires individual tweaks
to prevent fields from stretching to match the widest one.
To make them a bit more bigger.
Suggested by @teclator in an "pair-review" session.
Copy link
Contributor

@teclator teclator left a comment

Choose a reason for hiding this comment

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

LGTM

@dgdavid dgdavid merged commit c7377a8 into master Mar 17, 2025
7 checks passed
@dgdavid dgdavid deleted the hostname-ui branch March 17, 2025 14:00
@bittin
Copy link
Contributor

bittin commented Mar 18, 2025

Swedish translation for this updated

dgdavid added a commit that referenced this pull request Mar 18, 2025
The registration form was updated in PR #2147 to make the email field
optional, neither rendering nor validating it unless the user opts to
provide an email address.

However, the process was failing silently due to the backend complaining
about a malformed JSON.

> Failed to deserialize the JSON body into the target type: missing
> field `email` at line 1 column 16

This commit fixes the issue by sending the email as an empty string when
the user chooses not to provide it.
dgdavid added a commit that referenced this pull request Mar 18, 2025
The registration form was updated in PR #2147 to make the email field
optional, neither rendering nor validating it unless the user opts to
provide an email address.

However, the process was failing silently due to the backend complaining
about a malformed JSON.

> Failed to deserialize the JSON body into the target type: missing
> field `email` at line 1 column 16

This PR fixes the issue by sending the email as an empty string when the
user chooses not to provide it.
teclator added a commit that referenced this pull request Mar 25, 2025
Follow up to #2147 (and #2183)

This PR includes few improvements for the hostname page/form. Go commit
by commit to know more.

Please, add as many changes you want and merge it when it is in a shape
you think is acceptable.

To see the final result run it locally and test by yourself.
@imobachgs imobachgs mentioned this pull request Mar 27, 2025
imobachgs added a commit that referenced this pull request Mar 27, 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

Comments