From b3bf1b7e2990923afd41de5f3e26b04bd1a1d86f Mon Sep 17 00:00:00 2001 From: Christopher Gokey Date: Mon, 25 Nov 2024 08:43:17 -0500 Subject: [PATCH 1/8] MMT-3926: Fix for Science Keyword Picker Adds An Additional Science Keyword That Was Not Originally Selected --- .../KeywordPicker/KeywordPicker.jsx | 4 +++ .../__tests__/KeywordPicker.test.jsx | 35 ++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/static/src/js/components/KeywordPicker/KeywordPicker.jsx b/static/src/js/components/KeywordPicker/KeywordPicker.jsx index e1a38806c..2d8de47d7 100644 --- a/static/src/js/components/KeywordPicker/KeywordPicker.jsx +++ b/static/src/js/components/KeywordPicker/KeywordPicker.jsx @@ -262,6 +262,10 @@ const KeywordPicker = ({ // When a item from the search Typeahead is selected, this will capture that selection and add it // formData. const handleSearch = (text) => { + if (text.length === 0) { + return + } + const split = text.toString().split('>') const filter = fullPath.filter((path) => path.indexOf(split[split.length - 1]) !== -1) const filterArr = filter[0].toString().split('>').slice(1) diff --git a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx index 82998d344..f3deecdcd 100644 --- a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx +++ b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx @@ -1,4 +1,8 @@ -import { render, screen } from '@testing-library/react' +import { + fireEvent, + render, + screen +} from '@testing-library/react' import userEvent from '@testing-library/user-event' import React from 'react' @@ -366,6 +370,35 @@ describe('when searching for a keyword', () => { ]) }) + describe('when clicking the clear button in the textfield', () => { + test('it will only clear the text and not add another keyword', async () => { + const { props, user } = setup() + + const searchBox = screen.getByRole('combobox') + + await user.type(searchBox, 'Earth') + const option = screen.getByRole('option', { name: 'EARTH SCIENCE SERVICES>DATA ANALYSIS AND VISUALIZATION>GEOGRAPHIC INFORMATION SYSTEMS>DESKTOP GEOGRAPHIC INFORMATION SYSTEMS' }) + await user.click(option) + + const clearButton = screen.getByRole('button', { name: 'Clear' }) + console.log('clear button is ', clearButton) + + expect(props.onChange).toHaveBeenCalledTimes(1) + expect(props.onChange).not.toHaveBeenCalledWith([ + { + ToolCategory: 'EARTH SCIENCE SERVICES', + ToolTopic: 'DATA ANALYSIS AND VISUALIZATION', + ToolTerm: 'CALIBRATION/VALIDATION', + ToolSpecificTerm: undefined + } + ]) + + // MMT-3926 - The behavior before this fix caused it to perform another onChange of a keyword. + fireEvent.click(clearButton) + expect(props.onChange).toHaveBeenCalledTimes(1) // Still only called once. + }) + }) + describe('when keyword recommender is included', () => { test('keyword picker is rendered', () => { setup({}, true) From f6799c5ad886b32d6ffddb58ca3074d6061caabf Mon Sep 17 00:00:00 2001 From: Christopher Gokey Date: Mon, 25 Nov 2024 08:47:43 -0500 Subject: [PATCH 2/8] MMT-3926: Removed console log --- .../js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx index f3deecdcd..34067aac4 100644 --- a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx +++ b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx @@ -381,7 +381,6 @@ describe('when searching for a keyword', () => { await user.click(option) const clearButton = screen.getByRole('button', { name: 'Clear' }) - console.log('clear button is ', clearButton) expect(props.onChange).toHaveBeenCalledTimes(1) expect(props.onChange).not.toHaveBeenCalledWith([ From a067b0a084ab0eb916257a0f99d56864f2d4ea74 Mon Sep 17 00:00:00 2001 From: Christopher Gokey Date: Tue, 26 Nov 2024 14:17:52 -0500 Subject: [PATCH 3/8] MMT-3926: Now clears the typeahead selection after picking an item in the list. --- static/src/js/components/KeywordPicker/KeywordPicker.jsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/static/src/js/components/KeywordPicker/KeywordPicker.jsx b/static/src/js/components/KeywordPicker/KeywordPicker.jsx index 2d8de47d7..982840531 100644 --- a/static/src/js/components/KeywordPicker/KeywordPicker.jsx +++ b/static/src/js/components/KeywordPicker/KeywordPicker.jsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react' +import React, { useEffect, useRef, useState } from 'react' import PropTypes from 'prop-types' import { cloneDeep, isEmpty } from 'lodash-es' import { Typeahead } from 'react-bootstrap-typeahead' @@ -48,6 +48,7 @@ const KeywordPicker = ({ const [marginTop, setMarginTop] = useState(0) const [marginLeft, setMarginLeft] = useState(0) + const typeaheadRef = useRef(null) const filterKeywords = (path) => { const filteredPath = [] @@ -276,6 +277,7 @@ const KeywordPicker = ({ } onChange(formData) + typeaheadRef.current.clear() } const displayItems = (item) => { @@ -431,6 +433,7 @@ const KeywordPicker = ({
Date: Tue, 26 Nov 2024 15:40:25 -0500 Subject: [PATCH 4/8] MMT-3926: Fixed imports --- static/src/js/components/KeywordPicker/KeywordPicker.jsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/static/src/js/components/KeywordPicker/KeywordPicker.jsx b/static/src/js/components/KeywordPicker/KeywordPicker.jsx index 982840531..e853f7dae 100644 --- a/static/src/js/components/KeywordPicker/KeywordPicker.jsx +++ b/static/src/js/components/KeywordPicker/KeywordPicker.jsx @@ -1,4 +1,8 @@ -import React, { useEffect, useRef, useState } from 'react' +import React, { + useEffect, + useRef, + useState +} from 'react' import PropTypes from 'prop-types' import { cloneDeep, isEmpty } from 'lodash-es' import { Typeahead } from 'react-bootstrap-typeahead' From c50f8ebedbffe3164bbc10e64ee459a5c865541c Mon Sep 17 00:00:00 2001 From: Christopher Gokey Date: Tue, 26 Nov 2024 15:54:51 -0500 Subject: [PATCH 5/8] MMT-3926: Removed unnecessary test since we are clearing the selection after picking a keyword. --- .../__tests__/KeywordPicker.test.jsx | 34 +------------------ 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx index 34067aac4..82998d344 100644 --- a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx +++ b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx @@ -1,8 +1,4 @@ -import { - fireEvent, - render, - screen -} from '@testing-library/react' +import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import React from 'react' @@ -370,34 +366,6 @@ describe('when searching for a keyword', () => { ]) }) - describe('when clicking the clear button in the textfield', () => { - test('it will only clear the text and not add another keyword', async () => { - const { props, user } = setup() - - const searchBox = screen.getByRole('combobox') - - await user.type(searchBox, 'Earth') - const option = screen.getByRole('option', { name: 'EARTH SCIENCE SERVICES>DATA ANALYSIS AND VISUALIZATION>GEOGRAPHIC INFORMATION SYSTEMS>DESKTOP GEOGRAPHIC INFORMATION SYSTEMS' }) - await user.click(option) - - const clearButton = screen.getByRole('button', { name: 'Clear' }) - - expect(props.onChange).toHaveBeenCalledTimes(1) - expect(props.onChange).not.toHaveBeenCalledWith([ - { - ToolCategory: 'EARTH SCIENCE SERVICES', - ToolTopic: 'DATA ANALYSIS AND VISUALIZATION', - ToolTerm: 'CALIBRATION/VALIDATION', - ToolSpecificTerm: undefined - } - ]) - - // MMT-3926 - The behavior before this fix caused it to perform another onChange of a keyword. - fireEvent.click(clearButton) - expect(props.onChange).toHaveBeenCalledTimes(1) // Still only called once. - }) - }) - describe('when keyword recommender is included', () => { test('keyword picker is rendered', () => { setup({}, true) From f35a0d45113d2333a388065b1b168a9d3254c2f6 Mon Sep 17 00:00:00 2001 From: Christopher Gokey Date: Tue, 26 Nov 2024 16:20:35 -0500 Subject: [PATCH 6/8] MMT-3926: The check for empty is not needed anymore since I'm clearing out the text after picking a keyword. --- static/src/js/components/KeywordPicker/KeywordPicker.jsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/static/src/js/components/KeywordPicker/KeywordPicker.jsx b/static/src/js/components/KeywordPicker/KeywordPicker.jsx index e853f7dae..70afe2398 100644 --- a/static/src/js/components/KeywordPicker/KeywordPicker.jsx +++ b/static/src/js/components/KeywordPicker/KeywordPicker.jsx @@ -267,10 +267,6 @@ const KeywordPicker = ({ // When a item from the search Typeahead is selected, this will capture that selection and add it // formData. const handleSearch = (text) => { - if (text.length === 0) { - return - } - const split = text.toString().split('>') const filter = fullPath.filter((path) => path.indexOf(split[split.length - 1]) !== -1) const filterArr = filter[0].toString().split('>').slice(1) From 61c08a53f4e306af692c2cc88bb50dcf2f6b752f Mon Sep 17 00:00:00 2001 From: Christopher Gokey Date: Fri, 6 Dec 2024 14:01:31 -0500 Subject: [PATCH 7/8] MMT-3926: Added test for clearing the selection --- .../__tests__/KeywordPicker.test.jsx | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx index 82998d344..3fb74fc70 100644 --- a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx +++ b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx @@ -2,6 +2,7 @@ import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import React from 'react' +import { test } from 'vitest' import useControlledKeywords from '../../../hooks/useControlledKeywords' import parseCmrResponse from '../../../utils/parseCmrResponse' @@ -346,24 +347,39 @@ describe('when removing selected keyword', () => { }) describe('when searching for a keyword', () => { - test('adds the searched keyword', async () => { - const { props, user } = setup() + describe('and user selects a keyword', () => { + test('adds the searched keyword', async () => { + const { props, user } = setup() + + const searchBox = screen.getByRole('combobox') + + await user.type(searchBox, 'Earth') + const option = screen.getByRole('option', { name: 'EARTH SCIENCE SERVICES>DATA ANALYSIS AND VISUALIZATION>GEOGRAPHIC INFORMATION SYSTEMS>DESKTOP GEOGRAPHIC INFORMATION SYSTEMS' }) + await user.click(option) + + expect(props.onChange).toHaveBeenCalledTimes(1) + expect(props.onChange).not.toHaveBeenCalledWith([ + { + ToolCategory: 'EARTH SCIENCE SERVICES', + ToolTopic: 'DATA ANALYSIS AND VISUALIZATION', + ToolTerm: 'CALIBRATION/VALIDATION', + ToolSpecificTerm: undefined + } + ]) + }) - const searchBox = screen.getByRole('combobox') + test('clears the search box after the selection', async () => { + const { user } = setup() - await user.type(searchBox, 'Earth') - const option = screen.getByRole('option', { name: 'EARTH SCIENCE SERVICES>DATA ANALYSIS AND VISUALIZATION>GEOGRAPHIC INFORMATION SYSTEMS>DESKTOP GEOGRAPHIC INFORMATION SYSTEMS' }) - await user.click(option) + const searchBox = screen.getByRole('combobox') - expect(props.onChange).toHaveBeenCalledTimes(1) - expect(props.onChange).not.toHaveBeenCalledWith([ - { - ToolCategory: 'EARTH SCIENCE SERVICES', - ToolTopic: 'DATA ANALYSIS AND VISUALIZATION', - ToolTerm: 'CALIBRATION/VALIDATION', - ToolSpecificTerm: undefined - } - ]) + await user.type(searchBox, 'Earth') + + expect(searchBox).toHaveValue('Earth') + const option = screen.getByRole('option', { name: 'EARTH SCIENCE SERVICES>DATA ANALYSIS AND VISUALIZATION>GEOGRAPHIC INFORMATION SYSTEMS>DESKTOP GEOGRAPHIC INFORMATION SYSTEMS' }) + await user.click(option) + expect(searchBox).toHaveValue('') + }) }) describe('when keyword recommender is included', () => { From 789bdbf0fd88077ad084692fc40c39f76187fb1f Mon Sep 17 00:00:00 2001 From: Christopher Gokey Date: Mon, 9 Dec 2024 13:48:43 -0500 Subject: [PATCH 8/8] MMT-3926: Updated test to verify keyword picked. --- .../KeywordPicker/__tests__/KeywordPicker.test.jsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx index 3fb74fc70..ea85197d7 100644 --- a/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx +++ b/static/src/js/components/KeywordPicker/__tests__/KeywordPicker.test.jsx @@ -358,12 +358,12 @@ describe('when searching for a keyword', () => { await user.click(option) expect(props.onChange).toHaveBeenCalledTimes(1) - expect(props.onChange).not.toHaveBeenCalledWith([ + expect(props.onChange).toHaveBeenCalledWith([ { ToolCategory: 'EARTH SCIENCE SERVICES', ToolTopic: 'DATA ANALYSIS AND VISUALIZATION', - ToolTerm: 'CALIBRATION/VALIDATION', - ToolSpecificTerm: undefined + ToolTerm: 'GEOGRAPHIC INFORMATION SYSTEMS', + ToolSpecificTerm: 'DESKTOP GEOGRAPHIC INFORMATION SYSTEMS' } ]) })