Automattic / bugomattic

Bugomattic is a tool that guides bug reporters to the right actions within large, complex organizations
GNU General Public License v2.0
6 stars 0 forks source link

Making searching and URL history tracking play together nicely #95

Closed dpasque closed 1 year ago

dpasque commented 1 year ago

What Does This PR Add/Change?

Previously, our URL history state tracking and our duplicate searching controls did not work together very well.

There were two main bugs:

  1. The actual value in the search input was decoupled from the redux search value. So if you navigated back/forth, the value you saw wouldn't change, even though the state was changing. Same was true on initial load if a search term came in from the URL.
  2. We didn't actually update any of the search results! So all your search parameters would change, but all the results would be from old searches!

This PR touches a lot of files, but the core changes to fix the above are...

  1. The DebouncedSearch optionally lets you "control" the input local state, i.e., define it in the parent component and pass it down. This now lets us sync the local input state with the search term as stored in the redux state!
  2. We've moved our duplicate searching from an imperative approach (i.e., you click a button, and we dispatch a search thunk), to a reactive approach (if we see a search parameter change in the redux store, we fire a new search). We use redux middleware to implement this reactive approach because it is completely decoupled from the React render cycles (unlike useEffect). This effectively means that navigating forward and back in the browser, and doing an initial load with search parameters, will all kick off a new search, keeping the search results in sync with the search parameters. 🎉
  3. Because we now might be repeating a lot of similar searches (especially in the case of forward/back navigation), we add an in-memory cache for search requests. That makes the search result updates appear near instantaneously when using forward/back navigation.

Testing Instructions

I added a lotttt more component tests covering all kinds of permutations of history + duplicate searching!

For playing around manually, it might not be a bad idea to tweak the local-api-client.ts file and add a console.log to the search request. With caching now, it can be hard to tell when a request is firing!

The rules that govern when you should searches fire can feel a bit convoluted, so I'll lay them out here:

  1. Never fire a search if the search term is empty.
  2. If the user takes an action that dispatches an action that updates the search state in the redux store, fire a search. This is even true if the search parameters are the same! Put another way, hitting "enter" on the same search term will search again, albeit with a cache, so it may not feel like it.
  3. If the URL history changes (forward/back), only fire a search if the values of the search parameters have changed. That way, browser navigations that have nothing to do with duplicate searching aren't just spamming pointless search requests.

Issues

Related to #
Closes #93