From cb41db37cb384717bb483edf793973a5c7e2f3b2 Mon Sep 17 00:00:00 2001
From: Stefan Hengl <stefan@sourcegraph.com>
Date: Thu, 27 Jun 2024 12:50:12 +0200
Subject: [PATCH] search: fix regexp toggle if regexp is default (#63489)

I noticed that the regexp toggle doesn't work anymore if `"search.defaultPatternType": "regexp"`. This is related to a recent change #63410.

We also append `patternType:keyword` in that case which I don't think we want, because we have an UI element to indicate that keyword search is active.

The question I don't know how to answer is: What should happen if `regexp` is the default and the user toggles keyword search off. Should we go back to `regexp` or to `standard`?

Test plan:
- new unit test
- manual testing with default pattern type set to "keyword" and "regexp".
---
 .../search-ui/input/toggles/Toggles.test.tsx  | 33 +++++++++++++++++--
 .../src/search-ui/input/toggles/Toggles.tsx   |  8 ++++-
 .../__snapshots__/Toggles.test.tsx.snap       | 25 ++++++++++++++
 .../web/src/stores/navbarSearchQueryState.ts  |  6 +++-
 4 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/client/branded/src/search-ui/input/toggles/Toggles.test.tsx b/client/branded/src/search-ui/input/toggles/Toggles.test.tsx
index e5dce30e507..031f6c8edfd 100644
--- a/client/branded/src/search-ui/input/toggles/Toggles.test.tsx
+++ b/client/branded/src/search-ui/input/toggles/Toggles.test.tsx
@@ -1,5 +1,5 @@
-import { screen } from '@testing-library/react'
-import { describe, expect, test } from 'vitest'
+import { screen, fireEvent } from '@testing-library/react'
+import { describe, expect, test, vi } from 'vitest'
 
 import { SearchPatternType } from '@sourcegraph/shared/src/graphql-operations'
 import { SearchMode } from '@sourcegraph/shared/src/search'
@@ -83,5 +83,34 @@ describe('Toggles', () => {
             )
             expect(screen.getAllByRole('checkbox', { name: 'Regular expression toggle' })).toMatchSnapshot()
         })
+
+        test('Regex toggles off even if defaultPatternType is regexp', () => {
+            const setPatternType = vi.fn()
+
+            renderWithBrandedContext(
+                <Toggles
+                    navbarSearchQuery="foo.*bar"
+                    patternType={SearchPatternType.regexp}
+                    defaultPatternType={SearchPatternType.regexp}
+                    setPatternType={setPatternType}
+                    caseSensitive={false}
+                    setCaseSensitivity={() => undefined}
+                    searchMode={SearchMode.Precise}
+                    setSearchMode={() => undefined}
+                    telemetryService={NOOP_TELEMETRY_SERVICE}
+                    telemetryRecorder={noOpTelemetryRecorder}
+                />
+            )
+
+            // Initially, the regexp toggle should be checked
+
+            expect(screen.getByRole('checkbox', { name: 'Regular expression toggle' })).toMatchSnapshot()
+
+            // Toggle the regexp off
+            fireEvent.click(screen.getByRole('checkbox', { name: 'Regular expression toggle' }))
+
+            // Verify that setPatternType was called with patternType keyword
+            expect(setPatternType).toHaveBeenCalledWith(SearchPatternType.keyword)
+        })
     })
 })
diff --git a/client/branded/src/search-ui/input/toggles/Toggles.tsx b/client/branded/src/search-ui/input/toggles/Toggles.tsx
index dde6eee7afc..01d929f66b2 100644
--- a/client/branded/src/search-ui/input/toggles/Toggles.tsx
+++ b/client/branded/src/search-ui/input/toggles/Toggles.tsx
@@ -77,7 +77,13 @@ export const Toggles: React.FunctionComponent<React.PropsWithChildren<TogglesPro
     }, [caseSensitive, setCaseSensitivity, submitOnToggle, telemetryRecorder])
 
     const toggleRegexp = useCallback((): void => {
-        const newPatternType = patternType !== SearchPatternType.regexp ? SearchPatternType.regexp : defaultPatternType
+        const newPatternType =
+            patternType !== SearchPatternType.regexp
+                ? SearchPatternType.regexp
+                : // Handle the case where the user has regexp configured as the default pattern type.
+                defaultPatternType === SearchPatternType.regexp
+                ? SearchPatternType.keyword
+                : defaultPatternType
 
         setPatternType(newPatternType)
         submitOnToggle({ newPatternType })
diff --git a/client/branded/src/search-ui/input/toggles/__snapshots__/Toggles.test.tsx.snap b/client/branded/src/search-ui/input/toggles/__snapshots__/Toggles.test.tsx.snap
index 574ba9f76f5..ab1a3c2cc1d 100644
--- a/client/branded/src/search-ui/input/toggles/__snapshots__/Toggles.test.tsx.snap
+++ b/client/branded/src/search-ui/input/toggles/__snapshots__/Toggles.test.tsx.snap
@@ -1,5 +1,30 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
+exports[`Toggles > Query input toggle state > Regex toggles off even if defaultPatternType is regexp 1`] = `
+<div
+  aria-checked="true"
+  aria-disabled="false"
+  aria-label="Regular expression toggle"
+  class="btn btnIcon toggle test-regexp-toggle regularExpressionToggle toggleActive"
+  role="checkbox"
+  tabindex="0"
+>
+  <svg
+    aria-hidden="true"
+    class="mdi-icon iconInline"
+    fill="currentColor"
+    height="24"
+    role="img"
+    viewBox="0 0 24 24"
+    width="24"
+  >
+    <path
+      d="M16,16.92C15.67,16.97 15.34,17 15,17C14.66,17 14.33,16.97 14,16.92V13.41L11.5,15.89C11,15.5 10.5,15 10.11,14.5L12.59,12H9.08C9.03,11.67 9,11.34 9,11C9,10.66 9.03,10.33 9.08,10H12.59L10.11,7.5C10.3,7.25 10.5,7 10.76,6.76V6.76C11,6.5 11.25,6.3 11.5,6.11L14,8.59V5.08C14.33,5.03 14.66,5 15,5C15.34,5 15.67,5.03 16,5.08V8.59L18.5,6.11C19,6.5 19.5,7 19.89,7.5L17.41,10H20.92C20.97,10.33 21,10.66 21,11C21,11.34 20.97,11.67 20.92,12H17.41L19.89,14.5C19.7,14.75 19.5,15 19.24,15.24V15.24C19,15.5 18.75,15.7 18.5,15.89L16,13.41V16.92H16V16.92M5,19A2,2 0 0,1 7,17A2,2 0 0,1 9,19A2,2 0 0,1 7,21A2,2 0 0,1 5,19H5Z"
+    />
+  </svg>
+</div>
+`;
+
 exports[`Toggles > Query input toggle state > case toggle for case subexpressions 1`] = `
 Array [
   <div
diff --git a/client/web/src/stores/navbarSearchQueryState.ts b/client/web/src/stores/navbarSearchQueryState.ts
index c8616bfe48d..1ec5a8b4369 100644
--- a/client/web/src/stores/navbarSearchQueryState.ts
+++ b/client/web/src/stores/navbarSearchQueryState.ts
@@ -141,7 +141,11 @@ export function showPatternTypeInQuery(
     return patternType !== defaultPatternType && !explicitPatternTypes.has(patternType)
 }
 
-const explicitPatternTypes = new Set([SearchPatternType.regexp, SearchPatternType.structural])
+const explicitPatternTypes = new Set([
+    SearchPatternType.regexp,
+    SearchPatternType.structural,
+    SearchPatternType.keyword,
+])
 
 /**
  * Update or initialize query state related data from settings