Create User DB Layer for v9 Migration #12110
Conversation
domlimm
force-pushed
the
v9-user-db-layer
branch
4 times, most recently
from
4446e09
to
f01e51e
Compare
| /** | ||
| * Checks if a user exists by its {@code id}. | ||
| */ | ||
| private boolean doesUserExist(Integer id) { | ||
| assert id != null; | ||
|
|
||
| User user = HibernateUtil.getSessionFactory().getCurrentSession().get(User.class, id); | ||
|
|
||
| return user != null; | ||
| } |
| String courseId = user.getCourse().getId(); | ||
| String email = user.getEmail(); | ||
|
|
||
| if (doesUserExist(courseId, email)) { |
There was a problem hiding this comment.
I see the issue here, will need to split this method back into instructor/student. Validation for both would be different, since course/email is not unique for user.
There was a problem hiding this comment.
Thanks for your comment samuel! By splitting back into instructor/student does that mean reverting back to something like this?
// This logic is definitely wrong 😓
if (getInstructor(user.getId()) == null) {
throw new EntityAlreadyExistsException(String.format(ERROR_CREATE_ENTITY_ALREADY_EXISTS, user.toString()));
}
if (getStudent(user.getId()) == null) {
throw new EntityAlreadyExistsException(String.format(ERROR_CREATE_ENTITY_ALREADY_EXISTS, user.toString()));
}I checked on the EntitiesDb hasExisting methods in both Instructor and Student:
Instructor
generateId in Instructor
Student
generateId in Student
Do I follow these above? If I am, may I ask how should I go about doing it for Student? Noted that course + email is not unique for user too, for this new implementation it would be id itself?
There was a problem hiding this comment.
No, split createUser into createInstructor and createStudent. Validate existence separately for both.
There was a problem hiding this comment.
Okay, thanks samuel!!
There was a problem hiding this comment.
One last thing, you mentioned that validation will be different for both, are we not using courseId and email anymore? How should I do the validation? A little lost on this part
There was a problem hiding this comment.
Same, but need hasExistngInstructor and hasExistingStudent. courseId and email is only unique for instructor/student, not for users in general.
There was a problem hiding this comment.
Resolved! Thanks samuel for clarifying! Hope it's good now!
domlimm
force-pushed
the
v9-user-db-layer
branch
from
40e3779
to
c5d913b
Compare
- Split create method into subclass methods - Rename existence method to be uniform
samuelfangjw
force-pushed
the
v9-user-db-layer
branch
from
7e5c521
to
7a893f2
Compare
|
@samuelfangjw Hey samuel, noted on the changes! Thank you! For the Thanks again! 🍻 |
wkurniawan07
added
s.ToMerge
Part of #12048.
Outline of Solution
The User DB layer supports CRUD ops for both Instructor and Student entities.
To add tests in the future.