-
Notifications
You must be signed in to change notification settings - Fork 2
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
Public methods to create files and folders and generate share links for files and folders #8
Conversation
…ource - Clean up files and packages leftover from starter - Install axios for HTTP requests - Install axios-mock-adapter and ts-sinon for mocking during testing - Set up params for creating Permanent SDK client - Set up ApiService and BaseRepo, create AuthRepo with `/auth/loggedIn` endpoint - Set up BaseResource for exposing public methods, create AuthResource with `isLoggedIn` method - 100% test coverage!
Rather than using the static `axios`, using instances of `axios` from `axios.create` allows us to set defaults scoped to a particular instance of a Permanent client, and allows the API service to manage the cookies and baseURL rather than delgating this to each individual Repo.
# Conflicts: # README.md # src/lib/api/api.service.ts # src/lib/api/auth.repo.spec.ts # src/lib/api/base.repo.spec.ts # src/lib/api/base.repo.ts # src/lib/client.spec.ts # src/lib/client.ts # src/lib/model/index.ts # src/lib/model/request-vo.ts # src/lib/resources/auth.resource.spec.ts # src/lib/resources/auth.resource.ts # src/lib/resources/base.resource.ts
Add API errorto PermError, Add RecordVO Set up record post API endpoint
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.
Hey @andrewatwood, it looks like due to a bad merge this branch contains the alternate history that was originally in PR #3, including the WIP commits and the commits without the author email address set. Would you please rebase onto main
to clean that up?
I believe rebasing would bring the number of lines changed down by a few hundred, but this is still an extremely large PR, and I haven't been able to meaningfully review it yet. Do you think it would be feasible for you to split it up into multiple smaller PRs for the sake of reviewability?
For the record, @andrewatwood and @slifty are working together to split this into several smaller PRs, starting with #10. Thanks, both of you! |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the new behavior (if this is a feature change)?
Creates a few new resources on the client, exposing a few new methods:
record.uploadFromUrl()
,folder.create()
,share.createRecordShareLink
,share.createFolderShareLink
, as well as creating the requiredinit()
method that sets up the session to be called after a client is created.Client config parameters have also changed, exposing
baseUrl
as an optional param to help easily test against local, and usingarchiveNbr
instead ofarchiveId
, which is more practically useful internally it turns out. And now the client will throw an exception if required params are missing!This PR got a little larger change-wise than intended due to:
FolderVO | undefined
instead of justFolderVO
. This added a lot of line changes in otherwise untouched files. But a worthy change! Just makes the diff a little larger.folder.create
andcreateFolderShareLink
methods in this PR instead of for another later issue.My demo code (minus client config) that creates a folder, uploads files to the folder, and generates a share link is as follows for reference: