st-tu-dresden / videoshop

SalesPoint sample application
Apache License 2.0
87 stars 246 forks source link

Make comment and rating required #135

Closed Nitwel closed 3 years ago

Nitwel commented 3 years ago

By pressing the add comment button while having no comment written the page will end up in Whitelabel Error Page. The same happens when setting the rating to anything like -1 or 500. It would also be nice to have proper error handling (an /error page).

sonarcloud[bot] commented 3 years ago

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

martinmo commented 3 years ago

Hi Nitwel, thanks for your input. Your changes to the HTML templates seem to be okay, however, they only fix half of the problem (= the client side). The complete fix involves handling Spring's form validation information with a BindingResult parameter in the method CatalogController.comment(…).

If you want, you can take a shot at it, there is a good tutorial on https://spring.io/guides/gs/validating-form-input/. Please note that the videoshop already contains a form class (CommandAndRating) with the appropriate validation annotations.

Nitwel commented 3 years ago

As far as I have seen the error handling on the server side is already implemented here. The thing is that there is no error handling on the client side / you only get redirected to the error page. And when the rating only goes from 1-5 on the server side, this should also be restricted on the client side.

martinmo commented 3 years ago

As far as I have seen the error handling on the server side is already implemented here.

It's not completely implemented. Sure, the comment(…) controller method takes a CommentAndRating with @Valid, but it doesn't handle the case of a binding error. You can only handle such errors if you declare an additional parameter of type BindingResult (or its parent interface Error) and use its hasErrors() method. An example of this can be seen in the guestbook, in @PostMapping addEntry(…).

In addition to that, the Thymeleaf template needs to be adjusted to render the validation error messages in the <form>.

martinmo commented 3 years ago

Your change has found its way into the repo via commit 38c2447 🚀 I added the server side part. Thanks for contributing!