From b355296753914ce8799748dc63f2513c659a2af1 Mon Sep 17 00:00:00 2001
From: Max Cao <macao@redhat.com>
Date: Tue, 6 Sep 2022 06:24:46 -0700
Subject: [PATCH] fix(targets): disable delete button on non-custom targets
 (#502)

* disable delete button on non-custom targets

* fixed test

* rename symbol

* cleanup
---
 src/app/TargetSelect/TargetSelect.tsx         |  5 +-
 .../__snapshots__/CreateRule.test.tsx.snap    |  7 +--
 src/test/Targets/TargetSelect.test.tsx        | 53 ++++++++++++++-----
 3 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/src/app/TargetSelect/TargetSelect.tsx b/src/app/TargetSelect/TargetSelect.tsx
index 9c04e0016..3f15aa9c0 100644
--- a/src/app/TargetSelect/TargetSelect.tsx
+++ b/src/app/TargetSelect/TargetSelect.tsx
@@ -49,6 +49,7 @@ import { CreateTargetModal } from './CreateTargetModal';
 import _ from 'lodash';
 import { NotificationCategory } from '@app/Shared/Services/NotificationChannel.service';
 
+export const CUSTOM_TARGETS_REALM = "Custom Targets";
 export interface TargetSelectProps {
 }
 
@@ -62,7 +63,7 @@ export const TargetSelect: React.FunctionComponent<TargetSelectProps> = (props)
   const [isLoading, setLoading] = React.useState(false);
   const [isModalOpen, setModalOpen] = React.useState(false);
   const addSubscription = useSubscriptions();
-
+  
   const setCachedTargetSelection = React.useCallback((target) => {
     localStorage.setItem(TARGET_KEY, JSON.stringify(target));
   }, [localStorage]);
@@ -226,7 +227,7 @@ export const TargetSelect: React.FunctionComponent<TargetSelectProps> = (props)
             />
             <Button
               aria-label="Delete target"
-              isDisabled={isLoading || (selected == NO_TARGET)}
+              isDisabled={isLoading || (selected == NO_TARGET) || (selected.annotations?.cryostat['REALM'] !== CUSTOM_TARGETS_REALM)}
               onClick={deleteTarget}
               variant="control"
               icon={<TrashIcon />}
diff --git a/src/test/Rules/__snapshots__/CreateRule.test.tsx.snap b/src/test/Rules/__snapshots__/CreateRule.test.tsx.snap
index 1d09279fb..73e30a196 100644
--- a/src/test/Rules/__snapshots__/CreateRule.test.tsx.snap
+++ b/src/test/Rules/__snapshots__/CreateRule.test.tsx.snap
@@ -905,15 +905,16 @@ exports[`<CreateRule/> renders correctly 1`] = `
                           </span>
                         </button>
                         <button
-                          aria-disabled={false}
+                          aria-disabled={true}
                           aria-label="Delete target"
-                          className="pf-c-button pf-m-control"
+                          className="pf-c-button pf-m-control pf-m-disabled"
                           data-ouia-component-id="OUIA-Generated-Button-control-2"
                           data-ouia-component-type="PF4/Button"
                           data-ouia-safe={true}
-                          disabled={false}
+                          disabled={true}
                           onClick={[Function]}
                           role={null}
+                          tabIndex={null}
                           type="button"
                         >
                           <span
diff --git a/src/test/Targets/TargetSelect.test.tsx b/src/test/Targets/TargetSelect.test.tsx
index 4e258064e..934ddb7b8 100644
--- a/src/test/Targets/TargetSelect.test.tsx
+++ b/src/test/Targets/TargetSelect.test.tsx
@@ -37,32 +37,37 @@
  */
 import * as React from 'react';
 import renderer from 'react-test-renderer'
-import { render, screen, within } from '@testing-library/react';
+import { cleanup, render, screen, waitFor } from '@testing-library/react';
 import '@testing-library/jest-dom';
 
 import { of } from 'rxjs';
-import { createMemoryHistory } from 'history';
 
-import { TargetSelect } from '@app/TargetSelect/TargetSelect';
+import { CUSTOM_TARGETS_REALM, TargetSelect } from '@app/TargetSelect/TargetSelect';
 import { defaultServices, ServiceContext } from '@app/Shared/Services/Services';
 import userEvent from '@testing-library/user-event';
-import { NotificationMessage } from '@app/Shared/Services/NotificationChannel.service';
+import { Target } from '@app/Shared/Services/Target.service';
 
 const mockFooConnectUrl = 'service:jmx:rmi://someFooUrl';
 const mockBarConnectUrl = 'service:jmx:rmi://someBarUrl';
 const mockBazConnectUrl = 'service:jmx:rmi://someBazUrl';
-const mockFooTarget = { connectUrl: mockFooConnectUrl, alias: 'fooTarget' };
-const mockBarTarget = { connectUrl: mockBarConnectUrl, alias: 'barTarget' }
-const mockBazTarget = { connectUrl: mockBazConnectUrl, alias: 'bazTarget' }
 
-const mockHistoryPush = jest.fn();
+// Test fails if new Map([['REALM', 'Custom Targets']]) is used, most likely since 'cryostat' Map is not being utilized
+const cryostatAnnotation = new Map();
+cryostatAnnotation['REALM'] = CUSTOM_TARGETS_REALM;
+const mockFooTarget: Target = { 
+    connectUrl: mockFooConnectUrl, 
+    alias: 'fooTarget', 
+    annotations: { 
+        cryostat: cryostatAnnotation, 
+        platform : new Map() 
+    }
+};
+const mockBarTarget: Target = { ...mockFooTarget, connectUrl: mockBarConnectUrl, alias: 'barTarget' }
+const mockBazTarget: Target = { connectUrl: mockBazConnectUrl, alias: 'bazTarget' }
 
 jest.mock('react-router-dom', () => ({
     ...jest.requireActual('react-router-dom'),
     useRouteMatch: () => ({ url: '/baseUrl' }),
-    useHistory: () => ({
-      push: mockHistoryPush,
-    }),
   }));
 
 jest.spyOn(defaultServices.target, 'target')
@@ -70,6 +75,9 @@ jest.spyOn(defaultServices.target, 'target')
     .mockReturnValueOnce(of())
     .mockReturnValueOnce(of())
     .mockReturnValueOnce(of(mockFooTarget))
+    .mockReturnValueOnce(of(mockFooTarget))
+    .mockReturnValueOnce(of(mockFooTarget))
+    .mockReturnValueOnce(of(mockBazTarget)) // does nothing when trying to delete non-custom targets
 
 jest.spyOn(defaultServices.targets, 'targets')
     .mockReturnValue(of([mockFooTarget, mockBarTarget]))
@@ -90,6 +98,9 @@ jest.spyOn(defaultServices.targets, 'queryForTargets')
 jest.spyOn(defaultServices.notificationChannel, 'messages')
     .mockReturnValue(of())
 
+
+afterEach(cleanup);
+
 describe('<TargetSelect />', () => {
   it('renders correctly', () => {
     const tree = renderer.create(    
@@ -154,13 +165,16 @@ describe('<TargetSelect />', () => {
     expect(createTargetRequestSpy).toBeCalledWith(mockBazTarget);
   }); 
 
-  it('deletes target when delete button clicked', () => {
+  it('deletes target when delete button clicked', async () => {
     render(
         <ServiceContext.Provider value={defaultServices}>
             <TargetSelect />
         </ServiceContext.Provider>
     );
+
     const deleteButton = screen.getByLabelText('Delete target');
+    await waitFor(() => expect(deleteButton).not.toBeDisabled());
+
     userEvent.click(deleteButton);
 
     const deleteTargetRequestSpy = jest.spyOn(defaultServices.api, 'deleteTarget');
@@ -168,6 +182,21 @@ describe('<TargetSelect />', () => {
     expect(deleteTargetRequestSpy).toBeCalledWith(mockFooTarget);
   }); 
 
+  it('does nothing when trying to delete non-custom targets', () => {
+    render(
+        <ServiceContext.Provider value={defaultServices}>
+            <TargetSelect />
+        </ServiceContext.Provider>
+    );
+    const deleteButton = screen.getByLabelText('Delete target');
+    userEvent.click(deleteButton);
+
+    const deleteTargetRequestSpy = jest.spyOn(defaultServices.api, 'deleteTarget');
+
+    expect(deleteTargetRequestSpy).toBeCalledTimes(0);
+    expect(deleteButton).toBeDisabled();
+  }); 
+
   it('refreshes targets when button clicked', () => {
     render(
         <ServiceContext.Provider value={defaultServices}>