Merge pull request #369 from adamkankovsky/INSTALLER-3918

Re-add alert confirmation before Cockpit Storage
rhinstaller · Aug 3, 2024 · ecf0397 · ecf0397
2 parents 59e4ac5 + 049487f
commit ecf0397
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 37 deletions.
diff --git a/src/components/storage/CockpitStorageIntegration.jsx b/src/components/storage/CockpitStorageIntegration.jsx
@@ -110,15 +110,50 @@ export const useMaybeBackdrop = () => {
     return hasDialogOpen ? "cockpit-has-modal" : "";
 };
 
+const CockpitStorageConfirmationModal = ({ handleCancelOpenModal, handleConfirmOpenModal, showConfirmation }) => {
+    return (
+        <Modal
+          isOpen={showConfirmation}
+          onClose={handleCancelOpenModal}
+          title={_("Modify storage")}
+          titleIconVariant="warning"
+          variant="small"
+          actions={[
+              <Button
+                id={idPrefix + "-enter-storage-confirm"}
+                key="confirm"
+                variant="warning"
+                onClick={handleConfirmOpenModal}>
+                  {_("Launch storage editor")}
+              </Button>,
+              <Button
+                id={idPrefix + "-enter-storage-cancel"}
+                key="cancel"
+                variant="link"
+                onClick={handleCancelOpenModal}>
+                  {_("Cancel")}
+              </Button>
+          ]}
+        >
+            <TextContent>
+                <Text>
+                    {_("The advanced storage editor lets you resize, delete, and create partitions. It can set up LVM and much more.")}
+                </Text>
+                <Text component="strong">{_("Any changes you make will immediately affect your storage.")}</Text>
+            </TextContent>
+        </Modal>
+    );
+};
+
 export const CockpitStorageIntegration = ({
     dispatch,
     isFormDisabled,
     onCritFail,
     scenarioAvailability,
     setShowStorage,
-    showStorage,
 }) => {
     const [showDialog, setShowDialog] = useState(false);
+    const [isConfirmed, setIsConfirmed] = useState(false);
     const backdropClass = useMaybeBackdrop();
 
     useEffect(() => {
@@ -130,42 +165,59 @@ export const CockpitStorageIntegration = ({
         }
     }, [onCritFail]);
 
+    const handleConfirmOpenModal = () => {
+        setIsConfirmed(true);
+        setShowStorage(true);
+    };
+
+    const handleCancelOpenModal = () => {
+        setShowStorage(false);
+        setIsConfirmed(false);
+    };
+
     return (
-        <Modal
-          aria-label={_("Configure storage")}
-          className={backdropClass + " " + idPrefix + "-modal-page-section"}
-          footer={<ReturnToInstallationButton onAction={() => setShowDialog(true)} />}
-          hasNoBodyWrapper
-          isOpen={showStorage}
-          onClose={() => setShowDialog(true)}
-          showClose={false}
-          variant={ModalVariant.large}>
-            <Alert
-              isInline
-              title={_("Changes made here will immediately affect the system. There is no 'undo'.")}
-              variant="warning"
+        <>
+            <CockpitStorageConfirmationModal
+              handleCancelOpenModal={handleCancelOpenModal}
+              handleConfirmOpenModal={handleConfirmOpenModal}
+              showConfirmation={!isConfirmed}
             />
-            <Divider />
-            <div className={idPrefix + "-page-section-cockpit-storage"}>
-                <PageSection>
-                    <iframe
-                      src="/cockpit/@localhost/storage/index.html"
-                      name="cockpit-storage"
-                      id="cockpit-storage-frame"
-                      className={idPrefix + "-iframe-cockpit-storage"} />
-                </PageSection>
-                <ModifyStorageSideBar />
-            </div>
-            {showDialog &&
-            <CheckStorageDialog
-              dispatch={dispatch}
-              isFormDisabled={isFormDisabled}
-              onCritFail={onCritFail}
-              scenarioAvailability={scenarioAvailability}
-              setShowDialog={setShowDialog}
-              setShowStorage={setShowStorage}
-            />}
-        </Modal>
+            <Modal
+              aria-label={_("Configure storage")}
+              className={backdropClass + " " + idPrefix + "-modal-page-section"}
+              footer={<ReturnToInstallationButton onAction={() => setShowDialog(true)} />}
+              hasNoBodyWrapper
+              isOpen={isConfirmed}
+              onClose={() => setShowDialog(true)}
+              showClose={false}
+              variant={ModalVariant.large}>
+                <Alert
+                  isInline
+                  title={_("Changes made here will immediately affect the system. There is no 'undo'.")}
+                  variant="warning"
+                />
+                <Divider />
+                <div className={idPrefix + "-page-section-cockpit-storage"}>
+                    <PageSection>
+                        <iframe
+                          src="/cockpit/@localhost/storage/index.html"
+                          name="cockpit-storage"
+                          id="cockpit-storage-frame"
+                          className={idPrefix + "-iframe-cockpit-storage"} />
+                    </PageSection>
+                    <ModifyStorageSideBar />
+                </div>
+                {showDialog &&
+                    <CheckStorageDialog
+                      dispatch={dispatch}
+                      isFormDisabled={isFormDisabled}
+                      onCritFail={onCritFail}
+                      scenarioAvailability={scenarioAvailability}
+                      setShowDialog={setShowDialog}
+                      setShowStorage={setShowStorage}
+                    />}
+            </Modal>
+        </>
     );
 };
 

diff --git a/src/components/storage/InstallationMethod.jsx b/src/components/storage/InstallationMethod.jsx
@@ -64,13 +64,13 @@ const InstallationMethod = ({
           id={idPrefix + "-selector-form"}
           onSubmit={e => { e.preventDefault(); return false }}
         >
+            {showStorage &&
             <CockpitStorageIntegration
               dispatch={dispatch}
               isFormDisabled={isFormDisabled}
               onCritFail={onCritFail}
               setShowStorage={setShowStorage}
-              showStorage={showStorage}
-            />
+            />}
             <InstallationDestination
               isEfi={isEfi}
               dispatch={dispatch}

diff --git a/test/check-storage-basic b/test/check-storage-basic
@@ -187,6 +187,7 @@ class TestStorage(VirtInstallMachineCase, StorageHelpers):
         i.open()
         i.reach(i.steps.INSTALLATION_METHOD)
         s.modify_storage()
+        s.confirm_entering_cockpit_storage()
         b.wait_visible(".cockpit-storage-integration-sidebar")
         b._wait_present("iframe[name='cockpit-storage']")
         b.switch_to_frame("cockpit-storage")

diff --git a/test/check-storage-cockpit b/test/check-storage-cockpit
@@ -39,6 +39,7 @@ class TestStorageCockpitIntegration(VirtInstallMachineCase, StorageCase):
         s.wait_scenario_visible("use-configured-storage", False)
         s.check_single_disk_destination("vda")
         s.modify_storage()
+        s.confirm_entering_cockpit_storage()
         b.wait_visible(".cockpit-storage-integration-sidebar")
 
         frame = "iframe[name='cockpit-storage']"
@@ -101,6 +102,7 @@ class TestStorageCockpitIntegration(VirtInstallMachineCase, StorageCase):
         s.wait_scenario_visible("use-configured-storage", False)
         s.check_single_disk_destination("vda")
         s.modify_storage()
+        s.confirm_entering_cockpit_storage()
         b.wait_visible(".cockpit-storage-integration-sidebar")
 
         frame = "iframe[name='cockpit-storage']"
@@ -188,6 +190,7 @@ class TestStorageCockpitIntegration(VirtInstallMachineCase, StorageCase):
         s.wait_scenario_visible("use-configured-storage", False)
         s.check_single_disk_destination("vda")
         s.modify_storage()
+        s.confirm_entering_cockpit_storage()
         b.wait_visible(".cockpit-storage-integration-sidebar")
 
         frame = "iframe[name='cockpit-storage']"
@@ -261,6 +264,7 @@ class TestStorageCockpitIntegration(VirtInstallMachineCase, StorageCase):
         s.wait_scenario_visible("use-configured-storage", False)
         s.check_single_disk_destination("vda")
         s.modify_storage()
+        s.confirm_entering_cockpit_storage()
         b.wait_visible(".cockpit-storage-integration-sidebar")
 
         frame = "iframe[name='cockpit-storage']"

diff --git a/test/check-storage-mountpoints b/test/check-storage-mountpoints
@@ -318,6 +318,7 @@ class TestStorageMountPoints(VirtInstallMachineCase, StorageCase):
         s.check_single_disk_destination("vda")
 
         s.modify_storage()
+        s.confirm_entering_cockpit_storage()
         b.wait_visible(".cockpit-storage-integration-sidebar")
         s.check_constraint("/", True)
         s.check_constraint("biosboot", True)

diff --git a/test/helpers/storage.py b/test/helpers/storage.py
@@ -106,6 +106,9 @@ def check_constraint(self, constraint, required=True):
         else:
             self.browser.wait_visible(f"ul.cockpit-storage-integration-requirements-hint-list:nth-of-type(2) li:contains('{constraint}')")
 
+    def confirm_entering_cockpit_storage(self):
+        self.browser.click("#cockpit-storage-integration-enter-storage-confirm")
+
     def return_to_installation(self):
         self.browser.click("#cockpit-storage-integration-return-to-installation-button")