search

PermanentOrg / node-sdk

Node.js SDK for Permanent.org
GNU Affero General Public License v3.0
4 stars 2 forks source link

Add storage and upload #21

Closed cecilia-donnelly closed 3 years ago

cecilia-donnelly commented 3 years ago

The changes to upload.js may not be worthwhile, but my hope is that they illustrate what kind of data each piece of this flow needs. I did not actually upload a document to S3, so did not fully test the call to RegisterRecord (though I went far enough that it should work). Let me know if you want me to do that!

I think this is the last big piece. After this:

I think that's it. Let me know if this part works for you!

cecilia-donnelly commented 3 years ago

This does require a change on the backend, which will appear on this branch shortly.

cecilia-donnelly commented 3 years ago

Didn't have to change the backend because I am just passing RegisterRecord what it expects! However, in my testing I have not actually created an entry in S3 so I always get an error about "Could not look up S3 metadata." So it is possible there is an error later in the process, though this is what I would expect given there is no file in S3.

cecilia-donnelly commented 3 years ago

@jasonaowen I just rearranged this as we discussed. I've been testing by interrupting an upload in the web client, grabbing the record VO and destination url from the console, and using those in my upload script. You can see the added storage in the ledger_nonfinancial table or in the "Storage" modal in the web app itself (though that's a bit ugly at the moment).

jasonaowen commented 3 years ago

We need to better define the SimpleVO value: https://github.com/PermanentOrg/node-sdk/blob/19698f26e67813b9c6ecb36b738f8ca5f737a834/src/lib/model/simple-vo.ts#L1-L4

The get presigned URL endpoint returns an entire JSON object, and this definition doesn't allow for that. We need to add a type definition for that response. I haven't worked out what that is yet; for quick-and-dirty testing, I just replaced the union type with an any, but that will not pass our linter - and rightly so.

cecilia-donnelly commented 3 years ago

We need to better define the SimpleVO value:

https://github.com/PermanentOrg/node-sdk/blob/19698f26e67813b9c6ecb36b738f8ca5f737a834/src/lib/model/simple-vo.ts#L1-L4

The get presigned URL endpoint returns an entire JSON object, and this definition doesn't allow for that. We need to add a type definition for that response. I haven't worked out what that is yet; for quick-and-dirty testing, I just replaced the union type with an any, but that will not pass our linter - and rightly so.

I tried to do this but I'm not sure if I used the right approach or not - let me know what you think!

cecilia-donnelly commented 3 years ago

As promised, squashed some commits so the history is a bit cleaner. Thanks for all your testing and feedback on this, @jasonaowen !