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

ui: Use X-Range header as a signal as to whether to reconcile the ember-data store #8384

Merged
merged 2 commits into from
Jul 29, 2020

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Jul 27, 2020

Previously we used a shouldReconcile method in order to decide whether a response should trigger a reconciliation of the frontend ember-data 'source of truth' or not.

It's a lot nicer/clearer if this 'flag' can be set alongside the HTTP request information, moreover we almost have the same functionality in If-Range/Partial Content HTTP functionality.

Here we partly follow these HTTP semantics but use a custom X-Range header instead.

Additional note here, we are using 'header to meta' approach more and more for various little details, it would be nice to formalise this approach a little more. In fact, the code surrounding it is from the very beginnings of the Consul UI previous to the adapter cleanup we did in #5637 (which actually mentions a desire to do a similar cleanup of the serializer layer)

This PR is an alternative/replacement to #8334

John Cowen added 2 commits July 27, 2020 09:43
Previously we used a `shouldReconcile` method in order to decide whether
a response should trigger a reconciliation of the frontend ember-data
'source of truth' or not. It's a lot nicer/clearer if this 'flag' can be set
alongside the HTTP request information, moreover we almost have the same
functionality in `If-Range`/`Partial Content` HTTP functionality.

Here we partly follow this HTTP semantics but use a custom `X-Range` header
instead.
This header value controls whether ember-data should reconcile its local
store/data or not.
@johncowen johncowen added theme/ui Anything related to the UI backport/1.8 labels Jul 27, 2020
@johncowen johncowen requested a review from kaxcode July 27, 2020 10:06
? `
X-Range: ${filter}`
: ``
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ouch just found out how prettier has 'prettified' our nice one-liner conditional, it looked nicer than this beforehand honest! Joking aside, I think this is fine, we could add some prettier ignores in here but I don't think its worth it.

Copy link
Contributor

@kaxcode kaxcode left a comment

Choose a reason for hiding this comment

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

LGTM ♻️ 🔥

@johncowen johncowen merged commit 77b4d8f into master Jul 29, 2020
@johncowen johncowen deleted the ui/feature/x-range-reconcile branch July 29, 2020 08:16
@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit 77b4d8f onto release/1.8.x succeeded!

hashicorp-ci pushed a commit that referenced this pull request Jul 29, 2020
…er-data store (#8384)

* ui: Use `X-Range` header/meta to decide whether to reconcile or not

Previously we used a `shouldReconcile` method in order to decide whether
a response should trigger a reconciliation of the frontend ember-data
'source of truth' or not. It's a lot nicer/clearer if this 'flag' can be set
alongside the HTTP request information, moreover we almost have the same
functionality in `If-Range`/`Partial Content` HTTP functionality.

Here we partly follow this HTTP semantics but use a custom `X-Range` header
instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants