Fixes person navigation after image upload

[mdrazak2001]

Fixes #8949

[greptile-apps]

PR Summary

This PR modifies the record navigation logic to fix pagination issues that occurred after uploading profile images in the People section.

• Added id: { neq: objectRecordId } filter in /packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts to exclude current record from before/after queries
• Modified cursor-based pagination to properly handle record navigation after image uploads
• Fixed edge case where navigation buttons were unresponsive after profile image changes

_{💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!}

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

[charlesBochet]

Hi @mdrazak2001, thanks for opening a PR :)
Your fix does not feel right though, I've left a comment

[charlesBochet]

this does not feels right to me, we need to retrieve the previous (resp. next) record based on the viewFilters and sort. We cannot remove the filter as you are doing.

Could you explain your change and how you think it's fixing the issue?

[mdrazak2001]

Hi @charlesBochet,

Thanks for the feedback! You're right—replacing filter entirely was incorrect. I've updated the code to combine the original filter with { id: { neq: objectRecordId } } to ensure we respect view filters and sorting while excluding the current record (to prevent duplicate pagination results).

Let me know if this works!

Thanks!

[charlesBochet]

I'm a bit confused because to me this should not be tied to filters.
It should be tied to cursor / pagination logic. To get the next record for example, we do:

cursorFilter: cursorFromRequest
        ? {
            cursorDirection: 'after',
            cursor: cursorFromRequest,
            limit: 1,
          }
        : undefined,

This should already get the next item without having to change the filters. Why isn't this working?

[mdrazak2001]

I did some logging to understand the behavior further. Here’s what I observed:

1. Before the image upload (Added Dummy ids for the below example):

•     Records Before: []
  

•     Records After (contains the next person information):
        [
          {
            "__typename": "Person",
            "id": "B"
          }
        ]
  

2. After the image upload:

•     Records Before:
         [
            {
              "__typename": "Person",
              "id": "A"
            }
          ]
  

•     Records After :
         [
            {
              "__typename": "Person",
              "id": "A"
            },
            {
              "__typename": "Person",
              "id": "B"
            }
          ]
  

These responses were returned by useFindManyRecords. It seems that after the image upload, the Records After array includes both the current record and the next record. As a result, when the "Next" button is pressed, it navigates to the current person instead of the expected next person.
This behavior suggests that while the cursor logic retrieves records after the current position, the filtering isn't excluding the current record
Explicitly adding a filter like { id: { neq: currentRecordId } } ensures the current record is excluded, preventing this duplication and aligning the navigation behavior with expectations.

[charlesBochet]

Okay, I think this is an optimistic rendering issue. What's likely happening is that on record update (useUpdateOneRecord) we are triggering optimistic effects triggerUpdateRecordOptimisticEffect.

This Optimistic effects (through cache.modify) is checking if the updated record match the query and if yes it adds it.

  const updatedRecordShouldBeAddedToRootQueryEdges =
          updatedRecordMatchesThisRootQueryFilter &&
          !updatedRecordFoundInRootQueryEdges;

However this logic does not take the cursor into account : it's not because it matches the filter that the record should be added to the query result, it should also check that if we have orderBy + cursor + limit, it should be added.

This is not a big change but might be a bit tricky to implement.

Now I understand your change and even if not completely right it might solve the issue for now

[charlesBochet]

And I think it works

[mdrazak2001]

Thank you @charlesBochet for the detailed explanation, am able to connect the dots now!

[charlesBochet]

Thank you @mdrazak2001, interesting one

[github-actions]

Thanks @mdrazak2001 for your contribution!
This marks your 2nd PR on the repo. You're top 20% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter