#17 - Add derived query for ISBN - Githubissues

Project-Books / books-api

GraphQL Books API

https://project-books.github.io/#books-api

MIT License

35 stars 61 forks source link

#17 - Add derived query for ISBN #29

Closed ahsanbagwan closed 3 years ago

ahsanbagwan commented 3 years ago

Summary of change

Added derived query to fetch by ISBN13
Added test data
Added JUnit test

Related issue

Closes #17

Pull request checklist

Please keep this checklist in & ensure you have done the following:

[x] Read, understood and adhered to our contributing document.
- [x] Ensure that you were first assigned to a relevant issue before creating this pull request
- [x] Ensure code changes pass all tests
[x] Read, understood and adhered to our style guide. A lot of our code reviews are spent on ensuring compliance with our style guide, so it would save a lot of time if this was adhered to from the outset.
[x] Filled in the summary, context (if applicable) and related issue section. Replace the square brackets and its placeholder content with your contents. For an example, see any merged in pull request
[x] Created a branch that has a descriptive name (what your branch is for in a few words and includes the issue number at the end, e.g. test-reading-goal-123
[x] Set this pull request to 'draft' if you are still working on it
[x] Resolved any merge conflicts

For any of the optional checkboxes (e.g. the screenshots one), still check it if it does not apply.

If in doubt, get in touch with us via our Slack workspace

knjk04 commented 3 years ago

@ahsanbagwan Thanks for working on this. As per our contributing guidelines, we only accept PRs where you have first been assigned to the issue. Could you please request to be assigned to the issue? Also, I've added the PR checklist back in -- could you please fill it in?

knjk04 commented 3 years ago

Thanks for making the changes. Could you also please add to the query.graphqls file so that we can access it from the playground?

knjk04 commented 3 years ago

Thanks for making the change.

Does this work for you? This is the stack trace I get when I run this: https://gist.github.com/knjk04/d5fa917e9da9bd27d302e294fdaf570c

knjk04 commented 3 years ago

@ahsanbagwan Just following up. Did you manage to get a chance to see the message above?

ahsanbagwan commented 3 years ago

Oops missed this, will check it out tomorrow.

On Mon, 28 Dec, 2020, 9:57 pm Karan Kumar, notifications@github.com wrote:

@ahsanbagwan https://github.com/ahsanbagwan Just following up. Did you manage to get a chance to see the message above?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Project-Books/books-api/pull/29#issuecomment-751774400, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6M5PYGMDRINLMLLUOTVKDSXCWYFANCNFSM4UQVPB7A .

knjk04 commented 3 years ago

Let us know when you're ready for another review

ahsanbagwan commented 3 years ago

I'm no longer getting the graphql schema error on running the application...

Snip 2020-12-29 23:31:29.985 INFO 27712 --- [ restartedMain] o.s.b.w.embedded.tomcat.TomcatWebServer : Tomcat started on port(s): 8080 (http) with context path '' 2020-12-29 23:31:29.987 INFO 27712 --- [ restartedMain] DeferredRepositoryInitializationListener : Triggering deferred initialization of Spring Data repositories… 2020-12-29 23:31:30.856 INFO 27712 --- [ restartedMain] DeferredRepositoryInitializationListener : Spring Data repositories initialized! 2020-12-29 23:31:30.893 INFO 27712 --- [ restartedMain] c.k.booksapi.BooksApiApplication : Started BooksApiApplication in 18.509 seconds (JVM running for 21.016)

Could you please go ahead and add review comments if required?