-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: Apply data enhancement to realtime results for specific doctypes #1411
Conversation
* @returns {null} The component does not display anything. | ||
*/ | ||
const RealTimeQueries = ({ doctype }) => { | ||
const RealTimeQueries = ({ doctype, customNormalization }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Maybe we should have the ability to customize the "normalization" by event (create / update / delete).
Also, I would have done that outside of the normalization
step, because here it's not a normalization anymore, it's a "business logical" before dispatching the result.
I didn't have a good naming yet, but :
customComputeDocumentFromRealtime
can be more appropriated?
In fact, we already have this computeDocumentFromRealtime
without knowing it... :
{ ...couchDBDoc, _deleted: true },
So this one cas ben the default on deleteEvent.
9c74f68
to
7b9d75a
Compare
7b9d75a
to
68a7512
Compare
@@ -0,0 +1,39 @@ | |||
import { Q } from '../queries/dsl' | |||
import CozyClient from '../CozyClient' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Prefer absolute path rather than relative 😉
const handleDeleted = data => { | ||
let options = {} | ||
if (doctype === 'io.cozy.files') { | ||
options.computeDeletedDoc = ensureFilePath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a detail, but I'm not sure it's worth naming differently each method with "delete", "create", "update"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no advantage in doing this in-house now 👍
@@ -54,9 +47,23 @@ const dispatchChange = ( | |||
* @param {object} client - CozyClient instance | |||
* @param {import("../types").Doctype} doctype - Doctype of the document to create | |||
* @param {import("../types").CouchDBDocument} couchDBDoc - Document to create | |||
* @param {object} [options] Options | |||
* @param {function} [options.computeCreatedDoc] Optional function to transform the normalized document before dispatch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
computeCreatedDoc
sounds ok, but maybe a applyAttributeComputation
would be clearer: it better suggests that it is a function, you can reuse it whether it is a create/update/delete, and it explicit mention it is used for attribute computation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point 👍 I used the term enhance to mean the improvement of an existing object. @paultranvan does it seems more clearer ?
This enrichment closes the gap between the results of queries from the cozy-stack and realtime. In fact, realtime returns the document directly from the database, unlike the stack, which can add computed fields. Without them, it can lead to instable behaviour in applications. Some applications fix some issue by reimplementing the RealTimeQueries component. This commit aims to provide unifed api to avoid duplicated code. It can also be used as a reference when we want to close the gap.
68a7512
to
7bd3a29
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good to me, thanks @cballevre !
This enrichment closes the gap between the results of queries from the cozy-stack and realtime. In fact, realtime returns the document directly from the database, unlike the stack, which can add computed fields. Without them, it can lead to instable behaviour in applications. Some applications fix some issue by reimplementing the RealTimeQueries component.
This commit aims to provide unifed api to avoid duplicated code. It can also be used as a reference when we want to close the gap.
Applications reimplementation :
Note :
A function similar to
ensureFilePath
exists in the models, but I chose to implement a new one to avoid dependencies with the models because we want to extract them from cozy-client