Skip to content
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

Added partial request printing functions #19

Merged
merged 17 commits into from
Oct 28, 2017
Merged

Added partial request printing functions #19

merged 17 commits into from
Oct 28, 2017

Conversation

foxfriends
Copy link
Contributor

@foxfriends foxfriends commented Oct 27, 2017

Was having trouble sending GET requests with this properly as the only "query only" string was pretty printed, so I added some functions to break up that one for writing POST requests. Changed the output format a bit too, so it would be a bit more readable/standard looking. Seems to be working from my app, and fixed up the tests that had to change for this.

Not sure why there are so many commits listed. Seems like a lot of the ones from the last PR.

@codecov-io
Copy link

codecov-io commented Oct 27, 2017

Codecov Report

Merging #19 into master will decrease coverage by 3.19%.
The diff coverage is 74.19%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master      #19     +/-   ##
===========================================
- Coverage     85.21%   82.01%   -3.2%     
- Complexity       47       55      +8     
===========================================
  Files            14       15      +1     
  Lines           142      139      -3     
  Branches         28       25      -3     
===========================================
- Hits            121      114      -7     
- Misses            7       10      +3     
- Partials         14       15      +1
Impacted Files Coverage Δ Complexity Δ
core/src/main/kotlin/me/lazmaid/kraph/Kraph.kt 84.31% <0%> (-5.27%) 10 <4> (+2)
...in/me/lazmaid/kraph/lang/relay/CursorConnection.kt 100% <100%> (ø) 1 <1> (ø) ⬇️
.../src/main/kotlin/me/lazmaid/kraph/lang/Document.kt 100% <100%> (ø) 3 <2> (ø) ⬇️
...otlin/me/lazmaid/kraph/lang/relay/InputArgument.kt 100% <100%> (ø) 2 <1> (?)
.../main/kotlin/me/lazmaid/kraph/lang/SelectionSet.kt 100% <100%> (ø) 3 <2> (-1) ⬇️
...src/main/kotlin/me/lazmaid/kraph/lang/DataEntry.kt 100% <100%> (ø) 1 <0> (ø) ⬇️
.../src/main/kotlin/me/lazmaid/kraph/lang/Argument.kt 100% <100%> (ø) 2 <1> (ø) ⬇️
...src/main/kotlin/me/lazmaid/kraph/lang/Variables.kt 100% <100%> (ø) 2 <2> (?)
...src/main/kotlin/me/lazmaid/kraph/lang/Operation.kt 60% <55.55%> (+10%) 7 <7> (+3) ⬆️
...c/main/kotlin/me/lazmaid/kraph/lang/GraphQLNode.kt 70.83% <66.66%> (-8.58%) 16 <5> (-2)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58374f6...f1e4dd0. Read the comment docs.

@VerachadW
Copy link
Owner

IMHO, I think it would be better to create an enum to represent print mode instead of using two combinations of 2 boolean flags. For example,

  enum class PrintMode {
    NORMAL,
    ONELINE,
    PRETTY;
  }

and in requestQueryString() would be

   fun requestQueryString() = document.operation.print(PrintMode.ONELINE, 0)

Would it be better?

@foxfriends
Copy link
Contributor Author

Yeah that sounds like a better idea. I'll figure out a way to make it work. Maybe NORMAL, PRETTY and JSON?

@foxfriends
Copy link
Contributor Author

Alright it is changed. Rewrote the tests a lot too, so that there would be less stuff that gets duplicated. Doubt there would be another print format but if there is it should be easier to write tests for now.

README.md Outdated
which are sometimes necessary depending on your HTTP request builder and
GraphQL server setup. They provide the values for the `query`, `variables`,
and `operationName` parameters, respectively, and so are good for creating
GET requests.
Copy link
Owner

Choose a reason for hiding this comment

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

Please add a note here that requestVariableString() always return null for now. We will implement this after we support the variable.

Copy link
Owner

@VerachadW VerachadW left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks again for your help

@VerachadW VerachadW merged commit 99c00f3 into VerachadW:master Oct 28, 2017
VerachadW pushed a commit that referenced this pull request Nov 22, 2018
* Fix escaped string array (#6)

* Update Spek version

* Fix print wrong escaped character, Fix #4

* Rewrite the GraphQLPrintSpek for preetty print test

* Bump hotfix version

* Update build status link

* Migrate kotlin v1.1.1 (#7)

* bump to kotlin v1.1.1

* Add type alias for more readable code

* Change the package name to me.lazmaid.kraph

* Fix build failed dur to hamkrest error

* Update spek version

* Update ENV for release

* Update README

* Refactor arguments (#9)

* WIP

* Split the print logic into sealed class

* Update UTs

* Add exception message

* Refactor

* Add Test Coverage (#11)

* Add Test Coverage

* Update spek version

* Fix test result path

* Add codecov YAML

* Add onlyIf

* Update config

* Fix travis command

* Add coverage badge

* Fix wrong download badge

* Bump version to 0.5.1

* Support boolean type in Argument (#15)

* Add support for boolean type

* Add Tests

* Fix typo in README (#16)

missing "h" in the example call to `println(query.toGrapQueryString())`

* Added basic fragment support (#17)

* improve readme

* Allow using field with a block in place of field object

* Update readme to include new changes

* Cheap implementation of fragment that doesn't use actual GraphQL Fragments

* Add fragment docs to readme

* Fix tests

* Change function to 'defineFragment'

* Update Kotlin and library dependcies (#18)

* Update .gitignore

* Update dependecies

* Add secondary constructor for DataEntry.NonDecimalNUmber

* Add Test for DataEntry classes

* Fix failed UTs

* Added partial request printing functions (#19)

* improve readme

* Allow using field with a block in place of field object

* Update readme to include new changes

* Cheap implementation of fragment that doesn't use actual GraphQL Fragments

* Add fragment docs to readme

* Fix tests

* Add separate methods to print the different parts of the request separately

* Use spaces instead of newlines for request format

* Rewrite printing operations to optionally escape quotes

* update readme a bit

* Change function to 'defineFragment'

* Use an enum to choose print format and write tests to cover all formats

* Add note about variables

* Update version (#20)

* add support for variables (#23)

* add support for variables

* add support for variables

* Added enums support

* add some test for enums support

* add test for GraphQLPrintSpek
VerachadW pushed a commit that referenced this pull request Oct 5, 2019
* Fix escaped string array (#6)

* Update Spek version

* Fix print wrong escaped character, Fix #4

* Rewrite the GraphQLPrintSpek for preetty print test

* Bump hotfix version

* Update build status link

* Migrate kotlin v1.1.1 (#7)

* bump to kotlin v1.1.1

* Add type alias for more readable code

* Change the package name to me.lazmaid.kraph

* Fix build failed dur to hamkrest error

* Update spek version

* Update ENV for release

* Update README

* Refactor arguments (#9)

* WIP

* Split the print logic into sealed class

* Update UTs

* Add exception message

* Refactor

* Add Test Coverage (#11)

* Add Test Coverage

* Update spek version

* Fix test result path

* Add codecov YAML

* Add onlyIf

* Update config

* Fix travis command

* Add coverage badge

* Fix wrong download badge

* Bump version to 0.5.1

* Support boolean type in Argument (#15)

* Add support for boolean type

* Add Tests

* Fix typo in README (#16)

missing "h" in the example call to `println(query.toGrapQueryString())`

* Added basic fragment support (#17)

* improve readme

* Allow using field with a block in place of field object

* Update readme to include new changes

* Cheap implementation of fragment that doesn't use actual GraphQL Fragments

* Add fragment docs to readme

* Fix tests

* Change function to 'defineFragment'

* Update Kotlin and library dependcies (#18)

* Update .gitignore

* Update dependecies

* Add secondary constructor for DataEntry.NonDecimalNUmber

* Add Test for DataEntry classes

* Fix failed UTs

* Added partial request printing functions (#19)

* improve readme

* Allow using field with a block in place of field object

* Update readme to include new changes

* Cheap implementation of fragment that doesn't use actual GraphQL Fragments

* Add fragment docs to readme

* Fix tests

* Add separate methods to print the different parts of the request separately

* Use spaces instead of newlines for request format

* Rewrite printing operations to optionally escape quotes

* update readme a bit

* Change function to 'defineFragment'

* Use an enum to choose print format and write tests to cover all formats

* Add note about variables

* Update version (#20)

* add support for variables (#23)

* add support for variables

* add support for variables

* Added alias support for queries and mutations

* Checked errors from CI Build and added an example for usage

* Resolve error with arguments in RequestSpek.kt

* Resolve error in BuilderSpek.kt

* Replace /n on """ in the BuilderSpek.kt

* Change equalTo to requestString

* Change other test mutation with alias

* Added documentation about alias for field and fieldObject

* Resolved conflicts

* Check CI error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants