Skip to content

Commit

Permalink
Merge pull request #1040 from oncokb/slack-warn-duplicate-user
Browse files Browse the repository at this point in the history
added slack duplicate user warning
  • Loading branch information
bprize15 authored Oct 26, 2023
2 parents 1a5adc3 + aa63f13 commit 0ff9f5c
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 6 deletions.
7 changes: 5 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@
</dependencyManagement>

<dependencies>

<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-recaptchaenterprise</artifactId>
Expand Down Expand Up @@ -398,7 +397,11 @@
<artifactId>bucket4j-core</artifactId>
<version>${bucket4j.version}</version>
</dependency>

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-text</artifactId>
<version>1.10.0</version>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import org.springframework.data.jpa.repository.*;
import org.springframework.stereotype.Repository;

import io.github.jhipster.config.JHipsterProperties.Mail;

import java.time.Instant;
import java.util.List;

Expand All @@ -24,5 +26,7 @@ public interface UserMailsRepository extends JpaRepository<UserMails, Long> {

List<UserMails> findUserMailsByUserAndMailTypeAndSentDateAfter(User user, MailType mailType, Instant sentAfter);

List<UserMails> findUserMailByUserAndMailTypeIn(User user, List<MailType> mailTypes);

void deleteAllByUser(User user);
}
35 changes: 33 additions & 2 deletions src/main/java/org/mskcc/cbio/oncokb/service/SlackService.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,21 @@
import org.mskcc.cbio.oncokb.config.application.ApplicationProperties;
import org.mskcc.cbio.oncokb.domain.Company;
import org.mskcc.cbio.oncokb.domain.UserIdMessagePair;
import org.mskcc.cbio.oncokb.domain.UserMails;
import org.mskcc.cbio.oncokb.domain.enumeration.*;
import org.mskcc.cbio.oncokb.domain.enumeration.slack.*;
import org.mskcc.cbio.oncokb.service.dto.UserDTO;
import org.mskcc.cbio.oncokb.service.dto.UserMailsDTO;
import org.mskcc.cbio.oncokb.service.dto.useradditionalinfo.AdditionalInfoDTO;
import org.mskcc.cbio.oncokb.service.mapper.UserMapper;
import org.mskcc.cbio.oncokb.util.ObjectUtil;
import org.mskcc.cbio.oncokb.util.StringUtil;
import org.mskcc.cbio.oncokb.web.rest.slack.ActionId;
import org.mskcc.cbio.oncokb.web.rest.slack.BlockId;
import org.mskcc.cbio.oncokb.web.rest.slack.DropdownEmailOption;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.annotation.Lazy;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Service;
import org.thymeleaf.context.Context;
Expand All @@ -45,7 +49,6 @@
import java.nio.file.Paths;
import java.time.Instant;
import java.util.*;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand All @@ -72,14 +75,16 @@ public class SlackService {
private final ApplicationProperties applicationProperties;
private final MailService mailService;
private final EmailService emailService;
private final UserService userService;
private final UserMailsService userMailsService;
private final UserMapper userMapper;
private final Slack slack;

public SlackService(ApplicationProperties applicationProperties, MailService mailService, EmailService emailService, UserMailsService userMailsService, UserMapper userMapper, Slack slack) {
public SlackService(ApplicationProperties applicationProperties, MailService mailService, EmailService emailService, @Lazy UserService userService, UserMailsService userMailsService, UserMapper userMapper, Slack slack) {
this.applicationProperties = applicationProperties;
this.mailService = mailService;
this.emailService = emailService;
this.userService = userService;
this.userMailsService = userMailsService;
this.userMapper = userMapper;
this.slack = slack;
Expand Down Expand Up @@ -537,6 +542,25 @@ private List<LayoutBlock> buildAdditionalInfoBlocks(UserDTO userDTO, boolean tri
}
}

List<UserDTO> potentialDuplicateUsers = userService.getPotentialDuplicateAccountsByUser(userDTO);
if (!potentialDuplicateUsers.isEmpty()) {
StringBuilder sb = new StringBuilder(":warning: *This user may have already registered. A list of previously registered users:*");
for (UserDTO user : potentialDuplicateUsers) {
List<MailType> rejectionMailTypes = new ArrayList<>(Arrays.asList(MailType.REJECTION_US_SANCTION, MailType.REJECT_ALUMNI_ADDRESS, MailType.REJECTION));
List<UserMailsDTO> rejectionUserMails = userMailsService.findUserMailsByUserAndMailTypeIn(userMapper.userDTOToUser(user), rejectionMailTypes);

sb.append("\n\u2022 ");
sb.append(StringUtil.getFullName(user.getFirstName(), user.getLastName()));
sb.append(", <https://www.oncokb.org/users/" + user.getEmail() + "/|" + user.getEmail() + ">");
sb.append(", " + user.getCompanyName());
sb.append(", " + user.getCity());
sb.append(", " + user.getCountry());
if (!rejectionUserMails.isEmpty()) {
sb.append(", *REJECTED*");
}
}
layoutBlocks.add(buildMarkdownBlock(sb.toString(), DUPLICATE_USER_CLARIFICATION_NOTE));
}
return layoutBlocks;
}

Expand Down Expand Up @@ -742,6 +766,13 @@ private LayoutBlock buildPlainTextBlock(String text, BlockId blockId) {
return null;
}

private LayoutBlock buildMarkdownBlock(String text, BlockId blockId) {
if (text != null && blockId != null) {
return SectionBlock.builder().text(MarkdownTextObject.builder().text(text).build()).blockId(blockId.getId()).build();
}
return null;
}

private String getStringFromResourceTemplateMailTextFile(String fileName) {
StringBuilder sb = new StringBuilder();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ public List<UserMailsDTO> findUserMailsByUserAndMailTypeAndSentDateAfter(User us
.collect(Collectors.toCollection(LinkedList::new));
}

@Transactional(readOnly = true)
public List<UserMailsDTO> findUserMailsByUserAndMailTypeIn(User user, List<MailType> mailTypes) {
log.debug("Request to get all UserMails in a list of mail types for a particular user");

return userMailsRepository.findUserMailByUserAndMailTypeIn(user, mailTypes).stream()
.map(userMailsMapper::toDto)
.collect(Collectors.toCollection(LinkedList::new));
}

/**
* Get one userMails by id.
*
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/org/mskcc/cbio/oncokb/service/UserService.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.google.gson.Gson;
import io.github.jhipster.config.JHipsterProperties;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.text.similarity.JaroWinklerSimilarity;
import org.mskcc.cbio.oncokb.config.Constants;
import org.mskcc.cbio.oncokb.config.cache.CacheNameResolver;
import org.mskcc.cbio.oncokb.domain.*;
Expand Down Expand Up @@ -667,6 +668,23 @@ public boolean userHasAuthority(User user, String authority) {
return user.getAuthorities().stream().filter(userAuth -> userAuth.getName().equalsIgnoreCase(authority)).count() > 0;
}

public List<UserDTO> getPotentialDuplicateAccountsByUser(UserDTO user) {
return searchAccountsForPotentialDuplicateUser(user, getAllManagedUsers());
}

public List<UserDTO> searchAccountsForPotentialDuplicateUser(UserDTO user, List<UserDTO> allUsers) {
String userFullName = StringUtil.getFullName(user.getFirstName(), user.getLastName());
JaroWinklerSimilarity jw = new JaroWinklerSimilarity();
List<UserDTO> potentialDuplicateUsers = new ArrayList<>();
for (UserDTO potentialDuplicate : allUsers) {
String potentialDuplicateFullName = StringUtil.getFullName(potentialDuplicate.getFirstName(), potentialDuplicate.getLastName());
if (user.getId() != potentialDuplicate.getId() && jw.apply(userFullName, potentialDuplicateFullName) > .7) {
potentialDuplicateUsers.add(potentialDuplicate);
}
}
return potentialDuplicateUsers;
}

/**
* Gets a list of all the authorities.
*
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/mskcc/cbio/oncokb/util/StringUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,8 @@ public class StringUtil {
public static String getEmailDomain(String email) {
return email.substring(email.indexOf("@") + 1);
}

public static String getFullName(String firstName, String lastName) {
return firstName + " " + lastName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ class SlackServiceIT {
@Autowired
private EmailService emailService;

@Autowired
private UserService userService;

@Autowired
private UserMailsService userMailsService;

Expand Down Expand Up @@ -72,7 +75,7 @@ public void setup() throws IOException {
applicationProperties.setSlack(new SlackProperties());
applicationProperties.getSlack().setUserRegistrationWebhook(USER_REGISTRATION_WEBHOOK);

slackService = new SlackService(applicationProperties, mailService, emailService, userMailsService, userMapper, slack);
slackService = new SlackService(applicationProperties, mailService, emailService, userService, userMailsService, userMapper, slack);
}

@Test
Expand Down
57 changes: 57 additions & 0 deletions src/test/java/org/mskcc/cbio/oncokb/service/UserServiceIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@
import org.springframework.data.domain.PageRequest;
import org.springframework.transaction.annotation.Transactional;

import com.mysql.cj.conf.ConnectionUrlParser.Pair;

import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
Expand Down Expand Up @@ -374,4 +378,57 @@ public void assertThatUserTokenStatusIsExpected() {
assertThat(tokenService.findByUser(savedUser).get(0).isRenewable()).isTrue();
}

//UNIT TESTS
@Test
public void assertThatDuplicateUsersAreCaught() {
long id = -1;

UserDTO user = new UserDTO();
user.setFirstName("Jon");
user.setLastName("Doe");
user.setId(id);

List<Pair<String, String>> similarNames = Arrays.asList(
new Pair<>("Jon", "Doh"),
new Pair<>("Joon", "Doe"),
new Pair<>("Jonny", "Doe"),
new Pair<>("Jonathon", "Doe"),
new Pair<>("Jon", "Doel"),
new Pair<>("Jonny", "Dole")
);
List<Pair<String, String>> dissimilarNames = Arrays.asList(
new Pair<>("Mark", "Thompson"),
new Pair<>("Emily", "Davis"),
new Pair<>("William", "Brown"),
new Pair<>("Jessica", "White"),
new Pair<>("Christopher", "Lee")
);

List<UserDTO> allUsers = new ArrayList<>();

List<UserDTO> similarUsers = new ArrayList<>();
for (Pair<String, String> similarName : similarNames) {
UserDTO newUser = new UserDTO();
newUser.setFirstName(similarName.left);
newUser.setLastName(similarName.right);
newUser.setId(++id);
similarUsers.add(newUser);

allUsers.add(newUser);
}

List<UserDTO> dissimilarUsers = new ArrayList<>();
for (Pair<String, String> dissimilarName : dissimilarNames) {
UserDTO newUser = new UserDTO();
newUser.setFirstName(dissimilarName.left);
newUser.setLastName(dissimilarName.right);
newUser.setId(++id);
dissimilarUsers.add(newUser);

allUsers.add(newUser);
}

List<UserDTO> duplicateUsers = userService.searchAccountsForPotentialDuplicateUser(user, allUsers);
assertThat(duplicateUsers).containsExactlyInAnyOrder(similarUsers.toArray(new UserDTO[similarUsers.size()]));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ void setUp() throws IOException, SlackApiException {

// Inject mock dependencies
mailService = new MailService(jHipsterProperties, javaMailSender, messageSource, templateEngine, userMailsService, applicationProperties);
slackService = new SlackService(applicationProperties, mailService, emailService, userMailsService, userMapper, slack);
slackService = new SlackService(applicationProperties, mailService, emailService, userService, userMailsService, userMapper, slack);
slackController = new SlackController(userService, userRepository, mailService, slackService, userMapper);

/******************************
Expand Down

0 comments on commit 0ff9f5c

Please sign in to comment.