-
Notifications
You must be signed in to change notification settings - Fork 25
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
WIP feat: add saveBankingDocuments function to cozy-konnector-libs #214
Conversation
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.
A few concerns on my side, not sure any of them is actually a blocker, waiting for feedback from others.
* | ||
* Parameters: | ||
* | ||
* - `documents` is an array of objects with any attributes : | ||
* - `fields` (object) this is the first parameter given to BaseKonnector's constructor | ||
* - `options` is passed directly to `saveFiles`, `hydrateAndFilter`, `addData` and `linkBankOperations`. | ||
* - `doctype` option has `io.cozy.bills` | ||
* - `noBanking` option deactivates linkBankOperations. Default value is false. |
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.
Since we may want to prevent linking with bank operations, doesn't this mean there will be other possible uses of these money-related documents? Then what about saveFinancialDocuments()
as a name (with financial as in money-related)?
* Will create `io.cozy.bills` documents by default or any specified doctype. | ||
* The default deduplication keys are `['date', 'amount', 'vendor']`. | ||
* You need the full permission on the doctype, full permission on `io.cozy.files` and also | ||
* full permission on `io.cozy.bank.operations` in your manifest, to be able to use this function. |
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.
... to be able to use this function (unless option noBanking: true
is set, see below).?
Or will the permission still be needed anyway?
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.
That is right, I added the noBanking option later.
Then the name of the function should be saveDocuments since we may want to use it without banking, and since the banking part may be extracted from the connector anyway.
* return saveBills(documents, fields, { | ||
* identifiers: ['vendorj'] | ||
* return saveBankingDocuments(documents, fields, { | ||
* identifiers: ['vendorj'], |
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.
s/vendorj/vendor/
? Or does it have some special meaning?
// Deduplicate on this keys | ||
options.keys = options.keys || ['date', 'amount', 'vendor'] | ||
|
||
options.postProcess = function (entry) { | ||
if (entry.fileDocument) { | ||
entry.invoice = `io.cozy.files:${entry.fileDocument._id}` | ||
entry.$relationships = entry.$relationships || {} | ||
entry.$relationships.document = { |
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.
Is $relationships
a cheerio object? Then couldn't it happen that such a common name as document
would make its way into cheerio's API?
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.
$relationships
is not a cheerio object. It is a cozy-special fields for relationships (see cozy/cozy-doctypes#38)
9733b45
to
1505349
Compare
The goal of this PR is to replace saveBills with saveBankingDocuments which will be able to handle more doctypes than io.cozy.bills
and also handles link between documents and bank operations using relationships
Then this also fixes #79
The doctype stays
io.cozy.bills
by default and an option has been added to specify the doctype :At the moment :
These doctype have some attributes in common :
These attributes will make it easier (hopefully) for the cozy-bank application to link theses
documents to bank operations.
The cozy bank application will have to do some adaptions to be able to display the documents created with this function