-
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
[SCR-164] feat: New strategy for ensureKonnectorFolder #1443
Conversation
9378fbd
to
0bc07eb
Compare
@Crash-- @paultranvan This PR is ready to review |
!isTrashedFolder(file) && | ||
hasSourceAccountIdentifierReference(file, sourceAccountIdentifier) | ||
) | ||
} |
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.
You are bypassing all the cozy-client built-in query features, which is not good 😉
First, why using collection and not cozy-client query ? e.g. client.query(Q('io.cozy.files').referencedBy({...})
This way, you will benefit from the store
Second, you query the references, and after that filter the retrieved values, while it is precisely the client.query job.
So, that would make something like this:
const resp = await client.query(Q('io.cozy.files').partialIndex({type: "directory", trashed: false}.referencedBy({...})
For the hasSourceAccountIdentifierReference
, I'm not sure I understand correctly, the file must have a io.cozy.konnectors
reference + io.cozy.accounts.sourceAccountIdentifier
? And I'm surprised about this last one, is it a new doctype?
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.
I did not know about this Q().referencedBy
, I just took the same method used to query magic folders in folder.js. Ok, let's use cozy-client query (And I will update folder.js.
And second, I'm surprised the request you propose works, since the collection findReferencedBy uses a specific stack request but I see there are helpers for this in the QueryDefinition. let's do this then, there will be a lot less code 👍.
0bc07eb
to
4db3e0c
Compare
Now konnector destination folders will have the following references: - Main konnector folder : with a reference to the associated konnector slug - konnector account folder : with a reference to the associated konnector slug + another reference to the associated sourceAccountIdentifier This way, even if the user moves/or rename the konnector folder or the konnector account folder, the konnector will find it and reuse it after konnector disconnect/reconnect. Existing konnector main folders and konnector account folders will be migrated to these references
4db3e0c
to
c7fe23a
Compare
c7fe23a
to
7c6edf6
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.
I didn't have time to completely understand this PR scope, and my main concerns looks addressed, so nothing blocking on my side
@Crash-- Let's merge this on next monday morning if I have no news. |
Now konnector destination folders will have the following references:
slug
konnector slug + another reference to the associated
sourceAccountIdentifier
This way, even if the user moves/or rename the konnector folder or the
konnector account folder, the konnector will find it and reuse it after
konnector disconnect/reconnect.
Existing konnector main folders and konnector account folders will be
migrated to these references