fix: Can't toggle off from the ad-hoc tools menu if you've toggled on…

… in DetailsView (#7431) #### Details Two ad-hoc tools (Automated Checks and Needs Review) have the ability to view issues in DetailsView. Unfortunately, if you try to toggle either of them off in ad-hoc tools while their corresponding DetailsView list of issues is open, the DetailsView toggle will overwrite the "off" state and force it to be "on". Additionally, toggling off in DetailsView does not turn the toggle off in ad-hoc tools. This PR keeps these toggles in sync by: * moving the code that determines if a scan needs to be run based on state to `componentDidMount` instead of on render. * triggering `VisualizationActions.enableVisualization`/`VisualizationActions.disabledVisualization` when `CardSelectionActions.toggleVisualHelper` is triggered, which involves: * making sure `VisualizationActions` are accessible from `CardSelectionActionCreator`s * sending a `VisualizationTogglePayload` instead of `BasePayload` with the enabled state (passed in from the `VisualHelperToggle`'s `onClick`) * triggering `CardSelectionActions.toggleVisualHelper` when `VisualizationActions.enableVisualization`/`VisualizationActions.disabledVisualization` are triggered, which involves: * checking the value of `payload.test` and triggering `needsReviewSelectionActions.toggleVisualHelper` or `cardSelectionActions.toggleVisualHelper` if the `VisualizationType` matches. ##### Motivation Addresses issue #6253 ##### Context   #### Pull request checklist  - [x] Addresses an existing issue: #6253 - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. --------- Co-authored-by: Madalyn Parker <[email protected]>
microsoft · Aug 29, 2024 · 9c0dafa · 9c0dafa
1 parent b4de539
commit 9c0dafa
Show file tree

Hide file tree

Showing 15 changed files with 228 additions and 51 deletions.
diff --git a/src/DetailsView/components/issues-table.tsx b/src/DetailsView/components/issues-table.tsx
@@ -62,6 +62,24 @@ export class IssuesTable extends React.Component<IssuesTableProps> {
         super(props);
     }
 
+    componentDidMount(): void {
+        const cardCount = this.getCardCount();
+        const assessment = this.props.getProvider().forType(this.props.selectedVisualizationType);
+        const requirement = assessment?.requirements[0].key;
+        if (!this.props.issuesEnabled && cardCount > 0) {
+            this.props.deps.detailsViewActionMessageCreator.enableFastPassVisualHelperWithoutScan(
+                this.props.selectedVisualizationType,
+                requirement,
+            );
+        }
+        if (!this.props.issuesEnabled && cardCount === 0) {
+            this.props.deps.detailsViewActionMessageCreator.rescanVisualizationWithoutTelemetry(
+                this.props.selectedVisualizationType,
+                requirement,
+            );
+        }
+    }
+
     public render(): JSX.Element {
         return (
             <div className={styles.issuesTable}>
@@ -109,22 +127,6 @@ export class IssuesTable extends React.Component<IssuesTableProps> {
     }
 
     private renderComponent(): JSX.Element {
-        const cardCount = this.getCardCount();
-        const assessment = this.props.getProvider().forType(this.props.selectedVisualizationType);
-        const requirement = assessment?.requirements[0].key;
-        if (!this.props.issuesEnabled && cardCount > 0) {
-            this.props.deps.detailsViewActionMessageCreator.enableFastPassVisualHelperWithoutScan(
-                this.props.selectedVisualizationType,
-                requirement,
-            );
-        }
-        if (!this.props.issuesEnabled && cardCount === 0) {
-            this.props.deps.detailsViewActionMessageCreator.rescanVisualizationWithoutTelemetry(
-                this.props.selectedVisualizationType,
-                requirement,
-            );
-        }
-
         if (this.props.scanning) {
             return this.renderSpinner('Scanning...');
         }

diff --git a/src/background/actions/action-creator.ts b/src/background/actions/action-creator.ts
@@ -475,6 +475,16 @@ export class ActionCreator {
                 this.executingScope,
             );
         }
+        switch (payload.test) {
+            case VisualizationType.Issues:
+                await this.cardSelectionActions.toggleVisualHelper.invoke(null);
+                break;
+            case VisualizationType.NeedsReview:
+                await this.needsReviewCardSelectionActions.toggleVisualHelper.invoke(null);
+                break;
+            default:
+                break;
+        }
     };
 
     private onRescanVisualization = async (payload: RescanVisualizationPayload) => {

diff --git a/src/background/actions/card-selection-action-creator.ts b/src/background/actions/card-selection-action-creator.ts
@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
+import { VisualizationActions } from 'background/actions/visualization-actions';
 import { StoreNames } from 'common/stores/store-names';
 
 import * as TelemetryEvents from '../../common/extension-telemetry-events';
@@ -11,12 +12,14 @@ import {
     BaseActionPayload,
     CardSelectionPayload,
     RuleExpandCollapsePayload,
+    VisualizationTogglePayload,
 } from './action-payloads';
 
 export class CardSelectionActionCreator {
     constructor(
         private readonly interpreter: Interpreter,
         private readonly cardSelectionActions: CardSelectionActions,
+        private readonly visualizationActions: VisualizationActions,
         private readonly telemetryEventHandler: TelemetryEventHandler,
     ) {}
 
@@ -67,8 +70,13 @@ export class CardSelectionActionCreator {
         );
     };
 
-    private onToggleVisualHelper = async (payload: BaseActionPayload): Promise<void> => {
+    private onToggleVisualHelper = async (payload: VisualizationTogglePayload): Promise<void> => {
         await this.cardSelectionActions.toggleVisualHelper.invoke(null);
+        if (payload.enabled) {
+            await this.visualizationActions.disableVisualization.invoke(payload.test);
+        } else {
+            await this.visualizationActions.enableVisualization.invoke(payload);
+        }
         this.telemetryEventHandler.publishTelemetry(TelemetryEvents.VISUAL_HELPER_TOGGLED, payload);
     };
 

diff --git a/src/background/actions/needs-review-card-selection-action-creator.ts b/src/background/actions/needs-review-card-selection-action-creator.ts
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 import { NeedsReviewCardSelectionActions } from 'background/actions/needs-review-card-selection-actions';
+import { VisualizationActions } from 'background/actions/visualization-actions';
 import { StoreNames } from 'common/stores/store-names';
 
 import * as TelemetryEvents from '../../common/extension-telemetry-events';
@@ -11,12 +12,14 @@ import {
     BaseActionPayload,
     CardSelectionPayload,
     RuleExpandCollapsePayload,
+    VisualizationTogglePayload,
 } from './action-payloads';
 
 export class NeedsReviewCardSelectionActionCreator {
     constructor(
         private readonly interpreter: Interpreter,
         private readonly needsReviewCardSelectionActions: NeedsReviewCardSelectionActions,
+        private readonly visualizationActions: VisualizationActions,
         private readonly telemetryEventHandler: TelemetryEventHandler,
     ) {}
 
@@ -69,8 +72,13 @@ export class NeedsReviewCardSelectionActionCreator {
         );
     };
 
-    private onToggleVisualHelper = async (payload: BaseActionPayload): Promise<void> => {
+    private onToggleVisualHelper = async (payload: VisualizationTogglePayload): Promise<void> => {
         await this.needsReviewCardSelectionActions.toggleVisualHelper.invoke(null);
+        if (payload.enabled) {
+            await this.visualizationActions.disableVisualization.invoke(payload.test);
+        } else {
+            await this.visualizationActions.enableVisualization.invoke(payload);
+        }
         this.telemetryEventHandler.publishTelemetry(TelemetryEvents.VISUAL_HELPER_TOGGLED, payload);
     };
 

diff --git a/src/background/tab-context-factory.ts b/src/background/tab-context-factory.ts
@@ -152,11 +152,13 @@ export class TabContextFactory {
         const cardSelectionActionCreator = new CardSelectionActionCreator(
             interpreter,
             actionsHub.cardSelectionActions,
+            actionsHub.visualizationActions,
             this.telemetryEventHandler,
         );
         const needsReviewCardSelectionActionCreator = new NeedsReviewCardSelectionActionCreator(
             interpreter,
             actionsHub.needsReviewCardSelectionActions,
+            actionsHub.visualizationActions,
             this.telemetryEventHandler,
         );
         const injectionActionCreator = new InjectionActionCreator(

diff --git a/src/common/components/cards/visual-helper-toggle.tsx b/src/common/components/cards/visual-helper-toggle.tsx
@@ -15,7 +15,12 @@ export type VisualHelperToggleProps = {
 export const VisualHelperToggle = NamedFC<VisualHelperToggleProps>('VisualHelperToggle', props => {
     return (
         <Toggle
-            onClick={props.cardSelectionMessageCreator.toggleVisualHelper}
+            onClick={event => {
+                props.cardSelectionMessageCreator.toggleVisualHelper(
+                    event,
+                    props.visualHelperEnabled,
+                );
+            }}
             label="Visual helper"
             checked={props.visualHelperEnabled}
             className={css(styles.visualHelperToggle, props.className)}

diff --git a/src/common/message-creators/automated-checks-card-selection-message-creator.ts b/src/common/message-creators/automated-checks-card-selection-message-creator.ts
@@ -4,12 +4,14 @@ import {
     BaseActionPayload,
     CardSelectionPayload,
     RuleExpandCollapsePayload,
+    VisualizationTogglePayload,
 } from 'background/actions/action-payloads';
 import { TelemetryEventSource } from 'common/extension-telemetry-events';
 import { CardSelectionMessageCreator } from 'common/message-creators/card-selection-message-creator';
 import { ActionMessageDispatcher } from 'common/message-creators/types/dispatcher';
 import { Messages } from 'common/messages';
 import { SupportedMouseEvent, TelemetryDataFactory } from 'common/telemetry-data-factory';
+import { VisualizationType } from 'common/types/visualization-type';
 
 export class AutomatedChecksCardSelectionMessageCreator implements CardSelectionMessageCreator {
     constructor(
@@ -69,9 +71,14 @@ export class AutomatedChecksCardSelectionMessageCreator implements CardSelection
         });
     };
 
-    public toggleVisualHelper = (event: SupportedMouseEvent) => {
-        const payload: BaseActionPayload = {
-            telemetry: this.telemetryFactory.withTriggeredByAndSource(event, this.source),
+    public toggleVisualHelper = (event: SupportedMouseEvent, isEnabled: boolean) => {
+        const payload: VisualizationTogglePayload = {
+            enabled: isEnabled,
+            test: VisualizationType.Issues,
+            telemetry: {
+                ...this.telemetryFactory.withTriggeredByAndSource(event, this.source),
+                enabled: isEnabled,
+            },
         };
 
         this.dispatcher.dispatchMessage({

diff --git a/src/common/message-creators/card-selection-message-creator.ts b/src/common/message-creators/card-selection-message-creator.ts
@@ -12,5 +12,5 @@ export interface CardSelectionMessageCreator {
     toggleRuleExpandCollapse: (ruleId: string, event: React.SyntheticEvent) => void;
     collapseAllRules: (event: SupportedMouseEvent) => void;
     expandAllRules: (event: SupportedMouseEvent) => void;
-    toggleVisualHelper: (event: SupportedMouseEvent) => void;
+    toggleVisualHelper: (event: SupportedMouseEvent, isEnabled?: boolean) => void;
 }
diff --git a/src/common/message-creators/needs-review-card-selection-message-creator.ts b/src/common/message-creators/needs-review-card-selection-message-creator.ts
@@ -4,12 +4,14 @@ import {
     BaseActionPayload,
     CardSelectionPayload,
     RuleExpandCollapsePayload,
+    VisualizationTogglePayload,
 } from 'background/actions/action-payloads';
 import { TelemetryEventSource } from 'common/extension-telemetry-events';
 import { CardSelectionMessageCreator } from 'common/message-creators/card-selection-message-creator';
 import { ActionMessageDispatcher } from 'common/message-creators/types/dispatcher';
 import { Messages } from 'common/messages';
 import { SupportedMouseEvent, TelemetryDataFactory } from 'common/telemetry-data-factory';
+import { VisualizationType } from 'common/types/visualization-type';
 
 export class NeedsReviewCardSelectionMessageCreator implements CardSelectionMessageCreator {
     constructor(
@@ -69,11 +71,15 @@ export class NeedsReviewCardSelectionMessageCreator implements CardSelectionMess
         });
     };
 
-    public toggleVisualHelper = (event: SupportedMouseEvent) => {
-        const payload: BaseActionPayload = {
-            telemetry: this.telemetryFactory.withTriggeredByAndSource(event, this.source),
+    public toggleVisualHelper = (event: SupportedMouseEvent, isEnabled: boolean) => {
+        const payload: VisualizationTogglePayload = {
+            test: VisualizationType.NeedsReview,
+            enabled: isEnabled,
+            telemetry: {
+                ...this.telemetryFactory.withTriggeredByAndSource(event, this.source),
+                enabled: isEnabled,
+            },
         };
-
         this.dispatcher.dispatchMessage({
             messageType: Messages.NeedsReviewCardSelection.ToggleVisualHelper,
             payload,

diff --git a/src/tests/unit/tests/DetailsView/components/issues-table.test.tsx b/src/tests/unit/tests/DetailsView/components/issues-table.test.tsx
@@ -86,6 +86,7 @@ describe('IssuesTableTest', () => {
 
     it('includes subtitle if specified', () => {
         const props = new TestPropsBuilder()
+            .setDeps(deps)
             .setGetProviderMock(getProviderMock)
             .setSubtitle(<>test subtitle text</>)
             .build();