-
Notifications
You must be signed in to change notification settings - Fork 1
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: Use ensureKonnectorFolder from cozy-client #964
Conversation
571f8e6
to
8855c14
Compare
src/libs/Launcher.js
Outdated
konnector, | ||
account | ||
const folder = await models.konnectorFolder.ensureKonnectorFolder(client, { | ||
konnector: { ...konnector, _id: konnector._id }, |
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.
Why do you spread?
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.
_id attribute is missing in konnector object, which causes the reference to the konnector in the destination folder to be null
I think there is something to fix in harvest when the launcher context is sent
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 don't understand:
You do:
konnector: { ...konnector, _id: konnector._id }
and you say : "_id attribute is missing in konnector object,". But you do: _id: konnector._id
so _id is present in the konnector object. If this is not the case, you'll have _id: undefined
is it really wanted?
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.
Sorry, I miss typed. The right code is konnector: { ...konnector, _id: konnector.id }
and I fixed it now.
src/libs/Launcher.js
Outdated
@@ -289,6 +289,27 @@ export default class Launcher { | |||
trigger = triggerResponse.data | |||
result.createdTrigger = trigger | |||
log.debug(`ensureAccountAndTriggerAndJob: created trigger`, trigger) | |||
// @ts-ignore |
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.
can you add a comment to explain why you need to ignore 🙏 (if you can't fix it right now)
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.
Done, the message attribute of IOCozyTrigger is kind of a free object. Its content depends on the kind of trigger. But all the konnector trigger (client or node) which have a destination folder will have the .message.folder_to_save
attribute.
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.
So let's update the IOCozyTrigger type on Cozy-client 🙏
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.
Here it is cozy/cozy-client#1398
3db6ae7
to
b096cfb
Compare
@Crash-- Should be updated now |
And also update the destination folder id saved in the trigger when needed. Upgrade cozy-client to 41.4.1 to be able to update the trigger and to have ensureKonnectorFolder function.
b096cfb
to
3051efb
Compare
Upgraded cozy-client with the updated triggers types and removed some @ts-ignore |
And also update the destination folder id saved in the trigger when
needed.
Upgrade cozy-client to 41.4.1 to be able to update the trigger and to
have ensureKonnectorFolder function.
Checklist
Before merging this PR, the following things must have been done: