Show signature status on files app #4971
Conversation
Signed-off-by: David Lima <[email protected]>
Only show signature icon on files of pdf type and if the LibreSign certificate is ok Signed-off-by: David Lima <[email protected]>
Get file status by nodeId Signed-off-by: David Lima <[email protected]>
|
Thanks for opening your first pull request in this repository! ✌️ |
Change code style, add copyright text and fix end-point response type to pass previously failing tasks Signed-off-by: David Lima <[email protected]>
… ids Change the end-point filter parameter from nodeId to nodeIds, allowing the filtering by multiple node ids Signed-off-by: David Lima <[email protected]>
Update front end calls to /list/list end-point so it passes node ids filter as an array Signed-off-by: David Lima <[email protected]>
Signed-off-by: David Lima <[email protected]>
Change the status icon and label based on the response from the API Signed-off-by: David Lima <[email protected]>
Add the property signedNodeId to the /file/list end-point to show an icon on the original and signed file, according to the signing status Signed-off-by: David Lima <[email protected]>
| @@ -236,7 +236,7 @@ private function validate(?string $type = null, $identifier = null): DataRespons | |||
| * List account files that need to be approved | |||
| * | |||
| * @param string|null $signer_uuid Signer UUID | |||
| * @param string|null $nodeId The nodeId (also called fileId). Is the id of a file at Nextcloud | |||
| * @param list<string>|null $nodeIds The list of nodeIds (also called fileIds). It's the ids of files at Nextcloud | |||
There was a problem hiding this comment.
After implement, change or delete a controller, you should run the follow command at libresign folder inside the container to update the documentation:
composer openapiYou also need to check the other places that this endpoint is used with this argument. Tip: search at folder libresign/src by file/list. Have a place that use the argument nodeId.
There was a problem hiding this comment.
You also need to check the other places that this endpoint is used with this argument.
The only place I found it been used was on the src/store/files.js, and this is already adapted on this PR.
There was a problem hiding this comment.
The endpoit to 'file/list' is used only at src/store/files.js by the method getAllFiles and this method using the filter nodeId is used only at this file by method flushSelectedFile by this way:
const files = await this.getAllFiles({
nodeId: this.selectedNodeId,
})Changing nodeId to nodeIds and the parameter to an array will solve.
| $qb->andWhere( | ||
| $qb->expr()->eq('f.node_id', $qb->createNamedParameter($filter['nodeId'], IQueryBuilder::PARAM_INT)) | ||
| $qb->expr()->in('f.node_id', $qb->createNamedParameter($filter['nodeIds'], IQueryBuilder::PARAM_STR_ARRAY)) |
There was a problem hiding this comment.
You will need to ad OR at this point to also search by signed_node_id.
There was a problem hiding this comment.
I think in the way I implemented, this will not be necessary, since I already check at the front-end if the returned row have a signedNodeId, and if it have it, show it status.
Also, when I begin implementing, I didn't know how many places used this end-point, so I avoided change it so much.
There was a problem hiding this comment.
I think in the way I implemented, this will not be necessary, since I already check at the front-end if the returned row have a signedNodeId, and if it have it, show it status.
Check this scenario:
When go to app Files
I put a PDF file called "file.pdf" inside folder A
I send a herself signature request
I sign the file
Then I will see the file "file.signed.pdf" inside folder A
When I move the file "file.signed.pdf" to folder B
And I go to folder B
Then I need to see the LibreSign status at file "file.signed.pdf"I didn't reproduced this scenario but I think that don't will work with the current implementation.
Only an information about what's nodeId: The nodeId at Nextcloud context could be a directory or a file that was indexed and was added to table filecache to we have a good performance when fetch the content of a folder because we don't will have a big IO to list the content. This is the most common implementation. To have more performance, also is possible implement Redis and prevent a lot of hits at database. When we move a file from directory A to B, the nodeId don't will be changed, only will be changed the data of this file at the database.
Also, when I begin implementing, I didn't know how many places used this end-point, so I avoided change it so much.
Don't worry, we have unit and integration tests in the backend and they already broke 😅.
There was a problem hiding this comment.
So, when I saw this comment I really thought this flux would break, but I just tested it and it just worked in this scenario described by you and in other scenarios I've tried. Not sure though why this worked. I've thought that it was some kinda of caching that made it work anyway, but tried a different browser and it still worked.
Should I made this change even with it working this way?
Alter the Copyright text year to the file creation year Signed-off-by: David Lima <[email protected]>
Send function parameter already with the right name, so a useless if condition is avoided Signed-off-by: David Lima <[email protected]>
Signed-off-by: David Lima <[email protected]>
Add column signed_node_id to getFiesAssociatedFilesWithMeQueryBuilder method query group by Signed-off-by: David Lima <[email protected]>
Pull Request Description
This pull request adds an icon to the right of the files, in the files app, showing the file signature status on the original and the signed files. The original will have the status 'Original file signed elsewhere' and the signed file will have the status 'Partially signed' and 'Signed', depending on the signing progress.
To test the feature, after checking out to the pull request:
Related Issue
Issue Number: #4160
Pull Request Type
Pull request checklist