Skip to content

Display all affected devices at the table of results#1936

Merged
ancorgs merged 2 commits intoagama-project:storage-config-uifrom
ancorgs:storage_more_devices
Jan 23, 2025
Merged

Display all affected devices at the table of results#1936
ancorgs merged 2 commits intoagama-project:storage-config-uifrom
ancorgs:storage_more_devices

Conversation

@ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Jan 22, 2025

Problem

The result box at the Storage section displays all the devices affected by the installation proposal. But for doing that, it relies on the actions to be done on the disks. That means that a disk is not displayed there if is not going to be modified. In principle, that's fine but...

There is a corner case if a disk is chosen only for booting and no action is needed for that (eg. legacy boot with a disk that already includes the bios_boot partition). It's certainly not going to be affected, but the disk is displayed at the configuration section and omitted at the result. That's confusing.

Solution

Display at the result all disks that are mentioned at the configuration, even if no actions are needed to prepare the disks.

Testing

  • Tested manually.
  • Extended unit tests to cover the new cases.
  • Bonus: resurrected tests for ProposalResultSection (no longer skipped).

@ancorgs ancorgs force-pushed the storage_more_devices branch from 9f0b563 to 441ad2b Compare January 22, 2025 17:18
Copy link
Contributor

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

So far looks good, but as you commented in the PR description tests need to be adapted and extended. Thanks

* @method
*
* @note The used devices are extracted from the actions.
* @note The used devices are extracted from the actions, but the optional argument
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: DeviceManager is one of a few files that still pending to be migrated to TypeScript. Just saying 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Look! It's a three-headed monkey!

@ancorgs ancorgs force-pushed the storage_more_devices branch 3 times, most recently from 2370b90 to a7c4b75 Compare January 23, 2025 11:34
@ancorgs ancorgs force-pushed the storage_more_devices branch from a7c4b75 to 4c90f6d Compare January 23, 2025 11:38
@ancorgs ancorgs force-pushed the storage_more_devices branch from 4c90f6d to 682cb10 Compare January 23, 2025 11:55
@ancorgs ancorgs changed the title Storage more devices Display all affected devices at the table of results Jan 23, 2025
@ancorgs ancorgs merged commit 31635d4 into agama-project:storage-config-ui Jan 23, 2025
1 check passed
@imobachgs imobachgs mentioned this pull request Feb 26, 2025
imobachgs added a commit that referenced this pull request Feb 26, 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.

2 participants