Members backend CRUD functions#43
Conversation
…d MemberService to convert requests into members for createMember and updateMember.
…emberSqlRepo for implementation
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| Member updatedMember = memberRepo.updateMember(member); | ||
| return MemberDto.from(updatedMember); |
There was a problem hiding this comment.
Type mismatch: memberRepo.updateMember() returns Optional<Member> (as defined in MemberRepo interface line 40), but the code tries to assign it directly to Member updatedMember. This will cause a compilation error.
Fix:
Member updatedMember = memberRepo.updateMember(member)
.orElseThrow(() -> new ResponseStatusException(
HttpStatus.INTERNAL_SERVER_ERROR,
"Failed to update member"));
return MemberDto.from(updatedMember);| Member updatedMember = memberRepo.updateMember(member); | |
| return MemberDto.from(updatedMember); | |
| Member updatedMember = memberRepo.updateMember(member) | |
| .orElseThrow(() -> new ResponseStatusException( | |
| HttpStatus.INTERNAL_SERVER_ERROR, | |
| "Failed to update member")); | |
| return MemberDto.from(updatedMember); | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
Graphite Automations"Request reviewers once CI passes" took an action on this PR • (07/06/26)2 reviewers were added to this PR based on Henry Chen's automation. |
… updatedMember throws error if member not found
|
| email TEXT UNIQUE NOT NULL, | ||
| linked_url TEXT, | ||
| bio TEXT, | ||
| linked_in_url TEXT, |
There was a problem hiding this comment.
These modifications should be made in a follow up migration file, not in this existing one. The documentation talks about this (as these will only run once).
You can use ALTER TABLE ... RENAME COLUMN ... to accomplish this.
There was a problem hiding this comment.
Noted, got feedback from henry that changes to migration should be in its own PR
|
|
||
| public MemberDto createMember(CreateMemberRequest request) { | ||
| if (memberRepo.getMemberByEmail(request.email()).isPresent()) { | ||
| throw new ResponseStatusException( |
There was a problem hiding this comment.
Ideally we want to have an abstraction of logic between the MemberService and the controller.
MemberService should not necessarily know about what HTTP behavior/code we're going to emit for each case. That's what the controller is for. Any idea for a better way we could handle this kind of error?
There was a problem hiding this comment.
Since you are using 404's multiple times, here is a useful guide, I've used this before and it's pretty cool what Spring can do: https://dev.to/realnamehidden1_61/how-do-you-map-different-exceptions-to-different-http-status-codes-in-spring-boot-2j83
|
|
||
| public MemberDto updateMember(UpdateMemberRequest request, UUID id) { | ||
| if (memberRepo.getMemberById(id).isEmpty()) { | ||
| throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Member with ID " + id + " does not exist"); |
There was a problem hiding this comment.
Same comment as above. (For HTTP codes)
| throw new ResponseStatusException( | ||
| HttpStatus.CONFLICT, "Member with email " + request.email() + " already exists"); | ||
| } | ||
| Member member = Member.builder() |
There was a problem hiding this comment.
Wouldn't it be better to just use the existing member object from the repo? We already check if it exists above.
If we're creating a whole new member object, we might be losing information from the old object. (We can't assume that the update request has all the same data as the member object, there may be some stuff we don't want to update :))
There was a problem hiding this comment.
Did you mean to highlight the member builder in updateMember? If so, i'll work on making the updateMember support partial updates rather than full updates, and not have to build a member for every update.
2 participants
Note: Submitting this PR to see if I'm on track. Have yet to finish MemberSqlRepo.java, MemberController.java, and local testing
Files created:
1. Member.java
(src/main/java/org/patinanetwork/patchats/common/db/models/member/Member.java)
2. MemberRepo.java (src/main/java/org/patinanetwork/patchats/common/db/repos/member/MemberRepo.java)
3. MemberSqlRepo.java (src/main/java/org/patinanetwork/patchats/common/db/repos/member/MemberSqlRepo.java)
4. MemberService.java (src/main/java/org/patinanetwork/patchats/api/member/MemberService.java)
5. MemberController.java
(src/main/java/org/patinanetwork/patchats/api/member/MemberController.java)
5. MemberDto.java, CreateMemberRequest.java, UpdateMemberRequest.java (src/main/java/org/patinanetwork/patchats/common/dto/member/CreateMemberRequest.java, src/main/java/org/patinanetwork/patchats/common/dto/member/UpdateMemberRequest.java, src/main/java/org/patinanetwork/patchats/common/dto/member/MemberDto.java)
6. UserProfile.page.tsx (js/src/features/user-profile/UserProfile.page.tsx)
Files changed:
1. V0001__Create_members_table.sql (db/migration/V0001__Create_members_table.sql)
2. SignUpForm.tsx, user-profile/api/schemas.ts, user-profile/types.ts
(js/src/features/sign-up/components/SignUpForm.tsx, js/src/features/user-profile/api/schemas.ts, js/src/features/user-profile/types.ts)
3. signUpFormConfig.ts
(js/src/features/sign-up/components/signUpFormConfig.ts)