Repository: opossum-tool/OpossumUI (ts)

Issue:
Unsaved popup not shown when opening a new file
**Describe the bug**
When unsave changes are present and the user clicks on open file, the unsaved popup is not shown.

**To Reproduce**
Steps to reproduce the behavior:

1. Open a file
2. open/create an attribution
3. modify the content, without saving
4. Click on the open file icon (top-left)
5. No unsaved popup is shown

**Expected behavior**
By step 5. the usaved popup is shown, allowing the user not to loose their changes, if so desired.
 
**Desktop (please complete the following information):**
All


Pull request description:
fix: warn user of unsaved changes if file is opened via button in top bar
### Summary of changes

The user is warned about unsaved changes if he clicks the "open file" button in the top bar.
Buttons of the warning popup show the following behaviour:
Cancel -> changes are not saved and open file dialog is not displayed
Undo -> changes are discarded and open file dialog is displayed
Save -> popups that apply are shown (was preferred or prefer globally warning), changes are saved eventually and open file dialog is displayed in the end

### Context and reason for change

Closes #2234.

This is a full solution for the bug described in #2234, but only a partial solution to the problem. There is also the possibility to open a file via the menu. Opened a follow up ticket for this: #2379. (It seems to be a bit more effort, because it is handled entirely in the backend. Channels for frontend communication need to be introduced.)

### How can the changes be tested

Added an e2e test that tests paths leading up to the system dialog for opening a file, but doesn't include it in the test.

Patch:
diff --git a/src/Frontend/Components/NotSavedPopup/NotSavedPopup.tsx b/src/Frontend/Components/NotSavedPopup/NotSavedPopup.tsx
index f3be112d..6206f0e3 100644
--- a/src/Frontend/Components/NotSavedPopup/NotSavedPopup.tsx
+++ b/src/Frontend/Components/NotSavedPopup/NotSavedPopup.tsx
@@ -9,7 +9,7 @@ import {
   checkIfWasPreferredAndShowWarningOrUnlinkAndSave,
   checkIfWasPreferredOrPreferredStatusChangedAndShowWarningOrSave,
   closePopupAndUnsetTargets,
-  navigateToTargetResourceOrAttribution,
+  navigateToTargetResourceOrAttributionOrOpenFileDialog,
 } from '../../state/actions/popup-actions/popup-actions';
 import { useAppDispatch, useAppSelector } from '../../state/hooks';
 import {
@@ -47,7 +47,7 @@ export function NotSavedPopup(): ReactElement {
   }
 
   function handleDiscardClick(): void {
-    dispatch(navigateToTargetResourceOrAttribution());
+    dispatch(navigateToTargetResourceOrAttributionOrOpenFileDialog());
   }
 
   function handleCancelClick(): void {
diff --git a/src/Frontend/Components/TopBar/TopBar.tsx b/src/Frontend/Components/TopBar/TopBar.tsx
index d1eff228..7de21bc9 100644
--- a/src/Frontend/Components/TopBar/TopBar.tsx
+++ b/src/Frontend/Components/TopBar/TopBar.tsx
@@ -9,11 +9,18 @@ import MuiToggleButton from '@mui/material/ToggleButton';
 import MuiToggleButtonGroup from '@mui/material/ToggleButtonGroup';
 import { ReactElement } from 'react';
 
-import { View } from '../../enums/enums';
+import { PopupType, View } from '../../enums/enums';
 import { OpossumColors } from '../../shared-styles';
 import { setViewOrOpenUnsavedPopup } from '../../state/actions/popup-actions/popup-actions';
+import {
+  openPopup,
+  setOpenFileRequest,
+} from '../../state/actions/view-actions/view-actions';
 import { useAppDispatch, useAppSelector } from '../../state/hooks';
-import { getResources } from '../../state/selectors/all-views-resource-selectors';
+import {
+  getResources,
+  wereTemporaryDisplayPackageInfoModified,
+} from '../../state/selectors/all-views-resource-selectors';
 import { getSelectedView } from '../../state/selectors/view-selector';
 import { BackendCommunication } from '../BackendCommunication/BackendCommunication';
 import { CommitInfoDisplay } from '../CommitInfoDisplay/CommitInfoDisplay';
@@ -61,6 +68,9 @@ export function TopBar(): ReactElement {
   const selectedView = useAppSelector(getSelectedView);
   const showTopProgressBar = useAppSelector(getResources) !== null;
   const dispatch = useAppDispatch();
+  const isTemporaryPackageInfoModified = useAppSelector(
+    wereTemporaryDisplayPackageInfoModified,
+  );
 
   function handleClick(
     _: React.MouseEvent<HTMLElement>,
@@ -69,6 +79,15 @@ export function TopBar(): ReactElement {
     dispatch(setViewOrOpenUnsavedPopup(selectedView));
   }
 
+  function handleOpenFileClick(): void {
+    if (isTemporaryPackageInfoModified) {
+      dispatch(setOpenFileRequest(true));
+      dispatch(openPopup(PopupType.NotSavedPopup));
+    } else {
+      void window.electronAPI.openFile();
+    }
+  }
+
   return (
     <MuiBox aria-label={'top bar'} sx={classes.root}>
       <BackendCommunication />
@@ -76,7 +95,7 @@ export function TopBar(): ReactElement {
         tooltipTitle="open file"
         tooltipPlacement="right"
         onClick={(): void => {
-          void window.electronAPI.openFile();
+          handleOpenFileClick();
         }}
         icon={
           <FolderOpenIcon
diff --git a/src/Frontend/state/actions/popup-actions/popup-actions.ts b/src/Frontend/state/actions/popup-actions/popup-actions.ts
index b111f5e2..405257fa 100644
--- a/src/Frontend/state/actions/popup-actions/popup-actions.ts
+++ b/src/Frontend/state/actions/popup-actions/popup-actions.ts
@@ -39,7 +39,11 @@ import {
   getResolvedExternalAttributions,
   getSelectedResourceId,
 } from '../../selectors/audit-view-resource-selectors';
-import { getSelectedView, getTargetView } from '../../selectors/view-selector';
+import {
+  getOpenFileRequest,
+  getSelectedView,
+  getTargetView,
+} from '../../selectors/view-selector';
 import { AppThunkAction, AppThunkDispatch } from '../../types';
 import { setTemporaryDisplayPackageInfo } from '../resource-actions/all-views-simple-actions';
 import {
@@ -73,6 +77,7 @@ import {
   closePopup,
   navigateToView,
   openPopup,
+  setOpenFileRequest,
   setShowNoSignalsLocatedMessage,
   setTargetView,
 } from '../view-actions/view-actions';
@@ -183,7 +188,7 @@ export function unlinkAttributionAndSavePackageInfoAndNavigateToTargetViewIfSavi
         temporaryDisplayPackageInfo,
       ),
     );
-    dispatch(navigateToTargetResourceOrAttribution());
+    dispatch(navigateToTargetResourceOrAttributionOrOpenFileDialog());
   };
 }
 
@@ -228,7 +233,7 @@ export function saveTemporaryDisplayPackageInfoAndNavigateToTargetViewIfSavingIs
         convertDisplayPackageInfoToPackageInfo(temporaryDisplayPackageInfo),
       ),
     );
-    dispatch(navigateToTargetResourceOrAttribution());
+    dispatch(navigateToTargetResourceOrAttributionOrOpenFileDialog());
   };
 }
 
@@ -267,10 +272,19 @@ export function checkIfWasPreferredOrPreferredStatusChangedAndShowWarningOrSave(
   };
 }
 
-export function navigateToTargetResourceOrAttribution(): AppThunkAction {
+export function navigateToTargetResourceOrAttributionOrOpenFileDialog(): AppThunkAction {
   return (dispatch: AppThunkDispatch, getState: () => State): void => {
     const targetView = getTargetView(getState());
     const view = getSelectedView(getState());
+    const openFileRequest = getOpenFileRequest(getState());
+
+    dispatch(closePopup());
+    if (openFileRequest) {
+      void window.electronAPI.openFile();
+      dispatch(setOpenFileRequest(false));
+      return;
+    }
+
     dispatch(setSelectedResourceOrAttributionIdToTargetValue());
     if (targetView) {
       dispatch(navigateToView(targetView));
@@ -282,7 +296,6 @@ export function navigateToTargetResourceOrAttribution(): AppThunkAction {
       ),
     );
 
-    dispatch(closePopup());
     if (view === View.Report) {
       dispatch(closePopup());
     }
@@ -295,6 +308,7 @@ export function closePopupAndUnsetTargets(): AppThunkAction {
     dispatch(setTargetSelectedResourceId(''));
     dispatch(setTargetSelectedAttributionId(''));
     dispatch(closePopup());
+    dispatch(setOpenFileRequest(false));
   };
 }
 
diff --git a/src/Frontend/state/actions/view-actions/types.ts b/src/Frontend/state/actions/view-actions/types.ts
index fb6e97ca..db19c5b1 100644
--- a/src/Frontend/state/actions/view-actions/types.ts
+++ b/src/Frontend/state/actions/view-actions/types.ts
@@ -16,6 +16,8 @@ export const ACTION_SET_SHOW_NO_SIGNALS_LOCATED_MESSAGE =
 
 export const ACTION_SET_QA_MODE = 'ACTION_SET_QA_MODE';
 
+export const ACTION_SET_OPEN_FILE_REQUEST = 'ACTION_SET_OPEN_FILE_REQUEST';
+
 export type ViewAction =
   | SetView
   | SetTargetView
@@ -24,7 +26,8 @@ export type ViewAction =
   | OpenPopupAction
   | UpdateActiveFilters
   | SetShowNoSignalsLocatedMessage
-  | SetQAModeAction;
+  | SetQAModeAction
+  | SetOpenFileRequestAction;
 
 export interface ResetViewStateAction {
   type: typeof ACTION_RESET_VIEW_STATE;
@@ -63,3 +66,8 @@ export interface SetQAModeAction {
   type: typeof ACTION_SET_QA_MODE;
   payload: boolean;
 }
+
+export interface SetOpenFileRequestAction {
+  type: typeof ACTION_SET_OPEN_FILE_REQUEST;
+  payload: boolean;
+}
diff --git a/src/Frontend/state/actions/view-actions/view-actions.ts b/src/Frontend/state/actions/view-actions/view-actions.ts
index 0f1f8044..e78f6b95 100644
--- a/src/Frontend/state/actions/view-actions/view-actions.ts
+++ b/src/Frontend/state/actions/view-actions/view-actions.ts
@@ -18,6 +18,7 @@ import {
   ACTION_CLOSE_POPUP,
   ACTION_OPEN_POPUP,
   ACTION_RESET_VIEW_STATE,
+  ACTION_SET_OPEN_FILE_REQUEST,
   ACTION_SET_QA_MODE,
   ACTION_SET_SHOW_NO_SIGNALS_LOCATED_MESSAGE,
   ACTION_SET_TARGET_VIEW,
@@ -26,6 +27,7 @@ import {
   ClosePopupAction,
   OpenPopupAction,
   ResetViewStateAction,
+  SetOpenFileRequestAction,
   SetQAModeAction,
   SetShowNoSignalsLocatedMessage,
   SetTargetView,
@@ -111,3 +113,9 @@ export function setShowNoSignalsLocatedMessage(
 export function setQAMode(qaMode: boolean): SetQAModeAction {
   return { type: ACTION_SET_QA_MODE, payload: qaMode };
 }
+
+export function setOpenFileRequest(
+  openFileRequest: boolean,
+): SetOpenFileRequestAction {
+  return { type: ACTION_SET_OPEN_FILE_REQUEST, payload: openFileRequest };
+}
diff --git a/src/Frontend/state/reducers/view-reducer.ts b/src/Frontend/state/reducers/view-reducer.ts
index cc2769d2..1283da9f 100644
--- a/src/Frontend/state/reducers/view-reducer.ts
+++ b/src/Frontend/state/reducers/view-reducer.ts
@@ -9,6 +9,7 @@ import {
   ACTION_CLOSE_POPUP,
   ACTION_OPEN_POPUP,
   ACTION_RESET_VIEW_STATE,
+  ACTION_SET_OPEN_FILE_REQUEST,
   ACTION_SET_QA_MODE,
   ACTION_SET_SHOW_NO_SIGNALS_LOCATED_MESSAGE,
   ACTION_SET_TARGET_VIEW,
@@ -25,6 +26,7 @@ export interface ViewState {
   activeFilters: Set<FilterType>;
   showNoSignalsLocatedMessage: boolean;
   qaMode: boolean;
+  openFileRequest: boolean;
 }
 
 export const initialViewState: ViewState = {
@@ -34,6 +36,7 @@ export const initialViewState: ViewState = {
   activeFilters: new Set<FilterType>(),
   showNoSignalsLocatedMessage: false,
   qaMode: false,
+  openFileRequest: false,
 };
 
 export function viewState(
@@ -82,6 +85,11 @@ export function viewState(
         ...state,
         qaMode: action.payload,
       };
+    case ACTION_SET_OPEN_FILE_REQUEST:
+      return {
+        ...state,
+        openFileRequest: action.payload,
+      };
     default:
       return state;
   }
diff --git a/src/Frontend/state/selectors/view-selector.ts b/src/Frontend/state/selectors/view-selector.ts
index 502d999b..633c3713 100644
--- a/src/Frontend/state/selectors/view-selector.ts
+++ b/src/Frontend/state/selectors/view-selector.ts
@@ -45,3 +45,7 @@ export function getPopupAttributionId(state: State): string | null {
 export function getQAMode(state: State): boolean {
   return state.viewState.qaMode;
 }
+
+export function getOpenFileRequest(state: State): boolean {
+  return state.viewState.openFileRequest;
+}


Test patch:
diff --git a/src/Frontend/state/actions/popup-actions/__tests__/popup-actions.test.ts b/src/Frontend/state/actions/popup-actions/__tests__/popup-actions.test.ts
index aae03ee6..9128fa0f 100644
--- a/src/Frontend/state/actions/popup-actions/__tests__/popup-actions.test.ts
+++ b/src/Frontend/state/actions/popup-actions/__tests__/popup-actions.test.ts
@@ -93,7 +93,7 @@ import {
   locateSignalsFromLocatorPopup,
   locateSignalsFromProjectStatisticsPopup,
   navigateToSelectedPathOrOpenUnsavedPopup,
-  navigateToTargetResourceOrAttribution,
+  navigateToTargetResourceOrAttributionOrOpenFileDialog,
   openAttributionWizardPopup,
   removeWasPreferred,
   saveTemporaryDisplayPackageInfoAndNavigateToTargetViewIfSavingIsNotDisabled,
@@ -592,7 +592,9 @@ describe('The actions called from the unsaved popup', () => {
       testStore.dispatch(setTargetView(View.Attribution));
       testStore.dispatch(openPopup(PopupType.NotSavedPopup));
       testStore.dispatch(setTargetSelectedResourceId('newSelectedResource'));
-      testStore.dispatch(navigateToTargetResourceOrAttribution());
+      testStore.dispatch(
+        navigateToTargetResourceOrAttributionOrOpenFileDialog(),
+      );
       return testStore.getState();
     }
 
diff --git a/src/e2e-tests/__tests__/open-file-warnings.test.ts b/src/e2e-tests/__tests__/open-file-warnings.test.ts
new file mode 100644
index 00000000..c884bb05
--- /dev/null
+++ b/src/e2e-tests/__tests__/open-file-warnings.test.ts
@@ -0,0 +1,54 @@
+// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates
+// SPDX-FileCopyrightText: TNG Technology Consulting GmbH <https://www.tngtech.com>
+//
+// SPDX-License-Identifier: Apache-2.0
+import { faker } from '../../shared/Faker';
+import { test } from '../utils';
+
+const [resourceName1, resourceName2] = faker.opossum.resourceNames({
+  count: 2,
+});
+const license1 = faker.opossum.license();
+const license2 = faker.opossum.license();
+const [attributionId, packageInfo] = faker.opossum.manualAttribution();
+
+test.use({
+  data: {
+    inputData: faker.opossum.inputData({
+      resources: faker.opossum.resources({
+        [resourceName1]: 1,
+        [resourceName2]: 1,
+      }),
+      frequentLicenses: [license1, license2],
+    }),
+    outputData: faker.opossum.outputData({
+      manualAttributions: faker.opossum.manualAttributions({
+        [attributionId]: packageInfo,
+      }),
+      resourcesToAttributions: faker.opossum.resourcesToAttributions({
+        [faker.opossum.filePath(resourceName1)]: [attributionId],
+        [faker.opossum.filePath(resourceName2)]: [attributionId],
+      }),
+    }),
+  },
+});
+
+test('warns user of unsaved changes if user attempts to open new file before saving', async ({
+  attributionDetails,
+  notSavedPopup,
+  resourceBrowser,
+  topBar,
+}) => {
+  const comment = faker.lorem.sentences();
+  await resourceBrowser.goto(resourceName1);
+  await attributionDetails.comment().fill(comment);
+
+  await topBar.openFileButton.click();
+  await notSavedPopup.assert.isVisible();
+
+  await notSavedPopup.cancelButton.click();
+  await attributionDetails.assert.commentIs(comment);
+
+  await topBar.openFileButton.click();
+  await notSavedPopup.assert.isVisible();
+});