Skip to content

Commit

Permalink
Merge pull request #2551 from objectcomputing/bugfix-2549/reduce-emai…
Browse files Browse the repository at this point in the history 
…ls-for-guild-changes

Limit emails sent by GuildMemberServices
  • Loading branch information
@mkimberlin
mkimberlin authored Aug 6, 2024
2 parents b45d660 + 3ca2aac commit df3f1cf
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 26 deletions.
4 changes: 2 additions & 2 deletions server/src/main/java/com/objectcomputing/checkins/services/guild/GuildServicesImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public GuildResponseDTO update(GuildUpdateDTO guildDTO) {
Optional<GuildMember> first = existingGuildMembers.stream().filter(existing -> existing.getMemberId().equals(updatedMember.getMemberId())).findFirst();
MemberProfile existingMember = memberProfileServices.getById(updatedMember.getMemberId());
if(first.isEmpty()) {
newMembers.add(fromMemberEntity(guildMemberServices.save(fromMemberDTO(updatedMember, newGuildEntity.getId())), existingMember));
newMembers.add(fromMemberEntity(guildMemberServices.save(fromMemberDTO(updatedMember, newGuildEntity.getId()), false), existingMember));
addedMembers.add(existingMember);
} else {
newMembers.add(fromMemberEntity(guildMemberServices.update(fromMemberDTO(updatedMember, newGuildEntity.getId())), existingMember));
Expand All @@ -182,7 +182,7 @@ public GuildResponseDTO update(GuildUpdateDTO guildDTO) {
//delete any removed members from guild
existingGuildMembers.forEach(existingMember -> {
if(!guildDTO.getGuildMembers().stream().filter(updatedTeamMember -> updatedTeamMember.getMemberId().equals(existingMember.getMemberId())).findFirst().isPresent()) {
guildMemberServices.delete(existingMember.getId());
guildMemberServices.delete(existingMember.getId(), false);
removedMembers.add(memberProfileServices.getById(existingMember.getMemberId()));
}
});
Expand Down
8 changes: 5 additions & 3 deletions ...c/main/java/com/objectcomputing/checkins/services/guild/member/GuildMemberController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ public GuildMemberController(GuildMemberServices guildMemberServices) {
@Post
public HttpResponse<GuildMember> createMembers(@Body @Valid GuildMemberCreateDTO guildMember,
HttpRequest<?> request) {
GuildMember newGuildMember = guildMemberServices.save(new GuildMember(guildMember.getGuildId(),
guildMember.getMemberId(), guildMember.getLead()));
GuildMember newGuildMember = guildMemberServices.save(
new GuildMember(guildMember.getGuildId(), guildMember.getMemberId(), guildMember.getLead()),
true
);
return HttpResponse
.created(newGuildMember)
.headers(headers -> headers.location(
Expand Down Expand Up @@ -95,7 +97,7 @@ public Set<GuildMember> findGuildMembers(@Nullable UUID guildid,
*/
@Delete("/{id}")
public HttpResponse<?> deleteGuildMember(@NotNull UUID id) {
guildMemberServices.delete(id);
guildMemberServices.delete(id, true);
return HttpResponse
.ok();
}
Expand Down
4 changes: 2 additions & 2 deletions ...src/main/java/com/objectcomputing/checkins/services/guild/member/GuildMemberServices.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

public interface GuildMemberServices {

GuildMember save(GuildMember guildMember);
GuildMember save(GuildMember guildMember, boolean sendEmail);

GuildMember read(UUID id);

GuildMember update(GuildMember guildMember);

void delete(UUID id);
void delete(UUID id, boolean sendEmail);

Set<GuildMember> findByFields(UUID guildid, UUID memberid, Boolean lead);
}
28 changes: 16 additions & 12 deletions ...main/java/com/objectcomputing/checkins/services/guild/member/GuildMemberServicesImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void setEmailSender(EmailSender emailSender) {
this.emailSender = emailSender;
}

public GuildMember save(@Valid @NotNull GuildMember guildMember) {
public GuildMember save(@Valid @NotNull GuildMember guildMember, boolean sendEmail) {
final UUID guildId = guildMember.getGuildId();
final UUID memberId = guildMember.getMemberId();
Optional<Guild> guild = guildRepo.findById(guildId);
Expand All @@ -84,11 +84,13 @@ else if (!currentUserServices.isAdmin() && !guildMember.getMemberId().equals(cur
throw new PermissionException(NOT_AUTHORIZED_MSG);
}

emailSender
.sendEmail(null, null, "Membership changes have been made to the " + guild.get().getName() + " guild",
constructEmailContent(guildMember, true),
getGuildLeadsEmails(guildLeads, guildMember).toArray(new String[0])
);
if (sendEmail) {
emailSender
.sendEmail(null, null, "Membership changes have been made to the " + guild.get().getName() + " guild",
constructEmailContent(guildMember, true),
getGuildLeadsEmails(guildLeads, guildMember).toArray(new String[0])
);
}

GuildMember guildMemberSaved = guildMemberRepo.save(guildMember);
guildMemberHistoryRepository.save(buildGuildMemberHistory(guildId,memberId,"Added", LocalDateTime.now()));
Expand Down Expand Up @@ -145,7 +147,7 @@ public Set<GuildMember> findByFields(@Nullable UUID guildId, @Nullable UUID memb
return guildMembers;
}

public void delete(@NotNull UUID id) {
public void delete(@NotNull UUID id, boolean sendEmail) {
MemberProfile currentUser = currentUserServices.getCurrentUser();
GuildMember guildMember = guildMemberRepo.findById(id).orElse(null);

Expand All @@ -166,11 +168,13 @@ public void delete(@NotNull UUID id) {
Guild guild = guildRepo.findById(guildMember.getGuildId())
.orElseThrow(() -> new NotFoundException("No Guild found with id " + guildMember.getGuildId()));

emailSender
.sendEmail(null, null, "Membership Changes have been made to the " + guild.getName() + " guild",
constructEmailContent(guildMember, false),
getGuildLeadsEmails(guildLeads, guildMember).toArray(new String[0])
);
if (sendEmail) {
emailSender
.sendEmail(null, null, "Membership Changes have been made to the " + guild.getName() + " guild",
constructEmailContent(guildMember, false),
getGuildLeadsEmails(guildLeads, guildMember).toArray(new String[0])
);
}

guildMemberRepo.deleteById(id);
guildMemberHistoryRepository.save(buildGuildMemberHistory(guildMember.getGuildId(),guildMember.getMemberId(),"Deleted", LocalDateTime.now()));
Expand Down
11 changes: 4 additions & 7 deletions server/src/test/java/com/objectcomputing/checkins/services/guild/GuildControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,9 @@ void testEmailSentToGuildLeadWhenGuildMembersAdded() {
final HttpRequest<GuildUpdateDTO> request = HttpRequest.PUT("/", requestBody).basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE);
client.toBlocking().exchange(request, GuildResponseDTO.class);

assertEquals(2, emailSender.events.size());
assertEquals(1, emailSender.events.size());
assertEquals(
List.of(
// Email to the Guild lead
List.of("SEND_EMAIL", "null", "null", "Membership changes have been made to the Ninja guild", "<h3>Bill Charles has joined the Ninja guild.</h3><a href=\"https://checkins.objectcomputing.com/guilds\">Click here</a> to view the changes in the Check-Ins app.", memberProfile.getWorkEmail() + "," + memberProfile2.getWorkEmail()),
// Email to both the Guild leads
List.of("SEND_EMAIL", "null", "null", "Membership Changes have been made to the Ninja guild", "<h3>Changes have been made to the Ninja guild.</h3><h4>The following members have been added:</h4><ul><li>Bill Charles</li></ul><a href=\"https://checkins.objectcomputing.com/guilds\">Click here</a> to view the changes in the Check-Ins app.", memberProfile.getWorkEmail() + "," + memberProfile2.getWorkEmail())
),
Expand Down Expand Up @@ -132,9 +130,8 @@ void testEmailSentToGuildLeadWhenGuildMembersRemoved() {
final HttpRequest<GuildUpdateDTO> request = HttpRequest.PUT("/", requestBody).basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE);
client.toBlocking().exchange(request, GuildResponseDTO.class);

assertEquals(2, emailSender.events.size());
assertEquals(1, emailSender.events.size());
assertEquals(List.of(
List.of("SEND_EMAIL", "null", "null", "Membership Changes have been made to the Ninja guild", "<h3>Bill Charles has left the Ninja guild.</h3><a href=\"https://checkins.objectcomputing.com/guilds\">Click here</a> to view the changes in the Check-Ins app.", memberProfile.getWorkEmail()),
List.of("SEND_EMAIL", "null", "null", "Membership Changes have been made to the Ninja guild", "<h3>Changes have been made to the Ninja guild.</h3><h4>The following members have been removed:</h4><ul><li>Bill Charles</li></ul><a href=\"https://checkins.objectcomputing.com/guilds\">Click here</a> to view the changes in the Check-Ins app.", memberProfile.getWorkEmail())
),
emailSender.events
Expand Down Expand Up @@ -488,7 +485,7 @@ void deleteGuildByAdmin() {
createDefaultGuildMember(guildEntity, memberProfileOfAdmin);

final MutableHttpRequest<?> request = HttpRequest.DELETE(String.format("/%s", guildEntity.getId())).basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE);
final HttpResponse response = client.toBlocking().exchange(request);
final HttpResponse<?> response = client.toBlocking().exchange(request);

assertEquals(HttpStatus.OK, response.getStatus());
}
Expand All @@ -504,7 +501,7 @@ void deleteGuildByGuildLead() {
// createDefaultGuildMember(guild, memberProfileOfGuildMember);

final MutableHttpRequest<?> request = HttpRequest.DELETE(String.format("/%s", guildEntity.getId())).basicAuth(memberProfileofGuildLeadEntity.getWorkEmail(), MEMBER_ROLE);
final HttpResponse response = client.toBlocking().exchange(request);
final HttpResponse<?> response = client.toBlocking().exchange(request);

assertEquals(HttpStatus.OK, response.getStatus());
}
Expand Down

0 comments on commit df3f1cf

Please sign in to comment.