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

Feature: User reviews #48

Merged
merged 8 commits into from Aug 5, 2022

Conversation

Eukon05
Copy link
Contributor

@Eukon05 Eukon05 commented Jul 23, 2022

Introduced reviewing users

A review consists of a star-based rating (1-5) and an optional description (referred in the code as "content").

Everyone can see what reviews has a user received, as well as all reviews posted by the user.
A review can be edited and deleted by its author.
The input is validated in the controller using Spring validation, and additionally the Review entity is validated by Hibernate.
The reviews are retrieved in pages of 20 elements, to avoid loading a full list of reviews at once.
You can also sort the reviews by stars or creation date.

Endpoints:
api/v1/reviews - POST, used to create a review
api/v1/reviews/{id} - PUT, DELETE, used to modify or delete a review
api/v1/users/{userId}/reviews/received - GET, used to retrieve reviews that the user has received
api/v1/users/{userId}/reviews/posted - GET, used to retrieve reviews posted by the user

I'll provide a full documentation in a separate pull request.

Eukon05 and others added 6 commits July 16, 2022 21:58
Implemented updating and deleting reviews.
…here possible.

Fixed antMatchers related to reviews in WebSecurityConfig.
- Added a "@Valid" annotation to UpdateReviewDTO parameter in updateReview method in ReviewController.
- Added a new constructor to UserNotFoundException to allow for handling situations in which te user is being retrieved by username (so that we don't have to rely on Spring Security exceptions where not necessary).
- Moved review content parsing from ReviewService methods to a setter in the Review object
@Eukon05 Eukon05 changed the title Feature: user reviews Feature: User reviews Jul 23, 2022
@Eukon05 Eukon05 requested a review from dawciobiel July 23, 2022 12:43
@Eukon05 Eukon05 added the feature/ New feature or request label Jul 23, 2022
@Eukon05 Eukon05 marked this pull request as ready for review July 23, 2022 13:30
Copy link
Member

@dawciobiel dawciobiel left a comment

Choose a reason for hiding this comment

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

Code modification required, details in comments for individual class files.

- Changed id variable type to a primitive type
- Added a maximum size of the content variable (400)
- Added @NotNull constraints on user objects

DTOs:
- Modified constraint validation on CreateReviewDTO and UpdateReviewDTO to account for changes related to the Review object

Exceptions:
- Modified exception message variable names

Mappers:
- Removed custom entity and dto mappers, to rely on MapStruct instead

ReviewServiceImpl:
- Modified createReview method to account for changes related to the Review object
- Modified methods responsible for retrieving reviews to account for the MapStruct change
# Conflicts:
#	pom.xml
#	src/main/java/pl/simpleascoding/tutoringplatform/api/ReviewController.java
Copy link
Member

@dawciobiel dawciobiel left a comment

Choose a reason for hiding this comment

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

Accepted && admitted.

After successfully creating the review, you return HttpStatus.CREATED.
When you delete the review (successful), HttpStatus.OK is returned.

It is true that both statuses are in the range 200-299, I wonder if it is worth standardizing the return. It seems to me that the controller decides what exactly returns to the client. So it would simplify the issues of checking the scope in the controller minimally.

If we would like to extend the returned result then instead of HttpStatus.OK / CREATED we should use encapsulate result (i.e.: DTO).

I wonder if we should not create our own class in which we have defined all the statuses we need, for example:
OK
CREATED
DELETED
MODIFIED
...

However, in that case, the DTO could be used right away.

@MateuszCiapala
Copy link
Member

Accepting current state with bad method return types for Controller-Service communication layers due no official convetion picked yet. Further rework will be required.

@MateuszCiapala MateuszCiapala merged commit c1f2e33 into Simple-as-Coding:main Aug 5, 2022
@Eukon05 Eukon05 deleted the feat-tutor-reviews branch August 7, 2022 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants