✨ Tags: Mark stall data after create/edit mutation

[gildub]

Resolves https://issues.redhat.com/browse/MTA-802

[codecov]

Codecov Report

Patch coverage: 18.60% and project coverage change: -0.07 ⚠️

Comparison is base (573bce2) 44.11% compared to head (28b4237) 44.04%.

❗ Current head 28b4237 differs from pull request most recent head 83a6edb. Consider uploading reports for the commit 83a6edb to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1098      +/-   ##
==========================================
- Coverage   44.11%   44.04%   -0.07%     
==========================================
  Files         177      177              
  Lines        4477     4493      +16     
  Branches      997     1007      +10     
==========================================
+ Hits         1975     1979       +4     
- Misses       2491     2503      +12     
  Partials       11       11              

Flag	Coverage Δ
unitests	44.04% <18.60%> (-0.07%)	⬇️

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

Impacted Files	Coverage Δ
client/src/app/queries/tags.ts	20.27% <0.00%> (ø)
.../shared/components/FilterToolbar/FilterControl.tsx	37.50% <ø> (ø)
client/src/app/layout/SidebarApp/SidebarApp.tsx	26.66% <16.00%> (-2.97%)	⬇️
...d/components/FilterToolbar/SelectFilterControl.tsx	13.63% <28.57%> (+3.11%)	⬆️
...ponents/FilterToolbar/MultiselectFilterControl.tsx	10.93% <40.00%> (+2.60%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[sjd78]

Just a suggestion

[sjd78]

Since most of the mutations seem to need to invalidate both queries, why not just have a helper to make sure things are handled that same way for all?

E.g.

const useInvalidatingMutation = <T>(
  mutationFn: MutationFunction<T, T>,
  onSuccess: (res: any) => void,
  onError: (err: AxiosError) => void
) => {
  const queryClient = useQueryClient();

  return useMutation({
    mutationFn,
    onSuccess: (res) => {
      onSuccess(res);
      queryClient.invalidateQueries([TagsQueryKey]);
      queryClient.invalidateQueries([TagCategoriesQueryKey])
    },
    onError: (err: AxiosError) => {
      onError(err);
      queryClient.invalidateQueries([TagsQueryKey]);
      queryClient.invalidateQueries([TagCategoriesQueryKey])
    },
  });
}

And then this mutation can just look like:

export const useCreateTagMutation = (
  onSuccess: (res: any) => void,
  onError: (err: AxiosError) => void
) => {
  return useInvalidatingMutation(createTag, onSuccess, onError);
};

[mturley]

I would say we should perhaps include an additional mutationOptions object that is a Partial<UseMutationOptions> and gets spread into the inner useMutation, or perhaps just have the arguments for useInvalidatingMutation be themselves the same UseMutationOptions object, but the onSuccess and onError just get overridden? TypeScript generics might get weird I suppose.

Also, we may want to consider using compound query key arrays that contain some common element we can pass to invalidate any query that has to do with tags. The queryKey can be an array with any number of elements to identify the query, and if any of those elements is passed into invalidateQueries it will invalidate all matching queries. https://tanstack.com/query/v4/docs/react/guides/query-keys

That would at least mean we can do a single queryClient.invalidateQueries([TagsQueryKey]) if we just include TagsQueryKey along with TagCategoriesQueryKey in the tag categories query hook's key. It still doesn't prevent all the duplication @sjd78's suggestion would address though, so maybe we can do both.

[gildub]

Thanks @sjd78 and @mturley , for the review.

I've pushed an update using compound query key which simplifies and therefore it doesn't need to use an helper.
Effectively on the tags page, the tags are depending upon the tagCategories since we're displaying the tags for each tag category they belong to, therefore when a tag is touched (Created/updated/deleted) then we want to invalidate the same query key we use for fetching the tagCategories.

[sjd78]

See inline comments

[sjd78]

I debugged through the react-query code -- this only invalidates the query that matches both keys.

Effectively, this change invalidates the useFetchTagCategories query now instead of the useFetchTags query.

[sjd78]

AFAICT, the only way to invalidate 2 queries is to either:

1. call invalidate twice with exactly matching queryKey, or
2. setup multiple values in the initial queryKey and use one of those across all queries you want to invalidate at the same time.

For example:

export const useFetchTags = () => {
  const { data, isLoading, error, refetch } = useQuery({
    queryKey: ["allTags", TagsQueryKey],
    queryFn: getTags,
    onError: (error: AxiosError) => console.log("error, ", error),
  });
  return {
    tags: data || [],
    isFetching: isLoading,
    fetchError: error,
    refetch,
  };
};

export const useFetchTagCategories = () => {
  const { data, isLoading, error, refetch } = useQuery({
    queryKey: ["allTags", TagCategoriesQueryKey],
    queryFn: getTagCategories,
    onError: (error: AxiosError) => console.log("error, ", error),
  });
  return {
    tagCategories: data || [],
    isFetching: isLoading,
    fetchError: error,
    refetch,
  };
};

export const useCreateTagMutation = (
  onSuccess: (res: any) => void,
  onError: (err: AxiosError) => void
) => {
  const queryClient = useQueryClient();

  return useMutation({
    mutationFn: createTag,
    onSuccess: (res) => {
      onSuccess(res);
      queryClient.invalidateQueries(["allTags"]);
    },
    onError: (err: AxiosError) => {
      onError(err);
      queryClient.invalidateQueries(["allTags"]);
    },
  });
};

The extra "allTags" shouldn't do anything to the query itself other than make it easier to partial match on the invalidateQueries() call

[mturley]

Yeah, option 2 here is what I meant above.

[sjd78]

With invalidation only needed on the tag category query, this change LGTM.