refactor: Query search into functional component

[AAfghahi]

SUMMARY

Made Query Search into a functional component from a class component.

Added to package.json because I added userEvent.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Only refreshesQueries when you press enter in the QuerySearch or press the enter key:

TEST PLAN

I need to implement two more tests for checking when refreshQueries works.

ADDITIONAL INFORMATION

• Has associated issue:
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

[eschutho]

remove?

[eschutho]

@AAfghahi just ping me when you need another look!

[eschutho]

I see the issue here. You're saying that setDatabaseId and setUserId are called elsewhere and it would trigger the refreshQueries function as well. Yeah, looks like what you have is the best bet. good catch on that.

[codecov]

Codecov Report

Merging #13102 (0ec95d2) into master (892eef1) will decrease coverage by 3.91%.
The diff coverage is 72.87%.

@@            Coverage Diff             @@
##           master   #13102      +/-   ##
==========================================
- Coverage   77.05%   73.14%   -3.92%     
==========================================
  Files         897      599     -298     
  Lines       45710    21135   -24575     
  Branches     5497     5406      -91     
==========================================
- Hits        35224    15460   -19764     
+ Misses      10362     5549    -4813     
- Partials      124      126       +2     

Flag	Coverage Δ
cypress	58.00% <56.40%> (+0.40%)	⬆️
hive	?
javascript	62.94% <56.91%> (+0.45%)	⬆️
mysql	?
postgres	?
presto	?
python	?
sqlite	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset-frontend/src/chart/ChartContainer.jsx	100.00% <ø> (ø)
superset-frontend/src/chart/ChartRenderer.jsx	76.00% <0.00%> (-1.03%)	⬇️
...rset-frontend/src/components/ButtonGroup/index.tsx	100.00% <ø> (ø)
...et-frontend/src/components/ErrorBoundary/index.jsx	95.45% <ø> (ø)
...perset-frontend/src/components/FormLabel/index.tsx	100.00% <ø> (ø)
...-frontend/src/components/Select/Select.stories.tsx	0.00% <0.00%> (ø)
superset-frontend/src/components/Select/Select.tsx	90.81% <ø> (ø)
superset-frontend/src/components/Select/styles.tsx	86.30% <ø> (ø)
...et-frontend/src/components/Timer/Timer.stories.tsx	0.00% <0.00%> (ø)
superset-frontend/src/components/Timer/index.tsx	95.83% <ø> (ø)
... and 372 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 892eef1...93e3a59. Read the comment docs.

[eschutho]

@AAfghahi you can remove the package-lock.json file since you don't have a package.json update.

[eschutho]

prob don't need this async here/no await

[AAfghahi]

you don't need it for the test to pass, but without them the console throws a console warning and tells you to do it.

[eschutho]

we usually call name these in TS with the name of the component, i.e., QuerySearchProps

[eschutho]

can we be more specific on the type? string or number?

[eschutho]

array type?

[AAfghahi]

So, this one is a bit wonky because the queriesArray is actually taking in a json object that it puts in is an object. I went through some iteration where i was parsing through the json object you receive and pushing it into the empty array, but it wasn't rendering the table.

My issue here was that I can create an object prop and then put it in there, but then that would need me to also create aa prop object for the useState (though I could potentially use null, now that I am thinking about it?).

[eschutho]

this looks great. I think you can remove the comment?

[eschutho]

you can name this response. It looks like it's returning an object rather than a promise.

[eschutho]

I think React.KeyboardEvent will work

[eschutho]

React.ChangeEvent

[eschutho]

what does this do?

[AAfghahi]

oh! Yeah this is totally because of the debouncer. Apparently when you have a debouncer in hooks the event.target.value gets lost, so you have to tell it to persist the value. Without this formatting you could type something into the search bar, and it would show up on the screen but it wouldn't actually search for it. Kind of wild.

[eschutho]

you can also put the debounce where you call onChange below, so
onChange={debounce(onChange, 200)}

[AAfghahi]

I tried this, but ran into the same problem as above.

[eschutho]

ok, we can loop back to this another time.

[eschutho]

let's try to define something here rather than any

[AAfghahi]

yeah this bothered me too, and I looked through it is, is there a way to set up an object type without having to go through and create all the props for the object? Looking through the typescript documentation it didn't seem that there was.

[eschutho]

same thing on any

[eschutho]

nit, but you might as well destructure id and database_name in the arg list.

[eschutho]

looking great. I just left some ts feedback!

[AAfghahi]

@AAfghahi you can remove the package-lock.json file since you don't have a package.json update.
How do I do that? Is this a rebase or git merge master? Becuase I've tried both and this still persists.

[eschutho]

Thanks @AAfghahi. I think we're ready!

[eschutho]

🏷 ready-to-merge