-
Notifications
You must be signed in to change notification settings - Fork 56
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
[ENG-4681] Add mirage for v2 api #2051
[ENG-4681] Add mirage for v2 api #2051
Conversation
mirage/views/external-account.ts
Outdated
.models; | ||
const externalAccounts = userAddons.length ? userAddons[0].externalAccounts.models : null; | ||
if (externalAccounts) { | ||
externalAccounts.map((model: ModelInstance<ExternalAccountsModel>) => this.serialize(model).data); |
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 think this needs an additional filter on it to filter by provider. I was so happy I got it working that I forgot that step.
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.
Just a couple "nice-to-have" things. Not sure if they will be relevant, so no need to implement them now if we want this merged soon
const nodeAddons = schema.nodes | ||
.find(request.params.parentID)['nodeAddons'] | ||
.models | ||
.filter((m: ModelInstance<NodeAddonModel>) => filter(m, request)) |
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.
Forgot we have this filter
util 👀
f8b33f1
into
CenterForOpenScience:feature/addon-services
Purpose
Make the v2 endpoints work with mirage. This includes a lot of normalization of the API but not the extra features such as extended attributes for providers nor getting the folder lists.
Summary of Changes
Screenshot(s)