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

Feat/delete book #4

Merged
merged 7 commits into from
Aug 15, 2024
Merged

Feat/delete book #4

merged 7 commits into from
Aug 15, 2024

Conversation

SimonSta95
Copy link
Contributor

Added a DELETE endpoint in the backend to delete books by id.
Added a "Delete" Button to the Details page.

# Conflicts:
#	backend/src/main/java/com/github/esgoet/backend/controllers/BookController.java
#	backend/src/main/java/com/github/esgoet/backend/services/BookService.java
#	backend/src/test/java/com/github/esgoet/backend/controllers/BookControllerIntegrationTest.java
#	backend/src/test/java/com/github/esgoet/backend/services/BookServiceUnitTest.java
#	frontend/src/pages/BookGalleryPage/components/bookCard/BookCard.tsx
bookRepository.save(new Book("1", "Simon", "HowToDeleteBooksFast"));

mockMvc.perform(MockMvcRequestBuilders.delete("/api/books/1"))
.andExpect(status().isOk());
Copy link
Contributor

Choose a reason for hiding this comment

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

delete Integration test should include performing a getAll and expecting an empty list?

        mockMvc.perform(MockMvcRequestBuilders.get("/api/books"))
                .andExpect(status().isOk())
                .andExpect(content().json("""
                   []
                 """));

@@ -19,13 +19,19 @@ export default function BookDetailsPage() {
fetchBook();
},[])

const deleteBook = (id: string) => {
axios.delete("/api/books/" + id)
Copy link
Contributor

Choose a reason for hiding this comment

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

needs reload of data and going back to books page? can useNavigate and might be good to do have the actual deletion method in App.tsx and just pass it to BookDetailsPage, so we can call fetchData easily in the delete method
also should have catching error
In App.tsx:

     function deleteBook(string :id) {
        axios.delete("/api/books/" + id)
            .then((response) => response.status === 200 && fetchData())
            .catch((error)=>console.log(error.message))
      }

in BookDetailsPage.tsx:

Suggested change
axios.delete("/api/books/" + id)
deleteBook(id);
navigate("/books");

Copy link
Contributor

@esgoet esgoet left a comment

Choose a reason for hiding this comment

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

lgtm

@SimonSta95 SimonSta95 merged commit 0ee4f7e into main Aug 15, 2024
1 check passed
@SimonSta95 SimonSta95 deleted the Feat/DELETE-book branch August 15, 2024 09:00
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.

2 participants