-
Notifications
You must be signed in to change notification settings - Fork 36
Conversation
This pull request has been mentioned on SEF Hive. There might be relevant details there: https://sef.discourse.group/t/implementing-the-comments-adding-feature/377/2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome! @anjula-sack
Added some suggestions. We'll have to validate the user's profile to prevent normal users from changing the mentee comments
src/main/java/org/sefglobal/scholarx/controller/MenteeController.java
Outdated
Show resolved
Hide resolved
String msg = "Error, Mentee by id: " + menteeId + " doesn't exist."; | ||
log.error(msg); | ||
throw new ResourceNotFoundException(msg); | ||
} else if (optionalMentee.get().getProfile().getId() == profileId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throw unauthorized exception if the profile doesn't belong to the assignedMentor or an admin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll have to add the validation used in line 68-77
src/main/java/org/sefglobal/scholarx/service/CommentService.java
Outdated
Show resolved
Hide resolved
String msg = "Error, Mentee by id: " + menteeId + " doesn't exist."; | ||
log.error(msg); | ||
throw new ResourceNotFoundException(msg); | ||
} else if (optionalMentee.get().getProfile().getId() == profileId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll have to add the validation used in line 68-77
This pull request has been mentioned on SEF Hive. There might be relevant details there: https://sef.discourse.group/t/implementing-the-comments-adding-feature/377/9 |
src/main/java/org/sefglobal/scholarx/service/CommentService.java
Outdated
Show resolved
Hide resolved
src/main/java/org/sefglobal/scholarx/service/CommentService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! @anjula-sack
Thanks for getting this done
Purpose
Goals
Approach
Checklist