Skip to content

Commit 4b1cb1f

Browse files
authored
Update to API access request feature (#1120)
* Update to API access request feature - Do not auto approve when academic user requested API access - Include justification in slack channel - Show API Access Justification in User page - Allow admin to change API access justification in User page * update screenshot tests
1 parent ee3c78d commit 4b1cb1f

File tree

8 files changed

+407
-112
lines changed

8 files changed

+407
-112
lines changed
Loading
Loading

src/main/java/org/mskcc/cbio/oncokb/service/SlackService.java

+15-4
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.mskcc.cbio.oncokb.service.dto.UserDTO;
3434
import org.mskcc.cbio.oncokb.service.dto.UserMailsDTO;
3535
import org.mskcc.cbio.oncokb.service.dto.useradditionalinfo.AdditionalInfoDTO;
36+
import org.mskcc.cbio.oncokb.service.dto.useradditionalinfo.ApiAccessRequest;
3637
import org.mskcc.cbio.oncokb.service.mapper.UserMapper;
3738
import org.mskcc.cbio.oncokb.util.ObjectUtil;
3839
import org.mskcc.cbio.oncokb.util.StringUtil;
@@ -397,6 +398,20 @@ private List<LayoutBlock> buildWarningBlocks(UserDTO userDTO, Company company) {
397398
blocks.add(SectionBlock.builder().text(MarkdownTextObject.builder().text(text).build()).build());
398399
}
399400

401+
// Add API request info if exists
402+
Optional<Boolean> apiAccessRequestedOptional = Optional.ofNullable(userDTO)
403+
.map(UserDTO::getAdditionalInfo)
404+
.map(AdditionalInfoDTO::getApiAccessRequest)
405+
.map(ApiAccessRequest::isRequested);
406+
if (apiAccessRequestedOptional.isPresent() && apiAccessRequestedOptional.get()) {
407+
String apiRequestJustification = userDTO.getAdditionalInfo().getApiAccessRequest().getJustification();
408+
if (apiRequestJustification.isEmpty()) {
409+
apiRequestJustification = "[no justification provided]";
410+
}
411+
String text = ":information_source: *API Access Requested*: " + apiRequestJustification;
412+
blocks.add(SectionBlock.builder().text(MarkdownTextObject.builder().text(text).build()).build());
413+
}
414+
400415
return blocks;
401416
}
402417

@@ -405,10 +420,6 @@ private LayoutBlock buildCurrentLicense(UserDTO userDTO) {
405420
boolean isAcademicLicense = userDTO.getLicenseType().equals(LicenseType.ACADEMIC);
406421
sb.append("*" + userDTO.getLicenseType().getName() + "*" + (isAcademicLicense ? "" : " :clap:") +"\n");
407422

408-
boolean apiAccessRequested = userDTO.getAdditionalInfo() != null && userDTO.getAdditionalInfo().getApiAccessRequest() != null && userDTO.getAdditionalInfo().getApiAccessRequest().isRequested();
409-
if (isAcademicLicense && apiAccessRequested) {
410-
sb.append(":information_source: API Access\n");
411-
}
412423
if (StringUtils.isNotEmpty(userDTO.getCompanyName())) {
413424
sb.append("*" + userDTO.getCompanyName() + "*");
414425
}

src/main/java/org/mskcc/cbio/oncokb/service/UserService.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ public CompanyCandidate findCompanyCandidate(UserDTO userDTO) {
810810
return new CompanyCandidate(Optional.of(company), true);
811811
}
812812
} else if (companies.size() > 1) {
813-
// If there are mutliple companies with the domain, then we find a company with a regular license.
813+
// If there are multiple companies with the domain, then we find a company with a regular license.
814814
Optional<Company> foundCompany = companies
815815
.stream()
816816
.filter(c -> {

src/main/java/org/mskcc/cbio/oncokb/web/rest/AccountResource.java

+15-7
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
import org.mskcc.cbio.oncokb.domain.Token;
77
import org.mskcc.cbio.oncokb.domain.User;
88
import org.mskcc.cbio.oncokb.domain.enumeration.LicenseStatus;
9+
import org.mskcc.cbio.oncokb.domain.enumeration.LicenseType;
910
import org.mskcc.cbio.oncokb.repository.UserRepository;
10-
import org.mskcc.cbio.oncokb.security.AuthoritiesConstants;
1111
import org.mskcc.cbio.oncokb.security.SecurityUtils;
1212
import org.mskcc.cbio.oncokb.security.uuid.TokenProvider;
1313
import org.mskcc.cbio.oncokb.service.*;
@@ -197,13 +197,21 @@ private boolean activateUser(
197197
slackService.sendUserRegistrationToChannel(userDTO, userService.isUserOnTrial(userDTO), limitedCompany);
198198
} else {
199199
Company company = companyCandidate.getCompanyCandidate().get();
200-
userService.updateUserWithCompanyLicense(userDTO, company, false, false);
201-
// Don't send the automated approval message to slack if the company is on trial
202-
// We only want a message when the user accepts the trial license agreement.
203200
if (company.getLicenseStatus().equals(LicenseStatus.REGULAR)) {
204-
userService.approveUser(userDTO, false);
205-
slackService.sendApprovedConfirmation(userDTO, company);
206-
userIsActivated = true;
201+
Optional<Boolean> apiRequestedOptional = Optional.ofNullable(userDTO.getAdditionalInfo())
202+
.map(AdditionalInfoDTO::getApiAccessRequest)
203+
.map(ApiAccessRequest::isRequested);
204+
if (apiRequestedOptional.isPresent() && apiRequestedOptional.get() && LicenseType.ACADEMIC.equals(company.getLicenseType())) {
205+
// if user specifically asked for API access, we need to send the justification to user channel for approval
206+
slackService.sendUserRegistrationToChannel(userDTO, userService.isUserOnTrial(userDTO), company);
207+
} else {
208+
// Don't send the automated approval message to slack if the company is on trial
209+
// We only want a message when the user accepts the trial license agreement.
210+
userService.updateUserWithCompanyLicense(userDTO, company, false, false);
211+
userService.approveUser(userDTO, false);
212+
slackService.sendApprovedConfirmation(userDTO, company);
213+
userIsActivated = true;
214+
}
207215
}
208216
}
209217

src/main/webapp/app/config/constants.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,7 @@ export enum ACCOUNT_TITLES {
727727
API_TOKEN = 'API Token',
728728
LICENSE_TYPE = 'License',
729729
ADDITIONAL_INFO_USE_CASE = 'Use Case',
730+
ADDITIONAL_INFO_API_ACCESS_JUSTIFICATION = 'API Access Justification',
730731
}
731732

732733
export enum API_CALL_STATUS {

src/main/webapp/app/pages/userPage/UserPage.tsx

+35-1
Original file line numberDiff line numberDiff line change
@@ -412,13 +412,30 @@ export default class UserPage extends React.Component<IUserPage> {
412412
...this.user.additionalInfo,
413413
userCompany: updatedUserCompany,
414414
};
415-
if (!values.authorities.includes(AUTHORITIES.API)) {
415+
416+
// If previously the user has API role, but in the latest, it's removed, then we will clean up the api access request
417+
if (
418+
this.user.authorities.includes(AUTHORITIES.API) &&
419+
!values.authorities.includes(AUTHORITIES.API)
420+
) {
416421
updatedAdditionalInfo.apiAccessRequest = {
417422
requested: false,
418423
justification: '',
419424
};
420425
}
421426

427+
if (values.additionalInfoApiAccessJustification) {
428+
if (updatedAdditionalInfo.apiAccessRequest) {
429+
updatedAdditionalInfo.apiAccessRequest.justification =
430+
values.additionalInfoApiAccessJustification;
431+
} else {
432+
updatedAdditionalInfo.apiAccessRequest = {
433+
requested: true,
434+
justification: values.additionalInfoApiAccessJustification,
435+
};
436+
}
437+
}
438+
422439
const updatedUser: UserDTO = {
423440
...this.user,
424441
firstName: values.firstName,
@@ -917,6 +934,23 @@ export default class UserPage extends React.Component<IUserPage> {
917934
type={'textarea'}
918935
validate={LONG_TEXT_VAL}
919936
/>
937+
<AvField
938+
name="additionalInfoApiAccessJustification"
939+
label={
940+
<BoldAccountTitle
941+
title={
942+
ACCOUNT_TITLES.ADDITIONAL_INFO_API_ACCESS_JUSTIFICATION
943+
}
944+
/>
945+
}
946+
value={
947+
this.user.additionalInfo?.apiAccessRequest
948+
?.justification
949+
}
950+
rows={3}
951+
type={'textarea'}
952+
validate={LONG_TEXT_VAL}
953+
/>
920954
<AvField
921955
name="city"
922956
label={

0 commit comments

Comments
 (0)