-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat(api): add /import endpoint to support bulk importing json data #390
Conversation
Thanks @vnugent for the feedback on Discord about discussing the pros and cons of using the existing GraphQL API vs. creating a new REST endpoint first. First off: you are absolutely right to discuss the pros and cons before going forward with such a change. You built such an amazing product and we are very happy to hear your thoughts and feedback about the change we are about to implement. REST vs GraphQL for Bulk-Import of Data[Draft] Example
|
d1a297c
to
8a76076
Compare
@Silthus I'd be more comfortable if we still use GQL as the interface (not introducing REST in this PR). You can create a new GQL bulk mutation and reuse the all DB operations in import-json.ts. There's no need to call existing update APIs. |
OK, I can understand why you are more comfortable with sticking to the GraphQL only API. But I am not quite sure what you mean. Can you share a small example snippet or link to some docs? I just need to direction to head towards and will change the bulk import accordingly. |
You can piggyback Area GQL. I'm using
|
f7a472c
to
7ca922c
Compare
@vnugent thanks that helped a lot! I adjusted the code accordingly, the only thing left to do are some failing tests where the input doesn't match up. Let me know what you think of the changes. I intentionally left the exchange of the apollo-server-express package in there to decouple the tests from a real running server listening on a port on localhost. This significantly speeds up testing and reduces complexity without any disadvantages. |
7ca922c
to
6addf8c
Compare
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 have a small request to check both nullness and defined. Other than that we're good to merge.
Thanks I learned something again today :) |
520265f
to
aeb6e49
Compare
Hey Openbeta Team :)
As discussed in OpenBeta/open-tacos#1031 it would be great to have a simple page where users can bulk import data using a simple, validated json schema.
This PR adds the required functionality to the backend, enabling the integration of a bulk import in the frontend. The frontend is also actively being worked on by @l4u532.
What this PR does:
apollo-server
toapollo-server-express
allowing the integration of additional http endpoints without changing the graphql api. You can read more about the swapping of the packages here: https://www.apollographql.com/docs/apollo-server/v3/integrations/middleware/#swapping-out-apollo-server/import
for authorized users, allowing the import of a defined json schema. The schema will be defined in the PR for the frontend.Let me know what you think. I will happily make any adjustments to the PR.