Skip to content

Commit

Permalink
webui: When re-scanning don't reset disk selection
Browse files Browse the repository at this point in the history
Instead keep selected disks if they are still usable.

Relevant to https://bugzilla.redhat.com/show_bug.cgi?id=2231357#c3
  • Loading branch information
KKoukiou authored and rvykydal committed Sep 1, 2023
1 parent 42e7b19 commit 16b370f
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 14 deletions.
13 changes: 6 additions & 7 deletions ui/webui/src/components/storage/InstallationMethod.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,17 @@ const N_ = cockpit.noop;
/**
* Select default disks for the partitioning.
*
* If there are some disks already selected, do nothing.
* If there are some usable disks already selected, show these.
* In the automatic installation, select all disks. In
* the interactive installation, select a disk if there
* is only one available.
* @return: the list of selected disks
*/
const selectDefaultDisks = ({ ignoredDisks, selectedDisks, usableDisks }) => {
if (selectedDisks.length) {
// Do nothing if there are some disks selected
if (selectedDisks.length && selectedDisks.some(disk => usableDisks.includes(disk))) {
// Filter the selection by checking the usable disks if there are some disks selected
console.log("Selecting disks selected in backend:", selectedDisks.join(","));
return selectedDisks;
return selectedDisks.filter(disk => usableDisks.includes(disk));
} else {
const availableDisks = usableDisks.filter(disk => !ignoredDisks.includes(disk));
console.log("Selecting one or less disks by default:", availableDisks.join(","));
Expand Down Expand Up @@ -307,7 +307,6 @@ const InstallationDestination = ({ deviceData, diskSelection, dispatch, idPrefix
if (refUsableDisks.current !== undefined) {
return;
}
refUsableDisks.current = diskSelection.usableDisks;

const defaultDisks = selectDefaultDisks({
ignoredDisks: diskSelection.ignoredDisks,
Expand Down Expand Up @@ -343,7 +342,7 @@ const InstallationDestination = ({ deviceData, diskSelection, dispatch, idPrefix
icon={<SyncAltIcon />}
onClick={() => {
setIsRescanningDisks(true);
setSelectedDisks({ drives: [] });
refUsableDisks.current = undefined;
scanDevicesWithTask()
.then(res => {
return runStorageTask({
Expand Down Expand Up @@ -374,7 +373,7 @@ const InstallationDestination = ({ deviceData, diskSelection, dispatch, idPrefix
/>
);

const equalDisks = refUsableDisks.current && containEqualDisks(refUsableDisks.current, diskSelection.usableDisks);
const equalDisks = !refUsableDisks.current || containEqualDisks(refUsableDisks.current, diskSelection.usableDisks);
const headingLevel = isBootIso ? "h2" : "h3";

return (
Expand Down
8 changes: 3 additions & 5 deletions ui/webui/test/check-storage
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class TestStorage(anacondalib.VirtInstallMachineCase, StorageHelpers):
s.rescan_disks()

# Check that the disk selection persists when moving next and back
s.select_disk("vda", True)
s.check_disk_selected("vda", True)
i.next()
i.back()
s.check_disk_selected("vda", True)
Expand Down Expand Up @@ -292,14 +292,12 @@ class TestStorageExtraDisks(anacondalib.VirtInstallMachineCase, StorageHelpers):

s.wait_no_disks_detected_not_present()

s.check_disk_selected("vda", False)
s.check_disk_selected("vda", True)
s.check_disk_selected(dev, False)

s.rescan_disks()
s.wait_no_disks_detected()

# Check that disk selection is kept on Next and Back
disks = ["vda"]
for disk in disks:
s.select_disk(disk)
i.next()
Expand All @@ -324,7 +322,7 @@ class TestUtils():
b.wait(lambda : self.disks_loaded(s, disks))

for disk in disks:
current_selection = s.get_disk_selected(disk)
current_selection = s.get_disk_selected(disk[0])
if current_selection != disk[1]:
s.select_disk(disk[0], disk[1], len(disks) == 1)

Expand Down
5 changes: 3 additions & 2 deletions ui/webui/test/helpers/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ def check_disk_selected(self, disk, selected=True):
self.browser.wait_not_present(f"#{id_prefix}-selector-form li.pf-v5-c-chip-group__list-item:contains({disk})")

def get_disk_selected(self, disk):
return self.browser.is_present(f"#{id_prefix}-selector-form li.pf-v5-c-chip-group__list-item:contains({disk})")
return (
self.browser.is_present(f"#{id_prefix}-selector-form li.pf-v5-c-chip-group__list-item:contains({disk})") or (disk in self.browser.text(f"#{id_prefix}-target-disk"))
);

@log_step()
def wait_no_disks(self):
Expand Down Expand Up @@ -281,7 +283,6 @@ def add_basic_partitioning(self, target="vda", size="1GiB"):
# Add a partition for "Use free space" scenario to be present
self.machine.execute(f"sgdisk --new=0:0:+{size} /dev/{target}")
self.rescan_disks()
self.select_disk(target, True, True)

# partitions_params expected structure: [("size", "file system" {, "other mkfs.fs flags"})]
def partition_disk(self, disk, partitions_params):
Expand Down

0 comments on commit 16b370f

Please sign in to comment.