Remove global issues page and start using popup instead (#886)

## Problem

"We have agreed to use the sidebar for actions related to the installer
itself (e.g., diagnosis tools, display lang, etc). Everything related to
the target system to install should not be placed in the siderbar.

The only thing related to the target system still present in the sidebar
is the issues link. The issues should be globally accessible from
another place (footer, header, ...)."

As described in #872 

This PR fixes #872 

## Solution

Issues page link is removed from the sidebar and all other places where
it was present.

During the progress of this PR, decision was made to drop the showing of
global (all) issues at once in the popup.

Because of this, the Issues popup (previously a page) can no longer be
accessed from the sidebar (or from the icon in the header, which was an
idea at some point).

The Issues Dialog (popup) now shows issues related only to a single
section (software, product, storage, ...) and is opened by clicking on
the warning link in the sections on the overview page (and a few other
places).

For this reason, the prop id for the Section component was introduced,
so the issues could be filtered to a specific type for the popup based
on from where has the user clicked the warning.

## Testing

-  Tested manually
- Adjusted unit test files to some extent, possibly more work is needed
here.


## Screenshots


![294189450-e2b43bbc-65cb-42bb-aa64-092eb8e2b0f5](https://github.com/openSUSE/agama/assets/112288843/0177753e-c96b-42d4-a10e-7543b570104a)

web/package/cockpit-agama.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Thu Jan 04 21:44:32 UTC 2024 - Balsa Asanovic <[email protected]>
+
+- Removing global issues page and using popup dialog instead.
+  It shows issues only from a specific category (software,
+  product, storage, ...) and not all at once. (gh#openSUSE/agama#886).
+
 -------------------------------------------------------------------
 Tue Dec 26 11:12:45 UTC 2023 - David Diaz <[email protected]>
 

web/src/assets/styles/blocks.scss

@@ -442,3 +442,16 @@ ul[data-of="agama/timezones"] {
     text-align: end;
   }
 }
+
+[role="dialog"] {
+  section:not([class^="pf-c"]) {
+    > svg:first-child {
+    block-size: 24px;
+    inline-size: 24px;
+    }
+
+    h2 {
+      font-size: var(--fs-h3);
+    }
+  }
+}

web/src/assets/styles/patternfly-overrides.scss

@@ -236,6 +236,15 @@ table td > .pf-v5-c-empty-state {
   padding-inline: 0;
 }
 
+ul {
+  list-style: initial;
+  margin-inline: var(--spacer-normal);
+
+  li:not(:last-child) {
+    margin-block-end: var(--spacer-small);
+  }
+}
+
 @media screen and (width <= 768px) {
   .pf-m-grid-md.pf-v5-c-table tr:where(.pf-v5-c-table__tr):not(.pf-v5-c-table__expandable-row) {
     padding-inline: 0;

web/src/assets/styles/variables.scss

@@ -13,6 +13,7 @@
   --fs-large: 1rem;
   --fs-h1: 1.5rem;
   --fs-h2: 1.2rem;
+  --fs-h3: 1rem;
 
   --lh-normal: 1.5;
   --lh-medium: 1.6;

web/src/components/core/InstallButton.jsx

@@ -23,30 +23,27 @@ import React, { useState } from "react";
 import { useInstallerClient } from "~/context/installer";
 
 import { Button } from "@patternfly/react-core";
-import { Link } from "react-router-dom";
 
 import { If, Popup } from "~/components/core";
 import { _ } from "~/i18n";
 
 const InstallConfirmationPopup = ({ hasIssues, onAccept, onClose }) => {
   const IssuesWarning = () => {
     // TRANSLATORS: the installer reports some errors,
-    // the text in square brackets [] is a clickable link
-    const [msgStart, msgLink, msgEnd] = _("There are some reported issues. \
-Please, check [the list of issues] \
-before proceeding with the installation.").split(/[[\]]/);
 
     return (
       <p>
         <strong>
-          {msgStart}
-          <Link to="/issues">{msgLink}</Link>
-          {msgEnd}
+          { _("There are some reported issues. \
+Please review them in the previous steps before proceeding with the installation.") }
         </strong>
       </p>
     );
   };
 
+  const Cancel = hasIssues ? Popup.PrimaryAction : Popup.SecondaryAction;
+  const Continue = hasIssues ? Popup.SecondaryAction : Popup.PrimaryAction;
+
   return (
     <Popup
       title={_("Confirm Installation")}
@@ -63,11 +60,14 @@ according to the provided installation settings.") }
         </p>
       </div>
       <Popup.Actions>
-        <Popup.Confirm onClick={onAccept}>
+        <Cancel key="cancel" onClick={onClose} autoFocus={hasIssues}>
+          {/* TRANSLATORS: button label */}
+          {_("Cancel")}
+        </Cancel>
+        <Continue key="confirm" onClick={onAccept} autoFocus={!hasIssues}>
           {/* TRANSLATORS: button label */}
           {_("Continue")}
-        </Popup.Confirm>
-        <Popup.Cancel onClick={onClose} autoFocus />
+        </Continue>
       </Popup.Actions>
     </Popup>
   );

web/src/components/core/InstallButton.test.jsx

@@ -85,23 +85,22 @@ describe("when the button is clicked and there are not errors", () => {
       };
     });
 
-    it("shows a link to go to the issues page", async () => {
+    it("shows a message encouraging the user to review them", async () => {
       const { user } = installerRender(<InstallButton />);
       const button = await screen.findByRole("button", { name: "Install" });
       await user.click(button);
-
-      await screen.findByRole("link", { name: /list of issues$/ });
+      await screen.findByText(/There are some reported issues/);
     });
   });
 
   describe("if there are not issues", () => {
-    it("does not show a link to go to the issues page", async () => {
+    it("doest not show the message encouraging the user to review them", async () => {
       const { user } = installerRender(<InstallButton />);
       const button = await screen.findByRole("button", { name: "Install" });
       await user.click(button);
       await waitFor(() => {
-        const link = screen.queryByRole("link", { name: /list of issues$/ });
-        expect(link).toBeNull();
+        const text = screen.queryByText(/There are some reported issues/);
+        expect(text).toBeNull();
       });
     });
   });

web/src/components/core/IssuesDialog.jsx

@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) [2023] SUSE LLC
+ *
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, contact SUSE LLC.
+ *
+ * To contact SUSE LLC about this file by physical or electronic mail, you may
+ * find current contact information at www.suse.com.
+ */
+
+import React, { useCallback, useEffect, useState } from "react";
+
+import { partition, useCancellablePromise } from "~/utils";
+import { If, Popup } from "~/components/core";
+import { Icon } from "~/components/layout";
+import { useInstallerClient } from "~/context/installer";
+import { _ } from "~/i18n";
+
+/**
+ * Item representing an issue.
+ * @component
+ *
+ * @param {object} props
+ * @param {import ("~/client/mixins").Issue} props.issue
+ */
+const IssueItem = ({ issue }) => {
+  const hasDetails = issue.details.length > 0;
+
+  return (
+    <li>
+      {issue.description}
+      <If
+        condition={hasDetails}
+        then={<pre>{issue.details}</pre>}
+      />
+    </li>
+  );
+};
+
+/**
+ * Generates issue items sorted by severity.
+ * @component
+ *
+ * @param {object} props
+ * @param {import ("~/client/mixins").Issue[]} props.issues
+ */
+const IssueItems = ({ issues = [] }) => {
+  const sortedIssues = partition(issues, i => i.severity === "error").flat();
+
+  const items = sortedIssues.map((issue, index) => {
+    return <IssueItem key={`issue-${index}`} issue={issue} />;
+  });
+
+  return <ul>{items}</ul>;
+};
+
+/**
+ * Popup to show more issues details from the installation overview page.
+ *
+ * It initially shows a loading state,
+ * then fetches and displays a list of issues of the selected category, either 'product' or 'storage' or 'software'.
+ *
+ * It uses a Popup component to display the issues, and an If component to toggle between
+ * a loading state and the content state.
+ *
+ * @component
+ *
+ * @param {object} props
+ * @param {boolean} [props.isOpen] - A boolean value used to determine wether to show the popup or not.
+ * @param {function} props.onClose - A function to call when the close action is triggered.
+ * @param {string} props.sectionId - A string which indicates what type of issues are going to be shown in the popup.
+ * @param {string} props.title - Title of the popup.
+ */
+export default function IssuesDialog({ isOpen = false, onClose, sectionId, title }) {
+  const [isLoading, setIsLoading] = useState(true);
+  const [issues, setIssues] = useState([]);
+  const client = useInstallerClient();
+  const { cancellablePromise } = useCancellablePromise();
+
+  const load = useCallback(async () => {
+    setIsLoading(true);
+    const issues = await cancellablePromise(client.issues());
+    setIsLoading(false);
+    return issues;
+  }, [client, cancellablePromise, setIsLoading]);
+
+  const update = useCallback((issues) => {
+    setIssues(current => ([...current, ...(issues[sectionId] || [])]));
+  }, [setIssues, sectionId]);
+
+  useEffect(() => {
+    load().then(update);
+    return client.onIssuesChange(update);
+  }, [client, load, update]);
+
+  return (
+    <Popup
+      isOpen={isOpen}
+      title={title}
+      data-content="issues-summary"
+    >
+      <If
+        condition={isLoading}
+        then={<Icon name="loading" className="icon-xxxl" />}
+        else={<IssueItems issues={issues} />}
+      />
+      <Popup.Actions>
+        <Popup.Confirm onClick={onClose} autoFocus>{_("Close")}</Popup.Confirm>
+      </Popup.Actions>
+    </Popup>
+  );
+}

web/src/components/core/IssuesDialog.test.jsx

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) [2023] SUSE LLC
+ *
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, contact SUSE LLC.
+ *
+ * To contact SUSE LLC about this file by physical or electronic mail, you may
+ * find current contact information at www.suse.com.
+ */
+
+import React from "react";
+import { screen } from "@testing-library/react";
+import { installerRender } from "~/test-utils";
+import { createClient } from "~/client";
+import { IssuesDialog } from "~/components/core";
+
+jest.mock("~/client");
+jest.mock("@patternfly/react-core", () => {
+  return {
+    ...jest.requireActual("@patternfly/react-core"),
+    Skeleton: () => <div>PFSkeleton</div>
+  };
+});
+jest.mock("~/components/core/Sidebar", () => () => <div>Agama sidebar</div>);
+
+const issues = {
+  product: [],
+  storage: [
+    { description: "storage issue 1", details: "Details 1", source: "system", severity: "warn" },
+    { description: "storage issue 2", details: "Details 2", source: "config", severity: "error" }
+  ],
+  software: [
+    { description: "software issue 1", details: "Details 1", source: "system", severity: "warn" }
+  ]
+};
+
+let mockIssues;
+
+beforeEach(() => {
+  mockIssues = { ...issues };
+
+  createClient.mockImplementation(() => {
+    return {
+      issues: jest.fn().mockResolvedValue(mockIssues),
+      onIssuesChange: jest.fn()
+    };
+  });
+});
+
+it("loads the issues", async () => {
+  installerRender(<IssuesDialog isOpen sectionId="storage" title="Storage issues" />);
+
+  await screen.findByText(/storage issue 1/);
+});
+
+it('calls onClose callback when close button is clicked', async () => {
+  const mockOnClose = jest.fn();
+  const { user } = installerRender(<IssuesDialog isOpen onClose={mockOnClose} sectionId="software" title="Software issues" />);
+
+  await user.click(screen.getByText("Close"));
+  expect(mockOnClose).toHaveBeenCalled();
+});