-
Notifications
You must be signed in to change notification settings - Fork 0
MemberService.java updateMember #68
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,22 +40,35 @@ public MemberDto createMember(CreateMemberRequest request) { | |
| } | ||
|
|
||
| public MemberDto updateMember(UpdateMemberRequest request, UUID id) { | ||
| if (memberRepo.getMemberById(id).isEmpty()) { | ||
| throw new MemberNotFoundException(id); | ||
| Member member = memberRepo.getMemberById(id).orElseThrow(() -> new MemberNotFoundException(id)); | ||
|
|
||
| if (request.fullName() != null) { | ||
| member.setFullName(request.fullName()); | ||
| } | ||
| if (request.email() != null) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'd assume we wouldn't want to assign some of these to blanks either? I think spring has some utils for this |
||
| member.setEmail(request.email()); | ||
| } | ||
| if (request.linkedInUrl() != null) { | ||
| member.setLinkedInUrl(request.linkedInUrl()); | ||
| } | ||
| if (request.introduction() != null) { | ||
| member.setIntroduction(request.introduction()); | ||
| } | ||
| if (request.matchPref() != null) { | ||
| member.setMatchPref(request.matchPref()); | ||
| } | ||
| if (request.industryPref() != null) { | ||
| member.setIndustryPref(request.industryPref()); | ||
| } | ||
| if (request.rolePref() != null) { | ||
| member.setRolePref(request.rolePref()); | ||
| } | ||
| if (request.topics() != null) { | ||
| member.setTopics(request.topics()); | ||
| } | ||
| if (request.extraNotes() != null) { | ||
| member.setExtraNotes(request.extraNotes()); | ||
| } | ||
| // TODO: Implement partial update instead of full update | ||
| Member member = Member.builder() | ||
| .id(id) | ||
| .fullName(request.fullName()) | ||
| .email(request.email()) | ||
| .linkedInUrl(request.linkedInUrl()) | ||
| .introduction(request.introduction()) | ||
| .matchPref(request.matchPref()) | ||
| .industryPref(request.industryPref()) | ||
| .rolePref(request.rolePref()) | ||
| .topics(request.topics()) | ||
| .extraNotes(request.extraNotes()) | ||
| .build(); | ||
| Member updatedMember = memberRepo.updateMember(member).orElseThrow(() -> new MemberNotFoundException(id)); | ||
| return MemberDto.from(updatedMember); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,13 @@ | ||
| package org.patinanetwork.patchats.api.member.dto; | ||
|
|
||
| import jakarta.validation.constraints.Email; | ||
| import jakarta.validation.constraints.NotBlank; | ||
|
|
||
| public record UpdateMemberRequest( | ||
| @NotBlank String fullName, | ||
| String fullName, | ||
| // TODO: Changes to email require verification after authentication is implemented | ||
| @NotBlank @Email String email, | ||
| @Email String email, | ||
| String linkedInUrl, | ||
| @NotBlank String introduction, | ||
| String introduction, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will this require a DB migration? |
||
| String matchPref, | ||
| String industryPref, | ||
| String rolePref, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,9 @@ | |
| import org.patinanetwork.patchats.api.member.db.repos.MemberRepo; | ||
| import org.patinanetwork.patchats.api.member.dto.CreateMemberRequest; | ||
| import org.patinanetwork.patchats.api.member.dto.MemberDto; | ||
| import org.patinanetwork.patchats.api.member.dto.UpdateMemberRequest; | ||
| import org.patinanetwork.patchats.common.web.exception.MemberDuplicateException; | ||
| import org.patinanetwork.patchats.common.web.exception.MemberNotFoundException; | ||
|
|
||
| class MemberServiceTest { | ||
|
|
||
|
|
@@ -105,20 +107,152 @@ void createMember_throwsExceptionWhenEmailAlreadyExists() { | |
| verify(memberRepo, never()).createMember(any()); | ||
| } | ||
|
|
||
| // TODO: Implement test cases for MemberService methods after createMember | ||
| @Test | ||
| void updateMember_throwsExceptionWhenMemberNotFound() { | ||
| final UUID id = UUID.randomUUID(); | ||
| final UpdateMemberRequest request = new UpdateMemberRequest( | ||
| "Updated Name", | ||
| "updated@example.com", | ||
| "https://linkedin.com/in/updated", | ||
| "Updated intro", | ||
| "Mentor - I am looking for guidance from someone with more experience", | ||
| "Technology", | ||
| "Software Engineer", | ||
| "AI,ML", | ||
| "Notes"); | ||
|
|
||
| when(memberRepo.getMemberById(id)).thenReturn(Optional.empty()); | ||
|
|
||
| assertThrows(MemberNotFoundException.class, () -> memberService.updateMember(request, id)); | ||
| verify(memberRepo, never()).updateMember(any()); | ||
| } | ||
|
|
||
| @Test | ||
| void updateMember_successWithOnlyNameField() { | ||
| final UUID id = UUID.randomUUID(); | ||
| final UpdateMemberRequest request = | ||
| new UpdateMemberRequest("Updated Name", null, null, null, null, null, null, null, null); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does the controller serialize the JSON into nulls? |
||
|
|
||
| final Member existingMember = Member.builder() | ||
| .id(id) | ||
| .fullName("Old Name") | ||
| .email("old@example.com") | ||
| .linkedInUrl("https://linkedin.com/in/old") | ||
| .introduction("Old intro") | ||
| .matchPref("Mentor") | ||
| .industryPref("Finance") | ||
| .rolePref("Analyst") | ||
| .topics("Economics") | ||
| .extraNotes("Old notes") | ||
| .build(); | ||
|
|
||
| final Member updatedMember = Member.builder() | ||
| .id(id) | ||
| .fullName("Updated Name") | ||
| .email("old@example.com") | ||
| .linkedInUrl("https://linkedin.com/in/old") | ||
| .introduction("Old intro") | ||
| .matchPref("Mentor") | ||
| .industryPref("Finance") | ||
| .rolePref("Analyst") | ||
| .topics("Economics") | ||
| .extraNotes("Old notes") | ||
| .build(); | ||
|
|
||
| when(memberRepo.getMemberById(id)).thenReturn(Optional.of(existingMember)); | ||
| when(memberRepo.updateMember(any())).thenReturn(Optional.of(updatedMember)); | ||
|
|
||
| final MemberDto response = memberService.updateMember(request, id); | ||
|
|
||
| assertEquals("Updated Name", response.getFullName()); | ||
| assertEquals("old@example.com", response.getEmail()); | ||
| assertEquals("https://linkedin.com/in/old", response.getLinkedInUrl()); | ||
| assertEquals("Old intro", response.getIntroduction()); | ||
| } | ||
|
|
||
| @Test | ||
| void updateMember_successWithAllNullFields() { | ||
| final UUID id = UUID.randomUUID(); | ||
| final UpdateMemberRequest request = | ||
| new UpdateMemberRequest(null, null, null, null, null, null, null, null, null); | ||
|
|
||
| final Member existingMember = Member.builder() | ||
| .id(id) | ||
| .fullName("Name") | ||
| .email("email@example.com") | ||
| .linkedInUrl("https://linkedin.com/in/john") | ||
| .introduction("intro") | ||
| .matchPref("Friend") | ||
| .industryPref("Tech") | ||
| .rolePref("Engineer") | ||
| .topics("AI") | ||
| .extraNotes("notes") | ||
| .build(); | ||
|
|
||
| // @Test | ||
| // void testUpdateMember() { | ||
| // // Implement test logic for updateMember method | ||
| // } | ||
| when(memberRepo.getMemberById(id)).thenReturn(Optional.of(existingMember)); | ||
| when(memberRepo.updateMember(any())).thenReturn(Optional.of(existingMember)); | ||
|
|
||
| // @Test | ||
| // void testGetMemberById() { | ||
| // // Implement test logic for getMemberById method | ||
| // } | ||
| final MemberDto response = memberService.updateMember(request, id); | ||
|
|
||
| // @Test | ||
| // void testDeactivateMember() { | ||
| // // Implement test logic for deactivateMember method | ||
| // } | ||
| assertEquals("Name", response.getFullName()); | ||
| assertEquals("email@example.com", response.getEmail()); | ||
| assertEquals("https://linkedin.com/in/john", response.getLinkedInUrl()); | ||
| assertEquals("intro", response.getIntroduction()); | ||
| } | ||
|
|
||
| @Test | ||
| void updateMember_successWithAllFields() { | ||
| final UUID id = UUID.randomUUID(); | ||
| final UpdateMemberRequest request = new UpdateMemberRequest( | ||
| "Updated Name", | ||
| "updated@example.com", | ||
| "https://linkedin.com/in/updated", | ||
| "Updated intro", | ||
| "Mentor", | ||
| "Tech", | ||
| "Engineer", | ||
| "AI,ML", | ||
| "Notes"); | ||
|
|
||
| final Member existingMember = Member.builder() | ||
| .id(id) | ||
| .fullName("Old Name") | ||
| .email("old@example.com") | ||
| .linkedInUrl("https://linkedin.com/in/old") | ||
| .introduction("Old intro") | ||
| .matchPref("Friend") | ||
| .industryPref("Finance") | ||
| .rolePref("Analyst") | ||
| .topics("Economics") | ||
| .extraNotes("Old notes") | ||
| .build(); | ||
|
|
||
| final Member updatedMember = Member.builder() | ||
| .id(id) | ||
| .fullName(request.fullName()) | ||
| .email(request.email()) | ||
| .linkedInUrl(request.linkedInUrl()) | ||
| .introduction(request.introduction()) | ||
| .matchPref(request.matchPref()) | ||
| .industryPref(request.industryPref()) | ||
| .rolePref(request.rolePref()) | ||
| .topics(request.topics()) | ||
| .extraNotes(request.extraNotes()) | ||
| .build(); | ||
|
|
||
| when(memberRepo.getMemberById(id)).thenReturn(Optional.of(existingMember)); | ||
| when(memberRepo.updateMember(any())).thenReturn(Optional.of(updatedMember)); | ||
|
|
||
| final MemberDto response = memberService.updateMember(request, id); | ||
|
|
||
| assertEquals(request.fullName(), response.getFullName()); | ||
| assertEquals(request.email(), response.getEmail()); | ||
| assertEquals(request.linkedInUrl(), response.getLinkedInUrl()); | ||
| assertEquals(request.introduction(), response.getIntroduction()); | ||
| assertEquals(request.matchPref(), response.getMatchPref()); | ||
| assertEquals(request.industryPref(), response.getIndustryPref()); | ||
| assertEquals(request.rolePref(), response.getRolePref()); | ||
| assertEquals(request.topics(), response.getTopics()); | ||
| assertEquals(request.extraNotes(), response.getExtraNotes()); | ||
| } | ||
| } | ||
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.
How does this handle null vs empty in the string?