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

Complete Building REST services with Spring tutorial with OrderNotFoundAdvice Class #101

Open
oulanbator opened this issue Aug 8, 2021 · 1 comment

Comments

@oulanbator
Copy link

It may not be really relevant for "oppening an issue", but since the last part of the tutorial intends to put all together the Order associated classes, in the same way that we built the Employee classes (in order to build a "real" RESTful service), i felt like something was missing here : the OrderNotFoundAdvice Class.
Indeed, as we left our code at the end of the tutorial, a bad request on orders (i.e. curl -v localhost:8080/orders/5) send back a 500 error (internal server error) instead of the 404 we should expect.
It is probably just a small piece of code, and almost no explanations to add in the tutorial as the benefits are already explained earlier, when building the Employee classes.
It's my first contribution to a project on Github so I don't really know how to help more from here, but I could easily write the missing paragraph if needed.

@robertmcnees
Copy link
Contributor

Hi @oulanbator. Thank you for your issue. I see that there are inconsistencies in the links module. If you would like to submit a PR, you could contribute a new OrderNotFoundAdvice similar to the existing EmployeeNotFoundAdvice class. Make sure that the README.adoc is also updated, as some people may be building the code from scratch when going through the tutorial so all relevant instructions need to be declared.

This part seems like a separate issue to me:

Indeed, as we left our code at the end of the tutorial, a bad request on orders (i.e. curl -v localhost:8080/orders/5) send back a 500 error (internal server error) instead of the 404 we should expect.

I didn't have this issue when I went through the tutorial. While the approach between Order and Employee is inconsistent, as you called out, I did not find that it caused an unexpected result. Can you elaborate?

I'll keep this issue open for a future PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants