Batch actions and the new "shouldClearFilters" option

[JasonStoltz]

Description

In its current state, Search UI has a problem. Actions trigger API calls to the Search API. When there are in-flight calls to the Search API, Search UI will block any new actions.

That means, if a user were to call multiple actions in a row like this the following, that only the first action would be applied.

addFilter("states", "Alaska", "any");
addFilter("states", "California", "any");
setSearchTerm("park");

One way to fix this, would be to simply unblock subsequent actions from making API calls. However, you would then encounter an issue where the API is making excessive API calls. If you could imagine in the example above, that would result in 3 separate API calls.

In order to cleanly avoid this, I've added the ability for actions to be automatically batched.

An action can be thought of in 2 parts:

1. A state update
2. A side-effect

The primary side-effect of most actions is to execute a new query. All I've done is debounced that side effect, so only after the user is done calling a series of actions, does the side effect get executed.

Why would someone need to do this?

We currently don't have any sort of bulk API for applying various filters. Meaning, if you had a reason to apply 2 filters at once, there is no single action to do that. You must make two calls to the addFilter action.

addFilter("states", "Alaska", "any");
addFilter("world_heritage_site", "true");

Another case, is a search experience with a submit button, like an "Advanced Search" screen. These experiences typically are not updated live as you are making filter selections. Instead, you enter all of your selections and press a submit button to apply them at once.

In the future, we should have a better solution for this. But for now, that could be accomplished by simply hooking the "Submit" button up to a handler that calls all of the appropriate actions to apply the user's selections.

The case of "setSearchTerm" and "shouldClearFilters"

Currently, "setSearchTerm" clears out existing filters any time that it is executed. This is usually, but not always, desirable. Especially if you think about the case I mentioned above, where you are implementing an "Advanced Search" feature. Calling items in this order actually ends up clearing the filters that you just added when you call "setSearchTerm":

addFilter("states", "Alaska", "any");
addFilter("states", "California", "any");
setSearchTerm("park");

I added the ability to work around this with a "shouldClearFilters" flag:

addFilter("states", "Alaska", "any");
addFilter("states", "California", "any");
setSearchTerm("park", { shouldClearFilters: false });

The case of "setSearchTerm" and "debounce"

I had to think about what the implications of debouncing both the search side effect and also allowing users to debounce searches independently when using "setSearchTerm". For example:

setSearchTerm("park", { refresh: true, debounce: 1000 });
addFilter("states", "Alaska", "any");
addFilter("states", "California", "any");

What happened in this case, is that 2 queries were being executed.

1. The initial query which applied the search term "park" as well as the 2 filters on "states".
2. The second query, on "park", with NO filters applied. This is because the "setSearchTerm" side-effect was deferred by 1000 ms and ended up executing after the initial query.

To overcome this, I added logic that cancels any outstanding side-effects, as they should be considered stale and no longer valid to run if a new side-effect is applied.

Want to test this locally?

Add the following button to App.js in the sandbox example:

<button
  onClick={() => {
    addFilter("states", "Alaska", "any");
    setSearchTerm("park", { shouldClearFilters: false });
    addFilter("states", "California", "any");
  }}
>
  Click here
</button>

Clicking that button BEFORE my changes will only apply the California filter to results.

AFTER my changes, that will apply all 3 of those filters.

List of changes

• Added batch actions
• Added a "shouldClearFilters" flag to setSearchTerm
• Added side-effect cancelling in our actions
• Added the "shouldClearFilters" option to the SearchBox component as well.

Associated Github Issues

#65

[cee-chen]

It's been a while and this seems like a pretty meaty PR so I'm going to be going very very slowly in this review! To facilitate that and so I'm not leaving a wall of comments at once I'm going to batch some of my comments into smaller groups or into a couple files at a time. Hope that's not too annoying / gives you a chance to respond to them as I go!

[JasonStoltz]

@constancecchen Yes, sorry for dragging you into this big PR :( I sincerely appreciate it!

[cee-chen]

Fantastic stuff! 🎉

[JasonStoltz]

@constancecchen This is ready for a re-review.

After giving it some thought, I think we could make a really nice refactor to the SearchDriver.

The idea I had is to clearly distinguish "side effects" inside of the driver, and pull them out into their own modules.

For example, running actions could potentially result in 4 different "side effects".

• Updating request state (i.e., "_updateSearchResults")
• Making an API request (i.e., "_makeAPIRequest")
• Updating A11y notifications
• Updating the URL state

These could all exist in their own files and be tested independently. It would help a lot cleaning up the driver and making this code more reasonable.

[JasonStoltz]

For another PR though.