Update total count on record deletion

[khuddite]

Fixes #8228

1. Summary
   Total counts on record index tables are updated upon record deletion

2. Solution
   The optimistic update logic seemed a bit complicated to me, so I didn't understand everything in there. However, it seems triggerUpdateRecordOptimisticEffect should account for deletion as well as update since it's used for both. I ended up updating totalCount in Apollo cache along with nextEdges to make sure they are always in sync.
   I am not quite sure if that's the best way to handle it though. It seems to be solving the problem without impacting other parts of the app in my manual testing.

[greptile-apps]

PR Summary

This PR modifies the Apollo cache management to properly handle total record counts during deletion operations.

• Added logic in triggerUpdateRecordOptimisticEffect.ts to update totalCount based on edge differences when records are removed
• Ensures cache stays in sync by subtracting removed records from totalCount when records no longer match filters
• Handles both soft deletion (via deletedAt) and filter-based removal in a single consistent way
• Maintains cache consistency by updating counts alongside edge modifications

_{1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile}

[FelixMalfait]

I think the correct way to do this would be to
(1) find all cached queries in Apollo that contained that record
(2) decrease totalCount for each of these query

But honestly this feels a bit over-engineered and could get complex (e.g. what about the cached queries that explicitely look at the trash/deleted items, we'll need to exclude them).

So I would just advocate for doing a forced refetch of the list view query... @lucasbordeau what do you think?

[khuddite]

So I would just advocate for doing a forced refetch of the list view query... @lucasbordeau what do you think?

@FelixMalfait, does that mean we would not do optimistic updates for deletions, ignore caches, but just fetch it again from the server as if it were an initial rendering?

Also, could you help me understand any potential issues my fix would pose?

[FelixMalfait]

Hey, no we would keep the optimistic updates but still ask for a refetch for the list view. You can look for refetchQueries in the codebase to see how it's done already :)

I'm just thinking we will also introduce #8345 soon which will make it even more useful to refetch everything from the server side (we're never going to implement logic like "max of CreatedAt" on the client side, so it makes sense to let the server deal with that and not do it optimistically)

[lucasbordeau]

Solutions I'm thinking about :

• Implement a new triggerSoftDeleteOptimisticEffect() function to handle that clearly
• Create a new query only on small fragments like totalCount, max, and create utils to generate them for existing fetch queries (re-using the same parameters, even providing hooks like useTotalCountForThisQuery)

Refetching list queries would trigger too many re-renders on tables and boards, we should separate concerns even if choosing a refetch query strategy.

[FelixMalfait]

Discussed with @lucasbordeau: we should separate fetching aggregates into a separate request and refetch the aggregate query only (while still triggering the optimistic effect for rows/records)

[khuddite]

Discussed with @lucasbordeau: we should separate fetching aggregates into a separate request and refetch the aggregate query only (while still triggering the optimistic effect for rows/records)

Thanks @FelixMalfait . It might not be a smart question. But do we already have an API for fetching aggregates, or should we build a new API for that?

[lucasbordeau]

@khuddite I'm closing this PR because we need to first write a proper issue to explain what we want with @FelixMalfait I'll keep you posted for the new issue if you want to work on it.