Prevent entering of ':' into search term (#414)

* Do not allow input of ':'

* Hook up potential error message

* Update error text and test

amundsen_application/static/js/components/common/SearchBar/constants.ts

@@ -4,3 +4,5 @@ export const PLACEHOLDER_DEFAULT = 'search for data resources...';
 export const BUTTON_CLOSE_TEXT = 'Close';
 
 export const SIZE_SMALL = 'small';
+
+export const INVALID_SYNTAX_MESSAGE = "Your search term contains invalid syntax ':'.";

amundsen_application/static/js/components/common/SearchBar/index.tsx

@@ -16,6 +16,7 @@ import './styles.scss';
 
 import {
   BUTTON_CLOSE_TEXT,
+  INVALID_SYNTAX_MESSAGE,
   PLACEHOLDER_DEFAULT,
   SIZE_SMALL
 } from './constants';
@@ -92,19 +93,23 @@ export class SearchBar extends React.Component<SearchBarProps, SearchBarState> {
   handleValueChange = (event: React.SyntheticEvent<HTMLInputElement>) : void => {
     const searchTerm = (event.target as HTMLInputElement).value.toLowerCase();
 
-    if (searchTerm.length > 0) {
-      this.props.onInputChange(searchTerm);
-      this.setState({ searchTerm, showTypeAhead: true });
-    }
-    else {
-      this.clearSearchTerm();
+    if (this.isFormValid(searchTerm)) {
+      if (searchTerm.length > 0) {
+        this.props.onInputChange(searchTerm);
+        this.setState({ searchTerm, showTypeAhead: true });
+      }
+      else {
+        this.clearSearchTerm();
+      }
+    } else {
+      this.setState({ searchTerm, showTypeAhead: false });
     }
   };
 
   handleValueSubmit = (event: React.FormEvent<HTMLFormElement>) : void => {
     const searchTerm = this.state.searchTerm.trim();
     event.preventDefault();
-    if (this.isFormValid()) {
+    if (this.isFormValid(searchTerm)) {
       this.props.submitSearch(searchTerm);
       this.hideTypeAhead();
     }
@@ -114,9 +119,20 @@ export class SearchBar extends React.Component<SearchBarProps, SearchBarState> {
     this.setState({ showTypeAhead: false });
   };
 
-  isFormValid = () : boolean => {
+  isFormValid = (searchTerm: string) : boolean => {
     const form = document.getElementById("search-bar-form") as HTMLFormElement;
-    return form.checkValidity();
+    const input = document.getElementById("search-input") as HTMLInputElement;
+    const isValid = searchTerm.indexOf(':') < 0;
+
+    /* This will set the error message, it must be explicitly set or cleared each time */
+    input.setCustomValidity(isValid ? "": INVALID_SYNTAX_MESSAGE)
+
+    if (searchTerm.length > 0) {
+      /* This will show the error message */
+      form.reportValidity();
+    }
+
+    return isValid;
   };
 
   onSelectInlineResult = (resourceType: ResourceType, updateUrl: boolean = false) : void => {

amundsen_application/static/js/components/common/SearchBar/tests/index.spec.tsx

@@ -90,39 +90,61 @@ describe('SearchBar', () => {
   describe('handleValueChange', () => {
     let props;
     let wrapper;
+    let isFormValidSpy;
     beforeAll(() => {
       const setupResult = setup();
       props = setupResult.props;
       wrapper = setupResult.wrapper;
+      isFormValidSpy = jest.spyOn(wrapper.instance(), 'isFormValid');
     });
 
-    describe('if searchTerm has length', () => {
-      const mockSearchTerm = 'I have Length';
-      const expectedSearchTerm = 'i have length';
-      it('calls setState with searchTerm as lowercase target value & showTypeAhead as true ', () => {
-        setStateSpy.mockClear();
-        // @ts-ignore: mocked events throw type errors
-        wrapper.instance().handleValueChange({ target: { value: mockSearchTerm } });
-        expect(setStateSpy).toHaveBeenCalledWith({ searchTerm: expectedSearchTerm, showTypeAhead: true });
-      });
+    describe('when form is not valid', () => {
+      beforeAll(() => {
+        isFormValidSpy.mockImplementation(() => false)
+      })
 
-      it('calls onInputChange with searchTerm as lowercase target value', () => {
+      it('updates the searchTerm (for visual feedback) and hides type ahead', () => {
+        setStateSpy.mockClear();
+        const testTerm = 'hello';
         // @ts-ignore: mocked events throw type errors
-        props.onInputChange.mockClear();
-        wrapper.instance().handleValueChange({ target: { value: mockSearchTerm } });
-        expect(props.onInputChange).toHaveBeenCalledWith(expectedSearchTerm);
-      });
+        wrapper.instance().handleValueChange({ target: { value: testTerm } });
+        expect(setStateSpy).toHaveBeenCalledWith({ searchTerm: testTerm, showTypeAhead: false });
+      })
     })
 
-    describe('if searchTerm has zero length', () => {
-      const mockSearchTerm = '';
-      it('calls clearSearchTerm', () => {
-        const clearSearchTermSpy = jest.spyOn(wrapper.instance(), 'clearSearchTerm');
-        // @ts-ignore: mocked events throw type errors
-        wrapper.instance().handleValueChange({ target: { value: mockSearchTerm } });
-        expect(clearSearchTermSpy).toHaveBeenCalled();
+    describe('when form is valid', () => {
+      beforeAll(() => {
+        isFormValidSpy.mockImplementation(() => true)
+      })
+
+      describe('if searchTerm has length', () => {
+        const mockSearchTerm = 'I have Length';
+        const expectedSearchTerm = 'i have length';
+        it('calls setState with searchTerm as lowercase target value & showTypeAhead as true ', () => {
+          setStateSpy.mockClear();
+          // @ts-ignore: mocked events throw type errors
+          wrapper.instance().handleValueChange({ target: { value: mockSearchTerm } });
+          expect(setStateSpy).toHaveBeenCalledWith({ searchTerm: expectedSearchTerm, showTypeAhead: true });
+        });
+
+        it('calls onInputChange with searchTerm as lowercase target value', () => {
+          // @ts-ignore: mocked events throw type errors
+          props.onInputChange.mockClear();
+          wrapper.instance().handleValueChange({ target: { value: mockSearchTerm } });
+          expect(props.onInputChange).toHaveBeenCalledWith(expectedSearchTerm);
+        });
+      })
+
+      describe('if searchTerm has zero length', () => {
+        const mockSearchTerm = '';
+        it('calls clearSearchTerm', () => {
+          const clearSearchTermSpy = jest.spyOn(wrapper.instance(), 'clearSearchTerm');
+          // @ts-ignore: mocked events throw type errors
+          wrapper.instance().handleValueChange({ target: { value: mockSearchTerm } });
+          expect(clearSearchTermSpy).toHaveBeenCalled();
+        });
       });
-    });
+    })
   });
 
   describe('handleValueSubmit', () => {