Project-Books / books-api

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

Add series issue 6 #81

Closed kev711 closed 3 years ago

kev711 commented 3 years ago

Summary of change

If a book is in a series, it should be possible to see:

Its position in the series com.karankumar.booksapi.repository.BookRepository.getBookPositionInBookSeries() The name of the series (e.g. 'Harry Potter' for the Harry Potter books) com.karankumar.booksapi.repository.BookRepository.getAllBookSeriesForBook() Other books in the same series com.karankumar.booksapi.repository.BookSeriesMappingRepository.getAllBooksByBookSeries()

Related issue

Closes #6

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

knjk04 commented 3 years ago

Thanks for working on this! I'll review this shortly!

kev711 commented 3 years ago

I am working on resolving the other comments...

kev711 commented 3 years ago

Thanks for working on this! I've left a few comments and made a few changes (so I recommend pulling first).

Could you please add a Flyway migration?

Hi @knjk04, I added scripts for Flyway migration. How do you test them ?

Also, made changes for other comments.

knjk04 commented 3 years ago

Hi @kev711,

We're using integration tests for that. I noticed you have some DataJpaIntegration tests. I'll need to review this closer, but that should be fine

knjk04 commented 3 years ago

Would it be possible to resolve the merge conflicts?

kev711 commented 3 years ago

Working on it...

kev711 commented 3 years ago

Merge conflicts resolved.

knjk04 commented 3 years ago

There are quite a few unrelated changes to the issue you worked on in this PR diff. Would it be possible to create a new PR to make it easier to review?

kev711 commented 3 years ago

Sure.

kev711 commented 3 years ago

Closing this one, since its not allowing me to open a new one.