Remove movie edit functionality from MovieController.java

mohitudaysharma commented 3 months ago

Purpose

This PR removes the editMovie endpoint from the MovieController class, as the movie editing functionality is no longer required in the application. The delete movie functionality has been updated to handle the deletion of the associated movie poster file.

Critical Changes

Removed the editMovie method from the MovieController class, which was responsible for updating an existing movie. This functionality is no longer needed in the application.
Updated the deleteMovie method to handle the deletion of the associated movie poster file when a movie is deleted. The filename variable is now returned from the deleteMovie service method and used to delete the file from the file system.

===== Original PR title and description ============

Original Title: Update MovieController.java

Original Description: None

dev-archie-ai-code-explain-pr[bot] commented 3 months ago

PR Review Summary 🔍

This PR removes the editMovie method from the MovieController class. While this simplifies the controller, it potentially removes functionality for editing movies. The removal of this endpoint could impact existing clients or frontend applications that rely on this feature. It's crucial to ensure this change aligns with the overall application requirements and that any dependent systems are updated accordingly.

⚡ Logical error analysis

- Removal of editMovie method may break existing functionality if other parts of the application depend on it. - No replacement logic for editing movies is provided, potentially leaving a gap in CRUD operations.

🧪 Test coverage analysis

- Tests for the removed editMovie method should be updated or removed to maintain test consistency. - Ensure integration tests or any tests relying on the edit functionality are adjusted accordingly.

🔒 Security analysis

- No critical security issues identified. The removal of an endpoint generally reduces the attack surface, which can be seen as a minor security improvement.

dev-archie-ai-code-explain-pr[bot] commented 3 months ago

PR Code Suggestions Summary ✨

Category	Suggestion
Enhancement	Reintroduce the editMovie endpoint with improved error handling to maintain the ability to update movie information. File: MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java [L1-L15](https://github.com/mohitudaysharma/ticketing-app/blob/47f29cdb74b3c01b9841f5ce73da49ce3d37d7b8/MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java#L1-L15) Suggestion: Consider adding a replacement for the removed editMovie endpoint to maintain the ability to update movie information. If the edit functionality is no longer needed, ensure that this change is intentional and documented. ```diff --- Original +++ Improved @@ -1,15 +1,25 @@ @DeleteMapping("/remove/{code}") public ResponseEntity deleteMovie(@PathVariable String code) { String filename = movieService.deleteMovie(code); if(filename != null) { String filepath = new File("").getAbsolutePath() + "/src/main/resources/static/posters/" + filename; File file = new File(filepath); if(file.delete()) { System.out.println("File deleted successfully"); } } // else { // return new ResponseEntity>(CommonUtils.messageJson("The given movie was not found."), HttpStatus.NOT_FOUND); // } return new ResponseEntity>(CommonUtils.messageJson("The given movie was deleted."), HttpStatus.OK); +} + +@PutMapping("/edit/{code}") +public ResponseEntity editMovie(@PathVariable String code, @Valid @RequestBody MovieRequest movie) { + MovieResponse updatedMovie = movieService.editMovie(code, movie); + if (updatedMovie != null) { + return new ResponseEntity<>(updatedMovie, HttpStatus.OK); + } else { + return new ResponseEntity<>(CommonUtils.messageJson("Movie not found"), HttpStatus.NOT_FOUND); + } } ```
Enhancement	Enhance error handling in the deleteMovie endpoint to properly handle cases where the movie is not found and improve file deletion feedback. File: MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java [L1-L15](https://github.com/mohitudaysharma/ticketing-app/blob/47f29cdb74b3c01b9841f5ce73da49ce3d37d7b8/MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java#L1-L15) Suggestion: Improve error handling in the deleteMovie endpoint by uncommenting and enhancing the else block to handle cases where the movie is not found. ```diff --- Original +++ Improved @@ -1,15 +1,16 @@ @DeleteMapping("/remove/{code}") public ResponseEntity deleteMovie(@PathVariable String code) { String filename = movieService.deleteMovie(code); if(filename != null) { String filepath = new File("").getAbsolutePath() + "/src/main/resources/static/posters/" + filename; File file = new File(filepath); if(file.delete()) { System.out.println("File deleted successfully"); + } else { + System.out.println("Failed to delete file: " + filepath); } + return new ResponseEntity<>(CommonUtils.messageJson("The given movie was deleted."), HttpStatus.OK); + } else { + return new ResponseEntity<>(CommonUtils.messageJson("The given movie was not found."), HttpStatus.NOT_FOUND); } -// else { -// return new ResponseEntity>(CommonUtils.messageJson("The given movie was not found."), HttpStatus.NOT_FOUND); -// } - return new ResponseEntity>(CommonUtils.messageJson("The given movie was deleted."), HttpStatus.OK); } ```
Enhancement	Enhance the deleteMovie endpoint to provide more detailed information about the deletion process, including file deletion status and filename. File: MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java [L1-L15](https://github.com/mohitudaysharma/ticketing-app/blob/47f29cdb74b3c01b9841f5ce73da49ce3d37d7b8/MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java#L1-L15) Suggestion: Enhance the deleteMovie endpoint to return more detailed information about the deletion process, including whether the file was successfully deleted. ```diff --- Original +++ Improved @@ -1,15 +1,16 @@ @DeleteMapping("/remove/{code}") public ResponseEntity deleteMovie(@PathVariable String code) { String filename = movieService.deleteMovie(code); if(filename != null) { String filepath = new File("").getAbsolutePath() + "/src/main/resources/static/posters/" + filename; File file = new File(filepath); - if(file.delete()) { - System.out.println("File deleted successfully"); - } + boolean fileDeleted = file.delete(); + Map response = new HashMap<>(); + response.put("message", "The given movie was deleted."); + response.put("fileDeleted", Boolean.toString(fileDeleted)); + response.put("filename", filename); + return new ResponseEntity<>(response, HttpStatus.OK); + } else { + return new ResponseEntity<>(CommonUtils.messageJson("The given movie was not found."), HttpStatus.NOT_FOUND); } -// else { -// return new ResponseEntity>(CommonUtils.messageJson("The given movie was not found."), HttpStatus.NOT_FOUND); -// } - return new ResponseEntity>(CommonUtils.messageJson("The given movie was deleted."), HttpStatus.OK); } ```
Maintainability	Refactor the deleteMovie endpoint to separate the file deletion logic into a private method, improving code organization and maintainability. File: MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java [L1-L15](https://github.com/mohitudaysharma/ticketing-app/blob/47f29cdb74b3c01b9841f5ce73da49ce3d37d7b8/MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java#L1-L15) Suggestion: Refactor the deleteMovie endpoint to separate concerns by moving the file deletion logic to a separate method or service. ```diff --- Original +++ Improved @@ -1,15 +1,19 @@ @DeleteMapping("/remove/{code}") public ResponseEntity deleteMovie(@PathVariable String code) { String filename = movieService.deleteMovie(code); if(filename != null) { - String filepath = new File("").getAbsolutePath() + "/src/main/resources/static/posters/" + filename; - File file = new File(filepath); - if(file.delete()) { - System.out.println("File deleted successfully"); - } + boolean fileDeleted = deleteMoviePoster(filename); + Map response = new HashMap<>(); + response.put("message", "The given movie was deleted."); + response.put("fileDeleted", Boolean.toString(fileDeleted)); + return new ResponseEntity<>(response, HttpStatus.OK); + } else { + return new ResponseEntity<>(CommonUtils.messageJson("The given movie was not found."), HttpStatus.NOT_FOUND); } -// else { -// return new ResponseEntity>(CommonUtils.messageJson("The given movie was not found."), HttpStatus.NOT_FOUND); -// } - return new ResponseEntity>(CommonUtils.messageJson("The given movie was deleted."), HttpStatus.OK); +} + +private boolean deleteMoviePoster(String filename) { + String filepath = new File("").getAbsolutePath() + "/src/main/resources/static/posters/" + filename; + File file = new File(filepath); + return file.delete(); } ```
Enhancement	Add logging to the deleteMovie endpoint to improve traceability and debugging capabilities. File: MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java [L1-L15](https://github.com/mohitudaysharma/ticketing-app/blob/47f29cdb74b3c01b9841f5ce73da49ce3d37d7b8/MovieBookingApp/src/main/java/com/example/demo/controller/MovieController.java#L1-L15) Suggestion: Add logging to the deleteMovie endpoint to improve traceability and debugging capabilities. ```diff --- Original +++ Improved @@ -1,15 +1,32 @@ -@DeleteMapping("/remove/{code}") -public ResponseEntity deleteMovie(@PathVariable String code) { - String filename = movieService.deleteMovie(code); - if(filename != null) { - String filepath = new File("").getAbsolutePath() + "/src/main/resources/static/posters/" + filename; - File file = new File(filepath); - if(file.delete()) { - System.out.println("File deleted successfully"); - } - } -// else { -// return new ResponseEntity>(CommonUtils.messageJson("The given movie was not found."), HttpStatus.NOT_FOUND); -// } - return new ResponseEntity>(CommonUtils.messageJson("The given movie was deleted."), HttpStatus.OK); +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +// ... other imports ... + +@RestController +@RequestMapping("/api/v1/movies") +public class MovieController { + private static final Logger logger = LoggerFactory.getLogger(MovieController.class); + + // ... other methods ... + + @DeleteMapping("/remove/{code}") + public ResponseEntity deleteMovie(@PathVariable String code) { + logger.info("Attempting to delete movie with code: {}", code); + String filename = movieService.deleteMovie(code); + if(filename != null) { + String filepath = new File("").getAbsolutePath() + "/src/main/resources/static/posters/" + filename; + File file = new File(filepath); + if(file.delete()) { + logger.info("File deleted successfully: {}", filepath); + } else { + logger.warn("Failed to delete file: {}", filepath); + } + logger.info("Movie deleted successfully: {}", code); + return new ResponseEntity<>(CommonUtils.messageJson("The given movie was deleted."), HttpStatus.OK); + } else { + logger.warn("Movie not found for deletion: {}", code); + return new ResponseEntity<>(CommonUtils.messageJson("The given movie was not found."), HttpStatus.NOT_FOUND); + } + } } ```

mohitudaysharma / ticketing-app