Skip to content

File upload concept - Taken from #116#626

Merged
designatednerd merged 7 commits intoapollographql:masterfrom
kimdv:kimdv/graphql-file-upload
Jul 18, 2019
Merged

File upload concept - Taken from #116#626
designatednerd merged 7 commits intoapollographql:masterfrom
kimdv:kimdv/graphql-file-upload

Conversation

@kimdv
Copy link
Copy Markdown
Contributor

@kimdv kimdv commented Jul 11, 2019

This code is taken from #116

@kimdv
Copy link
Copy Markdown
Contributor Author

kimdv commented Jul 13, 2019

@designatednerd I have started the PR.
Tests are passing.

Now I need to figure out how to test the upload part.
Can you guide me here?

@designatednerd designatednerd mentioned this pull request Jul 15, 2019
Closed
designatednerd
designatednerd reviewed Jul 15, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@designatednerd designatednerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few preliminary comments.

For testing, something that would be really helpful is to make sure that output from MultipartFormData matches what's expected in the GraphQL File Upload Spec.

Comment thread Sources/Apollo/GraphQLFile.swift
Comment thread Sources/Apollo/HTTPNetworkTransport.swift Outdated
Comment thread Sources/Apollo/HTTPNetworkTransport.swift Outdated
Comment thread Sources/Apollo/MultipartFormData.swift Outdated
Comment thread Sources/Apollo/NetworkTransport.swift Outdated
Comment thread Sources/Apollo/GraphQLFile.swift Outdated
@kimdv
Copy link
Copy Markdown
Contributor Author

kimdv commented Jul 15, 2019

@designatednerd I have resolved the comments in the code!
I reached out on Spectrum with some questions 🚀

@designatednerd designatednerd mentioned this pull request Jul 15, 2019
Closed
designatednerd
designatednerd reviewed Jul 15, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@designatednerd designatednerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Looking forward to the tests!

Comment thread Sources/Apollo/HTTPNetworkTransport.swift Outdated
Comment thread Sources/Apollo/MultipartFormData.swift Outdated
kimdv
kimdv commented Jul 16, 2019
View reviewed changes
Comment thread Sources/Apollo/MultipartFormData.swift Outdated
@kimdv kimdv marked this pull request as ready for review July 16, 2019 12:26
@kimdv
Copy link
Copy Markdown
Contributor Author

kimdv commented Jul 16, 2019

@designatednerd rebased and made it ready for review 🚀

@designatednerd designatednerd mentioned this pull request Jul 16, 2019
Closed
designatednerd
designatednerd approved these changes Jul 17, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@designatednerd designatednerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any comments I have are anal-retentive formatting nonsense I can fix myself after merging. Thank you so much for this! Gonna leave it open until tomorrow at 12:00 CEST (11:00 GMT) for any further comments or objections, and then I'll merge!

@kimdv
Copy link
Copy Markdown
Contributor Author

kimdv commented Jul 17, 2019

Thanks a lot! 🚀

@designatednerd
Copy link
Copy Markdown
Contributor

designatednerd commented Jul 18, 2019

Time's up, let's ship this!

@designatednerd designatednerd merged commit 1ff23e3 into apollographql:master Jul 18, 2019
@designatednerd designatednerd added this to the 0.13.0 milestone Jul 18, 2019
@kimdv kimdv deleted the kimdv/graphql-file-upload branch July 18, 2019 11:46
@designatednerd designatednerd mentioned this pull request Jul 18, 2019
Merged
@Drusy
Copy link
Copy Markdown

Drusy commented Jul 30, 2019

Hello @designatednerd, could you provide a concret example how to use file upload ?
I can't find any official documentation on this topic.
How the .graphql file should look ?
Where GraphQLFile instances should be passed ?

Thanks in advance

@designatednerd
Copy link
Copy Markdown
Contributor

designatednerd commented Jul 30, 2019

@Drusy Mind opening the lack of documentation as a new issue? I'll try to get that going later this week. The short answer is that you should use the upload function in the HTTPNetworkTransport.

@Drusy
Copy link
Copy Markdown

Drusy commented Jul 30, 2019

Linked to #681 (File upload v0.13 - Missing documentation )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@designatednerd designatednerd designatednerd approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.13.0

Development

Successfully merging this pull request may close these issues.

3 participants

@kimdv @designatednerd @Drusy
⚡