Project-Books / books-api

GraphQL Books API
https://project-books.github.io/#books-api
MIT License
36 stars 61 forks source link

Literary award 15 #109

Closed swetharnaik closed 3 years ago

swetharnaik commented 3 years ago

Summary of change

Created an Award Entity which maps to Book having ManytoMany relationship. Added an Enum class, Updated SQL Script, Updated GraphQL Schema, Added test data. GraphQL Query Example- query { findByAwardName(awardName:"Nobel Prize in Literature") { title genre isbn13 yearOfPublication format } }

Related issue

Closes #15

Pull request checklist

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

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 or by creating a new Q&A discussion on GitHub

swetharnaik commented 3 years ago

@knjk04 I have raised PR but I have some queries/issues that I found during development-

  1. During development, while trying to get authors/awards fields also from findAllBooks query, I used to get the below exception- Caused by: org.hibernate.LazyInitializationException: failed to lazily initialize a collection of role: com.karankumar.booksapi.model.Book.awards, could not initialize proxy - no Session

This was because the FetchType was LAZY. When I changed it to EAGER, I got the proper output as seen below image

  1. The test cases written for the repository class are failing because of the deletion call which doesn't get execute successfully. It gets blocked because of its reference (foreign key) in another table. For example, in AuthorRepositoryTest, while trying to call deleteAll(), it will throw an DataIntegrityViolationException because author_id is present in book_award table as well. On making the CascadeType to REMOVE, this was successfully resolved.

If these are valid issues that are currently present, can I make these changes?

knjk04 commented 3 years ago

@swetharnaik Thanks for raising the PR & apologies for the slow response. I'll get back to you on this.

swetharnaik commented 3 years ago

@knjk04 Sure no problem.

knjk04 commented 3 years ago

Sorry for not getting back to you sooner.

  1. Ideally, we would be using lazy for performance reasons. Are you able to get this to work if you just use a join?

  2. That should be fine

knjk04 commented 3 years ago

@swetharnaik How are you getting on with this?

swetharnaik commented 3 years ago

@knjk04 Apologies for the late reply. Will work on it this week. Need to integrate with latest updates and check.

knjk04 commented 3 years ago

@swetharnaik How are you getting on with this?

knjk04 commented 3 years ago

@swetharnaik Closing due to no response (as per our contributing guidelines)