Rich Text Versioning optimizations [MAPS-137] #10334
Conversation
| }); | ||
| return fetchedAssets.items; | ||
| } catch (error) { | ||
| console.error('Error fetching assets:', error); |
There was a problem hiding this comment.
💭 should we notify users? So they know that something went wrong. Also in the getEntries and the content type fetch?
There was a problem hiding this comment.
We're already doing that 😎 In the createOptions function if we can't identify the entry, asset or content type we just display it as unknown. This just logs the specific error in the console to be able to debug
JuliRossi
left a comment
JuliRossi
left a comment
There was a problem hiding this comment.
Looks good! Left some questions
| name: 'Fruits', | ||
| sys: { id: 'fruits' }, | ||
| fields: [{ id: 'title', name: 'Title', type: 'Symbol' }], | ||
| mockSdk.cma.contentType.getMany = vi.fn().mockResolvedValue({ |
There was a problem hiding this comment.
Consider adding a test to trigger the response too big error and see if the app handles it appropriately. Although this PR optimizes to avoid this situation with the updated querying it might also be worth thinking about how it might fail even with these optimizations
ryunsong-contentful
left a comment
ryunsong-contentful
left a comment
There was a problem hiding this comment.
Certainly an improvement and probably would fix the customer issue, but it's still possible to get a response too big error. So probably worth adding a test to see what happens when it still gets that error and maybe come up with a creative solution around the worst case still.
Purpose
A customer is having an issue in which the dialog of the app is not displaying correctly due to a "response too big" error.
Approach
We've introduced some optimizations so requests made by the app have a smaller response size:
createOptionsmethodselectparamester to only retrieve the title, which is the only thing that gets usedinclude: 0parameter when usingentry.getPublishedsince by default the response includes anincludearray that is not used