Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix optimistic cache on records import #9927

Closed
prastoin opened this issue Jan 30, 2025 · 0 comments · Fixed by #10130
Closed

Fix optimistic cache on records import #9927

prastoin opened this issue Jan 30, 2025 · 0 comments · Fixed by #10130
Assignees

Comments

@prastoin
Copy link
Contributor

prastoin commented Jan 30, 2025

Bug Description

After importing records on Object Model that has composite field the optimistic cache update will be aborted because some expected Apollo expected field are missing.

It does not have an impact on the final operation for the back. Also the UI seems to be working fine because we mutate the cache on response. But we do not get optimistic cache plus value in that flow.

Screen.Recording.2025-01-30.at.11.53.05.mov

Expected behavior

We should provide a value for each expected fields.

Technical inputs

That might highly be due to the prefillRecord method replacing instead of merging with override input on defaultValues.
We might have to check metadata to determine if current value could be anIterable or not in order to spread and merge defaultValues and Input
To dig deeper

@charlesBochet charlesBochet moved this to 🔖 Planned in 🎯 Roadmap & Sprints Jan 30, 2025
@prastoin prastoin moved this from 🔖 Planned to 👀 In review in 🎯 Roadmap & Sprints Feb 10, 2025
@prastoin prastoin moved this from 👀 In review to 🏗 In Progress in 🎯 Roadmap & Sprints Feb 10, 2025
charlesBochet pushed a commit that referenced this issue Feb 11, 2025
…& `DELETE` (#10079)

# Introduction
At the moment when updating any record cache occurence, we will build a
fragment that will expect all of the object metadata item fields to be
provided.
Which result in the following traces: ( in the video companies aren't
fetch with companyId and other missing fields )


https://github.com/user-attachments/assets/56eab7c1-8f01-45ff-8f5d-78737b788b92

By definition as we're using graphql we might not request every record's
fields each time we wanna consume them.
In this way we will now dynamically compute or expect depending on the
CRUD operation specific fields to be written in the cache, and not all
of them

Tested all optimistic and failure management use cases

## Covering cache
Added coverage only for the `deleteOne` and `deleteMany` hooks, it cover
only the record record cache and not its relations hydratation ( for the
moment )

## Why not closing #9927 
Unless I'm mistaken everything done here have fixed the same logs/traces
issue for updates and deletion but not creation.
Which means we still need to investigate the mass upload from import and
prefillRecord behavior

In a nutshell: went over each `updateRecordFromCache` calls, still need
to do all `createRecordInCache` calls

related to #9927 

## Conlusion
Sorry for the big PR should have ejected into a specific one for the
`MinimalRecord` refactor
Will also continue covering others hooks later in my week as for the
`deleteOne`
As always any suggestions are welcomed !
prastoin added a commit that referenced this issue Feb 13, 2025
…10130)

# Introduction
While importing records encountering missing expected fields when
writting a fragment from apollo cache

## Updates

### 1/ `createdBy` Default value
When inserting in cache in create single or many we will now make
optimistic behavior on the createdBy value

### 2/ `createRecordInCache` dynamically create `recordGrqlFields`
When creating an entry in cache, we will now dynamically generate fields
to be written in the fragment instead of expecting all of them. As by
nature record could be partial

### 3/ Strictly typed `RecordGqlFields`

# Conclusion
closes #9927
@github-project-automation github-project-automation bot moved this from 🏗 In Progress to ✅ Done in 🎯 Roadmap & Sprints Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants