Do we want to handle the error for getTemplate?

It is already handled. If it fails, it will throw an error that will be passed to the Alert component. But I guess, you were talking about handling errors in a more granular way. Since these operations are consecutive, one by one, the first one that fails will throw the error, so only one error will happen at a time. But even in case we could have two errors happening at the same time in this operation, I would ask: As a user are those two errors helpful? Or would be good enough to just show a single error like "Update template version failed"? IMO, as a user, usually don't need to know what is happening under the hood so I would avoid showing multiple errors which can make me confused. Makes sense?

Sure, totally makes sense. I just see a blank screen with a toast when that error is thrown. Maybe down the road and outside of the PR, we can have a cuter error state, like even an empty table.

Ah, it should not be a toast, should be an alert banner, not VERY different tho. I agree we could have better error components when there is a main loading data error.

Instead of prop drilling several levels for this, can we look for the existence of the filter query string in the URL using react-router? It doesn't seem to be present unless explicitly set by the user. Maybe your useFilter hook could return that boolean if you want to keep things colocated.

We can, but this brings back the "container/page view" discussion. If we do that, we have to change our stories in Storybook to wrap the components with the <Router /> and pass the proper routes, adding some work. Let's see how painful it is and we can take a better decision, maybe? Curious to know your thoughts.

Sounds good to me!

Curious about the separation of WorkspacesPage and WorkspacesPageView. I notice some of the layout is in the parent component, and some is in the child - should we have the parent handle all the page layout, e.g. margins, page headers, subtitles?
I also see we're passing a lot of props down from the parent still - is this to facilitate testing or do we like that pattern for another reason?

I have no strong opinion on this, I kept what we were using, to be honest. But, in the current state, I would like to only keep data logic in the Page component and all the visuals in PageView. I think in this way, we can test more logic in the stories. What about you?

If it keeps our stories running, I'm all for it.

Why would the response on onSuccess be oldResponse? It's not an updated WorkspacesResponse?

The updateWorkspaceVersion function does not return a WorkspacesResponse. It returns a workspace build of an individual workspace, so what we have to do is get the old response, find the workspace that was updated, and replace the last build with the one returned from the updateWorkspaceVersion response.

Do we need an onError key?

Ahhh, good catch. I forgot to handle the update error. Thank you! But as the useQuery, the useMutation also returns an error object so if you want to just get the error and display it in the UI no onError is needed.

const updateVersion = useMutation()

if(updateVersion.error) {
   return <Alert error={error}>
}

but in this case, I think we can use onError to trigger a displayError notification.

I might call this file hooks.ts

Yes, I thought the same and I was using it but when thinking about what this does, it just loads/handles the page data so I thought it would be a better name. Makes sense?

Not distinguishing whether this is the first page makes a search with no results say "No results on this page" which I think is a little bit confusing because it implies there are results on other pages (at least in my mind) but in this case there are actually no results at all (before it would say "No results matched your search").

Yes, I saw that in Storybook 😔 I'm going to fix it but I think there is a better way of doing that without using an extra prop. I can be wrong so let's see.

I think we might need Boolean(count) or a ternary or embed the zero logic into the component itself or something like that because I think when this is zero it displays a zero on the page.

That is true, I've been seeing this happening some times.