Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,10 @@ export function MemberProfileForm({ id }: { id: string }) {
required
type="email"
label="Email Address"
description="Use an email address you check regularly, as this is how we will contact you about your matches!"
description="Use an email address you check regularly, as this is how we will contact you about your matches! (Email changes are not currently supported.)"
value={values.email}
disabled={!isEditing}
// Always disabled for now until we implement email change verification
disabled
onChange={(event) => handleFieldChange("email", event.target.value)}
onBlur={() => handleFieldBlur("email")}
error={errors.email}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.patinanetwork.patchats.common.web.exception.MemberDuplicateException;
import org.patinanetwork.patchats.common.web.exception.MemberNotFoundException;
import org.patinanetwork.patchats.common.web.exception.ValidationException;
import org.springframework.dao.DuplicateKeyException;
import org.springframework.stereotype.Service;

@Service
Expand Down Expand Up @@ -73,9 +72,12 @@
return MemberDto.from(member);
}

if (request.email().isPresent() && !request.email().get().equals(member.getEmail())) {
throw new ValidationException("Email changes are not currently supported");
}

validateAndUpdate(request.firstName(), member::setFirstName, "firstName", true);
validateAndUpdate(request.lastName(), member::setLastName, "lastName", true);
validateAndUpdate(request.email(), member::setEmail, "email", true);
validateAndUpdate(request.introduction(), member::setIntroduction, "introduction", true);
validateAndUpdate(request.linkedInUrl(), member::setLinkedInUrl, "linkedInUrl", false);
validateAndUpdate(request.matchPref(), member::setMatchPref, "matchPref", false);
Expand All @@ -84,12 +86,13 @@
validateAndUpdate(request.topics(), member::setTopics, "topics", false);
validateAndUpdate(request.extraNotes(), member::setExtraNotes, "extraNotes", false);

try {
Member updatedMember = memberRepo.updateMember(member).orElseThrow(() -> new MemberNotFoundException(id));
return MemberDto.from(updatedMember);
} catch (DuplicateKeyException e) {
throw new MemberDuplicateException(member.getEmail());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why change this block? i get that we don't need it for now and we can enable later, but it leaves us prone to forgetting to add it back.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the only argument i could see for removing this is that we're not unnecessarily querying the DB here, but even then it should be commented out with a note to re enable when email verification is added later.

}
// Commented out duplicate check since email changes are not currently supported
// try {

Check warning on line 90 in src/main/java/org/patinanetwork/patchats/api/member/MemberService.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This block of commented-out lines of code should be removed.

See more on https://sonarcloud.io/project/issues?id=Patina-Network_patchats&issues=AaBJm9l8GqlOf_YDLNrM&open=AaBJm9l8GqlOf_YDLNrM&pullRequest=99
Member updatedMember = memberRepo.updateMember(member).orElseThrow(() -> new MemberNotFoundException(id));
return MemberDto.from(updatedMember);
// } catch (DuplicateKeyException e) {

Check warning on line 93 in src/main/java/org/patinanetwork/patchats/api/member/MemberService.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This block of commented-out lines of code should be removed.

See more on https://sonarcloud.io/project/issues?id=Patina-Network_patchats&issues=AaBJm9l8GqlOf_YDLNrN&open=AaBJm9l8GqlOf_YDLNrN&pullRequest=99
// throw new MemberDuplicateException(member.getEmail());
// }
}

private void validateAndUpdate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,30 +231,6 @@
.andExpect(jsonPath("$.payload.firstName").value("UpdatedFirstName"));
}

@Test
void updateMember_badRequestWhenBlankRequiredField() throws Exception {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

removed since this test is redundant with updateMember_badRequestWhenValidationFails

final UUID id = UUID.randomUUID();
final UpdateMemberRequest request = new UpdateMemberRequest(
Optional.of(""),
Optional.of("UpdatedLastName"),
Optional.of("updated@example.com"),
Optional.empty(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
Optional.empty());

when(memberService.updateMember(any(), any())).thenThrow(new ValidationException("firstName cannot be empty"));

mockMvc.perform(patch("/api/members/{id}", id)
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.message").value("firstName cannot be empty"));
}

@Test
void updateMember_notFoundWhenMemberDoesNotExist() throws Exception {
final UUID id = UUID.randomUUID();
Expand All @@ -280,30 +256,30 @@
.andExpect(jsonPath("$.success").value(false));
}

@Test
void updateMember_conflictWhenEmailIsDuplicate() throws Exception {
final UUID id = UUID.randomUUID();
final UpdateMemberRequest request = new UpdateMemberRequest(
Optional.empty(),
Optional.empty(),
Optional.of("existing@example.com"),
Optional.empty(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
Optional.empty());

when(memberService.updateMember(any(UpdateMemberRequest.class), eq(id)))
.thenThrow(new MemberDuplicateException("existing@example.com"));

mockMvc.perform(patch("/api/members/{id}", id)
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
.andExpect(status().isConflict())
.andExpect(jsonPath("$.success").value(false));
}
// @Test
// void updateMember_conflictWhenEmailIsDuplicate() throws Exception {

Check warning on line 260 in src/test/java/org/patinanetwork/patchats/api/member/MemberControllerTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This block of commented-out lines of code should be removed.

See more on https://sonarcloud.io/project/issues?id=Patina-Network_patchats&issues=AaBJqPr4ZC2NEC2OFGGT&open=AaBJqPr4ZC2NEC2OFGGT&pullRequest=99
// final UUID id = UUID.randomUUID();
// final UpdateMemberRequest request = new UpdateMemberRequest(
// Optional.empty(),
// Optional.empty(),
// Optional.of("existing@example.com"),
// Optional.empty(),
// Optional.empty(),
// Optional.empty(),
// Optional.empty(),
// Optional.empty(),
// Optional.empty(),
// Optional.empty());

// when(memberService.updateMember(any(UpdateMemberRequest.class), eq(id)))
// .thenThrow(new MemberDuplicateException("existing@example.com"));

Check warning on line 275 in src/test/java/org/patinanetwork/patchats/api/member/MemberControllerTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This block of commented-out lines of code should be removed.

See more on https://sonarcloud.io/project/issues?id=Patina-Network_patchats&issues=AaBJqPr4ZC2NEC2OFGGU&open=AaBJqPr4ZC2NEC2OFGGU&pullRequest=99

// mockMvc.perform(patch("/api/members/{id}", id)
// .contentType(MediaType.APPLICATION_JSON)
// .content(objectMapper.writeValueAsString(request)))
// .andExpect(status().isConflict())
// .andExpect(jsonPath("$.success").value(false));

Check warning on line 281 in src/test/java/org/patinanetwork/patchats/api/member/MemberControllerTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This block of commented-out lines of code should be removed.

See more on https://sonarcloud.io/project/issues?id=Patina-Network_patchats&issues=AaBJqPr4ZC2NEC2OFGGV&open=AaBJqPr4ZC2NEC2OFGGV&pullRequest=99
// }

@Test
void updateMember_badRequestWhenValidationFails() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.patinanetwork.patchats.api.member.dto.UpdateMemberRequest;
import org.patinanetwork.patchats.common.web.exception.MemberDuplicateException;
import org.patinanetwork.patchats.common.web.exception.MemberNotFoundException;
import org.springframework.dao.DuplicateKeyException;
import org.patinanetwork.patchats.common.web.exception.ValidationException;

class MemberServiceTest {

Expand Down Expand Up @@ -272,7 +272,6 @@

assertEquals("UpdatedFirstName", captured.getFirstName());
assertEquals("UpdatedLastName", captured.getLastName());
assertEquals("updated@example.com", captured.getEmail());
assertEquals("https://linkedin.com/in/updated", captured.getLinkedInUrl());
assertEquals("Updated intro", captured.getIntroduction());
assertEquals("Mentor - I am looking for guidance from someone with more experience", captured.getMatchPref());
Expand All @@ -282,13 +281,40 @@
assertEquals("Notes", captured.getExtraNotes());
}

// @Test
// void updateMember_throwsExceptionWhenEmailIsDuplicate() {

Check warning on line 285 in src/test/java/org/patinanetwork/patchats/api/member/MemberServiceTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This block of commented-out lines of code should be removed.

See more on https://sonarcloud.io/project/issues?id=Patina-Network_patchats&issues=AaBJqPojZC2NEC2OFGGR&open=AaBJqPojZC2NEC2OFGGR&pullRequest=99

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i just commented out the code, no idea why its not lining up correctly

// final UUID id = UUID.randomUUID();
// final UpdateMemberRequest request = new UpdateMemberRequest(
// Optional.empty(),
// Optional.empty(),
// Optional.of("existing@example.com"),
// Optional.empty(),
// Optional.empty(),
// Optional.empty(),
// Optional.empty(),
// Optional.empty(),
// Optional.empty(),
// Optional.empty());

// final Member existingMember = Member.builder()
// .id(id)
// .firstName("John")
// .lastName("Doe")
// .email("john@example.com")
// .build();

// when(memberRepo.updateMember(any(Member.class))).thenThrow(new DuplicateKeyException("Email already
// exists"));
// when(memberRepo.getMemberById(id)).thenReturn(Optional.of(existingMember));

Check warning on line 308 in src/test/java/org/patinanetwork/patchats/api/member/MemberServiceTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This block of commented-out lines of code should be removed.

See more on https://sonarcloud.io/project/issues?id=Patina-Network_patchats&issues=AaBJqPojZC2NEC2OFGGS&open=AaBJqPojZC2NEC2OFGGS&pullRequest=99
// assertThrows(MemberDuplicateException.class, () -> memberService.updateMember(request, id));
// }
@Test
void updateMember_throwsExceptionWhenEmailIsDuplicate() {
void updateMember_throwsValidationExceptionWhenEmailIsChanged() {
final UUID id = UUID.randomUUID();
final UpdateMemberRequest request = new UpdateMemberRequest(
Optional.empty(),
Optional.empty(),
Optional.of("existing@example.com"),
Optional.of("new@example.com"),
Optional.empty(),
Optional.empty(),
Optional.empty(),
Expand All @@ -304,16 +330,17 @@
.email("john@example.com")
.build();

when(memberRepo.updateMember(any(Member.class))).thenThrow(new DuplicateKeyException("Email already exists"));
when(memberRepo.getMemberById(id)).thenReturn(Optional.of(existingMember));
assertThrows(MemberDuplicateException.class, () -> memberService.updateMember(request, id));

assertThrows(ValidationException.class, () -> memberService.updateMember(request, id));
verify(memberRepo, never()).updateMember(any());
}

@Test
void updateMember_successWhenUpdatingWithSameEmail() {
void updateMember_successWhenUpdatingOtherFieldsWithUnchangedEmail() {
final UUID id = UUID.randomUUID();
final UpdateMemberRequest request = new UpdateMemberRequest(
Optional.empty(),
Optional.of("UpdatedFirstName"),
Optional.empty(),
Optional.of("john@example.com"),
Optional.empty(),
Expand All @@ -329,25 +356,19 @@
.firstName("John")
.lastName("Doe")
.email("john@example.com")
.linkedInUrl("https://linkedin.com/in/john")
.introduction("intro")
.matchPref("Mentor")
.industryPref("Tech")
.rolePref("Engineer")
.topics("AI")
.extraNotes("notes")
.build();

when(memberRepo.getMemberById(id)).thenReturn(Optional.of(existingMember));
when(memberRepo.updateMember(any())).thenReturn(Optional.of(existingMember));
when(memberRepo.updateMember(any(Member.class))).thenReturn(Optional.of(existingMember));

final ArgumentCaptor<Member> captor = ArgumentCaptor.forClass(Member.class);
memberService.updateMember(request, id);

verify(memberRepo).updateMember(captor.capture());
final Member captured = captor.getValue();

assertEquals("UpdatedFirstName", captured.getFirstName());
assertEquals("john@example.com", captured.getEmail());
verify(memberRepo, never()).getMemberByEmail(any());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ final class MemberTestFixtures {
static final UpdateMemberRequest UPDATE_REQUEST_ALL_FIELDS = new UpdateMemberRequest(
Optional.of("UpdatedFirstName"),
Optional.of("UpdatedLastName"),
Optional.of("updated@example.com"),
Optional.empty(), // Email changes are not currently supported

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is no representative of how the data actually comes in from the frontend, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

email changes are always disabled from the frontend so it will always be empty

Optional.of("https://linkedin.com/in/updated"),
Optional.of("Updated intro"),
Optional.of("Mentor - I am looking for guidance from someone with more experience"),
Expand Down
Loading