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

Edits to backend ticket guide up to sub-router #55

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

liammoyn
Copy link
Contributor

Left some todos in there

@liammoyn liammoyn requested a review from connernilsen April 19, 2021 22:23
Copy link
Member

@connernilsen connernilsen left a comment

Choose a reason for hiding this comment

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

Looks good, but I saw there are still a few TODOs in there. Do we want to get this merged in and figure it out later, or fix them first?

- __common/__ is a directory containing useful methods and classes for general use cases. This can include things
like authorization, passwords, logging, and properties files.
- __persist/__ is a directory for defining and handling the data used in our application.
- __service/__ holds the main method for the project, and it handles
Copy link
Member

Choose a reason for hiding this comment

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

Extra space

Suggested change
- __service/__ holds the main method for the project, and it handles
- __service/__ holds the main method for the project, and it handles

Comment on lines +345 to 349
if (this.title == null || this.title.length() == 0) {
invalidFields.add(fieldName + "title");
}
if (this.body == null) {
if (this.body == null || this.body.length() == 0) {
invalidFields.add(fieldName + "body");
Copy link
Member

Choose a reason for hiding this comment

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

Should we change these to String.isEmpty() or do you think length is fine?

when you're done. We'll later be registering this sub-router under a protected path, so we'll also be adding information
to it, like the `jwt_data` we get on the next line. The `JWTData` object contains secure information about the user,
such as their id, which we'll need for completing this ticket.
Route handlers methods take in
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Route handlers methods take in
Route handler methods take in

such as their id, which we'll need for completing this ticket.
Route handlers methods take in
a `RoutingContext`, defined by the Vert.x library, and don't return anything. The `RoutingContext` object gives
acess to request properties (like the body, path parameters, query parameters, ...), and allows you to respond to the request
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
acess to request properties (like the body, path parameters, query parameters, ...), and allows you to respond to the request
access to request properties (like the body, path parameters, query parameters, ...), and allows you to respond to the request

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