From d4fd70bed7f997af7104eb0b150a125d6283bc4f Mon Sep 17 00:00:00 2001 From: John Konecny <24961694+jfkonecn@users.noreply.github.com> Date: Tue, 23 Apr 2024 10:01:35 -0400 Subject: [PATCH] Slack reports error and limits justification section --- .../cbio/oncokb/service/SlackService.java | 24 +++++++--- .../newAccountForm/NewAccountForm.tsx | 6 ++- .../cbio/oncokb/service/SlackServiceIT.java | 45 +++++++++++++++++-- .../oncokb/web/rest/SlackControllerIT.java | 5 ++- 4 files changed, 69 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/mskcc/cbio/oncokb/service/SlackService.java b/src/main/java/org/mskcc/cbio/oncokb/service/SlackService.java index 15cc47ff3..5b7909c0d 100644 --- a/src/main/java/org/mskcc/cbio/oncokb/service/SlackService.java +++ b/src/main/java/org/mskcc/cbio/oncokb/service/SlackService.java @@ -1,5 +1,6 @@ package org.mskcc.cbio.oncokb.service; +import com.google.gson.Gson; import com.slack.api.Slack; import com.slack.api.app_backend.interactive_components.payload.BlockActionPayload; import com.slack.api.app_backend.views.payload.ViewSubmissionPayload; @@ -17,8 +18,12 @@ import com.slack.api.model.view.View; import com.slack.api.model.view.ViewSubmit; import com.slack.api.model.view.ViewTitle; +import com.slack.api.util.json.GsonFactory; import com.slack.api.webhook.Payload; import com.slack.api.webhook.WebhookResponse; + +import io.sentry.SentryLevel; + import org.apache.commons.lang3.StringUtils; import org.mskcc.cbio.oncokb.config.application.ApplicationProperties; import org.mskcc.cbio.oncokb.domain.Company; @@ -73,20 +78,26 @@ 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, @Lazy UserService userService, UserMailsService userMailsService, UserMapper userMapper, Slack slack) { + private final SentryService sentryService; + + public SlackService(ApplicationProperties applicationProperties, + MailService mailService, + @Lazy UserService userService, + UserMailsService userMailsService, + UserMapper userMapper, + Slack slack, + SentryService sentryService) { this.applicationProperties = applicationProperties; this.mailService = mailService; - this.emailService = emailService; this.userService = userService; this.userMailsService = userMailsService; this.userMapper = userMapper; this.slack = slack; + this.sentryService = sentryService; } @Async @@ -260,9 +271,12 @@ private void sendBlocks(String url, List layoutBlocks) { log.info("Send the latest user blocks to slack with response code " + response.getCode()); if (!Integer.valueOf(200).equals(response.getCode())) { log.error("Getting a response code other than 200, {}", response); + String payloadStr = GsonFactory.createSnakeCase().toJson(payload); + String sentryMessage = String.format("Non-200 response from slack %s. Sent to \"%s\"\n\n%s", response.getCode(), url, payloadStr); + this.sentryService.throwMessage(SentryLevel.ERROR, sentryMessage, null); } } catch (Exception e) { - log.error("Failed to send message to slack {}", e); + log.error("Failed to send message to slack {}", e.toString()); } } diff --git a/src/main/webapp/app/components/newAccountForm/NewAccountForm.tsx b/src/main/webapp/app/components/newAccountForm/NewAccountForm.tsx index c005f217e..ddf38c50e 100644 --- a/src/main/webapp/app/components/newAccountForm/NewAccountForm.tsx +++ b/src/main/webapp/app/components/newAccountForm/NewAccountForm.tsx @@ -44,6 +44,7 @@ import { SHORT_TEXT_VAL, TEXT_VAL, OPTIONAL_TEXT_VAL, + textValidation, } from 'app/shared/utils/FormValidationUtils'; import { NOT_USED_IN_AI_MODELS } from 'app/config/constants/terms'; @@ -82,6 +83,8 @@ type CompanySelectOptionType = { value: CompanyDTO; }; +export const SLACK_TEXT_VAL = textValidation(2, 1900); + export const ACCOUNT_TYPE_DEFAULT = AccountType.REGULAR; @observer export class NewAccountForm extends React.Component { @@ -641,6 +644,7 @@ export class NewAccountForm extends React.Component { type={'textarea'} placeholder={this.companyDescriptionPlaceholder} rows={4} + validate={...SLACK_TEXT_VAL} /> {this.isCommercialLicense && ( <> @@ -669,7 +673,7 @@ export class NewAccountForm extends React.Component { placeholder={this.useCasePlaceholder} rows={6} validate={{ - ...LONG_TEXT_VAL, + ...SLACK_TEXT_VAL, required: { value: true, errorMessage: 'Your use case is required.', diff --git a/src/test/java/org/mskcc/cbio/oncokb/service/SlackServiceIT.java b/src/test/java/org/mskcc/cbio/oncokb/service/SlackServiceIT.java index 52005b2c6..1ca2bbb75 100644 --- a/src/test/java/org/mskcc/cbio/oncokb/service/SlackServiceIT.java +++ b/src/test/java/org/mskcc/cbio/oncokb/service/SlackServiceIT.java @@ -5,6 +5,10 @@ import com.slack.api.model.block.SectionBlock; import com.slack.api.model.block.composition.TextObject; import com.slack.api.webhook.Payload; +import com.slack.api.webhook.WebhookResponse; + +import io.sentry.SentryLevel; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -23,6 +27,7 @@ import org.springframework.boot.test.context.SpringBootTest; import java.io.IOException; +import java.util.HashMap; import java.util.HashSet; import java.util.Set; @@ -42,9 +47,6 @@ class SlackServiceIT { @Autowired private MailService mailService; - @Autowired - private EmailService emailService; - @Autowired private UserService userService; @@ -57,6 +59,9 @@ class SlackServiceIT { @Spy private Slack slack; + @Spy + private SentryService sentryService; + @Captor private ArgumentCaptor urlCaptor; @@ -75,13 +80,21 @@ public void setup() throws IOException { applicationProperties.setSlack(new SlackProperties()); applicationProperties.getSlack().setUserRegistrationWebhook(USER_REGISTRATION_WEBHOOK); - slackService = new SlackService(applicationProperties, mailService, emailService, userService, userMailsService, userMapper, slack); + slackService = new SlackService(applicationProperties, mailService, userService, userMailsService, userMapper, slack, sentryService); + } + + private void setMockResponse(Integer code) throws IOException { + WebhookResponse response = WebhookResponse.builder() + .code(code) + .build(); + doReturn(response).when(slack).send(any(String.class), any(Payload.class)); } @Test void testSendUserRegistrationToChannel() throws IOException { // Create mock user UserDTO user = new UserDTO(); + setMockResponse(200); user.setId(0L); user.setEmail("john.doe@example.com"); user.setFirstName("john"); @@ -131,4 +144,28 @@ void testSendUserRegistrationToChannel() throws IOException { } assertThat(expectedValues).isEmpty(); } + + @Test + void testSend400Response() throws IOException { + // Create mock user + UserDTO user = new UserDTO(); + setMockResponse(400); + user.setId(0L); + user.setEmail("john.doe@example.com"); + user.setFirstName("john"); + user.setLastName("doe"); + user.setJobTitle("job title"); + user.setCompanyName("company name"); + user.setCity("city"); + user.setCountry("country"); + user.setLicenseType(LicenseType.COMMERCIAL); + Company company = new Company(); + company.setName("company name"); + company.setLicenseType(LicenseType.COMMERCIAL); + company.setLicenseStatus(LicenseStatus.UNKNOWN); + + slackService.sendUserRegistrationToChannel(user, false, company); + verify(slack).send(urlCaptor.capture(), payloadCaptor.capture()); + verify(sentryService).throwMessage(eq(SentryLevel.ERROR), anyString(), isNull()); + } } diff --git a/src/test/java/org/mskcc/cbio/oncokb/web/rest/SlackControllerIT.java b/src/test/java/org/mskcc/cbio/oncokb/web/rest/SlackControllerIT.java index ab70ae337..3b4dc0c61 100644 --- a/src/test/java/org/mskcc/cbio/oncokb/web/rest/SlackControllerIT.java +++ b/src/test/java/org/mskcc/cbio/oncokb/web/rest/SlackControllerIT.java @@ -119,6 +119,9 @@ public class SlackControllerIT { @Spy private Slack slack; + @Spy + private SentryService sentryService; + @Spy private MethodsClient methodsClient; @@ -181,7 +184,7 @@ void setUp() throws IOException, SlackApiException { // Inject mock dependencies mailService = new MailService(jHipsterProperties, javaMailSender, messageSource, templateEngine, userMailsService, applicationProperties); - slackService = new SlackService(applicationProperties, mailService, emailService, userService, userMailsService, userMapper, slack); + slackService = new SlackService(applicationProperties, mailService, userService, userMailsService, userMapper, slack, sentryService); slackController = new SlackController(userService, userRepository, mailService, slackService, userMapper); /******************************