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

Handle entity update with sparse attributes #1392

Merged
merged 2 commits into from
Dec 15, 2020

Conversation

Gnito
Copy link
Contributor

@Gnito Gnito commented Dec 11, 2020

I think we have forgotten to think about this merging setup when sparse attributes were added to templates.
However, I'm not entirely sure if we should make this update or not. Is there any reasonable customization concept that relies on "reduction" of marketplace data in the store?
(This PR was motivated by a recent question in Slack.)

const attributesOld = oldRes.attributes || {};
const attributesNew = newRes.attributes || {};
// Allow (potentially) sparse attributes to update only relevant fields
const attrs = attributes ? { attributes: { ...attributesOld, ...attributesNew } } : null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine. We really should always use the latest attributes we have already fetched.

This will result in part of the data being stale, while part being updated. Before we showed only fully stale or fully updated data. But I think this is perfectly ok. If the sparse updates don't make sense, e.g. in cases where extended data is synced along with other attributes, then more sparse attributes can always be added where the update happens, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kpuputti Yes. Mainly, I was thinking about the reduction of data. E.g. if a listing has 50kb worth of publicData that somehow cumulates in the long-living app. But I guess that's an edge-case scenario that needs to be addressed case-by-case in customizations. (and full page-refreshes do happen every now and then)

@Gnito Gnito force-pushed the handle-sparse-attributes-in-data-update branch from 47b46c7 to 4f9cf2d Compare December 15, 2020 10:48
@Gnito Gnito force-pushed the handle-sparse-attributes-in-data-update branch from 4f9cf2d to ef4366c Compare December 15, 2020 11:13
@Gnito Gnito merged commit bc88c06 into master Dec 15, 2020
@Gnito Gnito deleted the handle-sparse-attributes-in-data-update branch December 15, 2020 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants