From 1506ccd68208ad8f1c5e280b4ad42f1a6c79a0ab Mon Sep 17 00:00:00 2001 From: Nick Angelou Date: Mon, 14 Dec 2020 13:25:47 -0600 Subject: [PATCH] Add Jira labels (#2268) * feat: add custom labels * graphql * feat: working Jira labels TODO: tests * mage gen fmt * fix: tests * fix: remove comments * fix: tests * fix: pr feedback Co-authored-by: panther-bot --- api/graphql/schema.graphql | 2 ++ api/lambda/outputs/models/api.go | 13 +++---- internal/core/alert_delivery/outputs/jira.go | 3 +- .../core/alert_delivery/outputs/jira_test.go | 5 +-- internal/core/outputs_api/api/utils.go | 4 +-- web/__generated__/schema.tsx | 3 ++ web/__tests__/__mocks__/builders.generated.ts | 2 ++ .../DestinationFormSwitcher.tsx | 1 + .../JiraDestinationForm.test.tsx | 32 +++++++++++++++-- .../JiraDestinationForm.tsx | 36 ++++++++++++++----- .../ConfigureDestinationPanel.tsx | 1 + .../fragments/DestinationFull.generated.ts | 3 +- .../graphql/fragments/DestinationFull.graphql | 1 + web/src/helpers/utils.tsx | 6 ++++ .../CreateDestination.test.tsx | 9 +++++ .../EditDestination/EditDestination.test.tsx | 1 + .../DestinationCards/JiraDestinationCard.tsx | 4 +++ .../__tests__/JiraDestinationCard.test.tsx | 1 + 18 files changed, 104 insertions(+), 23 deletions(-) diff --git a/api/graphql/schema.graphql b/api/graphql/schema.graphql index f8ed5b6eca..4c1b38fada 100644 --- a/api/graphql/schema.graphql +++ b/api/graphql/schema.graphql @@ -694,6 +694,7 @@ type JiraConfig { apiKey: String! assigneeId: String issueType: String! + labels: [String!]! } type AsanaConfig { @@ -763,6 +764,7 @@ input JiraConfigInput { apiKey: String! assigneeId: String issueType: String! + labels: [String!] } input AsanaConfigInput { diff --git a/api/lambda/outputs/models/api.go b/api/lambda/outputs/models/api.go index 5f397afddc..cea8923b5c 100644 --- a/api/lambda/outputs/models/api.go +++ b/api/lambda/outputs/models/api.go @@ -219,12 +219,13 @@ type GithubConfig struct { // JiraConfig defines options for each Jira output type JiraConfig struct { - OrgDomain string `json:"orgDomain"` - ProjectKey string `json:"projectKey"` - UserName string `json:"userName"` - APIKey string `json:"apiKey"` - AssigneeID string `json:"assigneeId"` - Type string `json:"issueType"` + OrgDomain string `json:"orgDomain" validate:"url"` + ProjectKey string `json:"projectKey" validate:"required"` + UserName string `json:"userName" validate:"required"` + APIKey string `json:"apiKey"` + AssigneeID string `json:"assigneeId"` + Type string `json:"issueType"` + Labels []string `json:"labels" validate:"required,dive,min=1"` } // OpsgenieConfig defines options for each Opsgenie output diff --git a/internal/core/alert_delivery/outputs/jira.go b/internal/core/alert_delivery/outputs/jira.go index 9ffe0c8fab..8e7cd01953 100644 --- a/internal/core/alert_delivery/outputs/jira.go +++ b/internal/core/alert_delivery/outputs/jira.go @@ -38,7 +38,7 @@ func (client *OutputClient) Jira( alert *alertModels.Alert, config *outputModels.JiraConfig) *AlertDeliveryResponse { description := "*Description:* " + alert.AnalysisDescription - link := "\n [Click here to view in the Panther UI](" + generateURL(alert) + ")" + link := "\n [Click here to view in the Panther UI|" + generateURL(alert) + "]" runBook := "\n *Runbook:* " + alert.Runbook severity := "\n *Severity:* " + alert.Severity tags := "\n *Tags:* " + strings.Join(alert.Tags, ", ") @@ -55,6 +55,7 @@ func (client *OutputClient) Jira( "issuetype": map[string]*string{ "name": aws.String(config.Type), }, + "labels": aws.StringSlice(config.Labels), } if config.AssigneeID != "" { diff --git a/internal/core/alert_delivery/outputs/jira_test.go b/internal/core/alert_delivery/outputs/jira_test.go index d09a9ff7da..7d51fdb184 100644 --- a/internal/core/alert_delivery/outputs/jira_test.go +++ b/internal/core/alert_delivery/outputs/jira_test.go @@ -37,6 +37,7 @@ var jiraConfig = &outputModels.JiraConfig{ ProjectKey: "QR", Type: "Task", UserName: "username", + Labels: []string{"panther", "test-label"}, } func TestJiraAlert(t *testing.T) { @@ -53,12 +54,11 @@ func TestJiraAlert(t *testing.T) { Severity: "INFO", Context: map[string]interface{}{"key": "value"}, } - jiraPayload := map[string]interface{}{ "fields": map[string]interface{}{ "summary": "Policy Failure: policyId", "description": "*Description:* policyDescription\n " + - "[Click here to view in the Panther UI](https://panther.io/policies/policyId)\n" + + "[Click here to view in the Panther UI|https://panther.io/policies/policyId]\n" + " *Runbook:* \n *Severity:* INFO\n *Tags:* \n *AlertContext:* {\"key\":\"value\"}", "project": map[string]*string{ "key": aws.String(jiraConfig.ProjectKey), @@ -69,6 +69,7 @@ func TestJiraAlert(t *testing.T) { "assignee": map[string]*string{ "id": aws.String(jiraConfig.AssigneeID), }, + "labels": aws.StringSlice(jiraConfig.Labels), }, } auth := jiraConfig.UserName + ":" + jiraConfig.APIKey diff --git a/internal/core/outputs_api/api/utils.go b/internal/core/outputs_api/api/utils.go index 1381a55658..500cf22b9a 100644 --- a/internal/core/outputs_api/api/utils.go +++ b/internal/core/outputs_api/api/utils.go @@ -161,7 +161,7 @@ func mergeConfigs(oldConfig, newConfig *models.OutputConfig) (*models.OutputConf } } // Turn the bytes into a map so we can work with it more easily - var oldMap map[string]map[string]string + var oldMap map[string]map[string]interface{} err = jsoniter.Unmarshal(oldBytes, &oldMap) if err != nil { return nil, &genericapi.InternalError{ @@ -176,7 +176,7 @@ func mergeConfigs(oldConfig, newConfig *models.OutputConfig) (*models.OutputConf Message: "Unable to extract the new configuration", } } - var newMap map[string]map[string]string + var newMap map[string]map[string]interface{} err = jsoniter.Unmarshal(newBytes, &newMap) if err != nil { return nil, &genericapi.InternalError{ diff --git a/web/__generated__/schema.tsx b/web/__generated__/schema.tsx index 17a8f9d7c0..0656f32918 100644 --- a/web/__generated__/schema.tsx +++ b/web/__generated__/schema.tsx @@ -534,6 +534,7 @@ export type JiraConfig = { apiKey: Scalars['String']; assigneeId?: Maybe; issueType: Scalars['String']; + labels: Array; }; export type JiraConfigInput = { @@ -543,6 +544,7 @@ export type JiraConfigInput = { apiKey: Scalars['String']; assigneeId?: Maybe; issueType: Scalars['String']; + labels?: Maybe>; }; export type ListAlertsInput = { @@ -2352,6 +2354,7 @@ export type JiraConfigResolvers< apiKey?: Resolver; assigneeId?: Resolver, ParentType, ContextType>; issueType?: Resolver; + labels?: Resolver, ParentType, ContextType>; __isTypeOf?: IsTypeOfResolverFn; }; diff --git a/web/__tests__/__mocks__/builders.generated.ts b/web/__tests__/__mocks__/builders.generated.ts index 0a4d4546c0..f27d4151dc 100644 --- a/web/__tests__/__mocks__/builders.generated.ts +++ b/web/__tests__/__mocks__/builders.generated.ts @@ -859,6 +859,7 @@ export const buildJiraConfig = (overrides: Partial = {}): JiraConfig apiKey: 'apiKey' in overrides ? overrides.apiKey : 'bluetooth', assigneeId: 'assigneeId' in overrides ? overrides.assigneeId : 'bleeding-edge', issueType: 'issueType' in overrides ? overrides.issueType : 'Iowa', + labels: 'labels' in overrides ? overrides.labels : ['Rhode Island'], }; }; @@ -870,6 +871,7 @@ export const buildJiraConfigInput = (overrides: Partial = {}): apiKey: 'apiKey' in overrides ? overrides.apiKey : 'Sleek Cotton Car', assigneeId: 'assigneeId' in overrides ? overrides.assigneeId : 'Virgin Islands, British', issueType: 'issueType' in overrides ? overrides.issueType : 'strategic', + labels: 'labels' in overrides ? overrides.labels : ['magenta'], }; }; diff --git a/web/src/components/forms/DestinationFormSwitcher/DestinationFormSwitcher.tsx b/web/src/components/forms/DestinationFormSwitcher/DestinationFormSwitcher.tsx index c00bd37033..393b3c2f8b 100644 --- a/web/src/components/forms/DestinationFormSwitcher/DestinationFormSwitcher.tsx +++ b/web/src/components/forms/DestinationFormSwitcher/DestinationFormSwitcher.tsx @@ -83,6 +83,7 @@ const DestinationFormSwitcher: React.FC = ({ 'jira.apiKey', 'jira.assigneeId', 'jira.issueType', + 'jira.labels', ]), }} onSubmit={onSubmit} diff --git a/web/src/components/forms/JiraDestinationForm/JiraDestinationForm.test.tsx b/web/src/components/forms/JiraDestinationForm/JiraDestinationForm.test.tsx index 5637042194..588454c688 100644 --- a/web/src/components/forms/JiraDestinationForm/JiraDestinationForm.test.tsx +++ b/web/src/components/forms/JiraDestinationForm/JiraDestinationForm.test.tsx @@ -33,12 +33,14 @@ const emptyInitialValues = { projectKey: '', issueType: '', userName: '', + labels: [], }, }, }; const displayName = 'Jira'; const severity = SeverityEnum.Critical; +const labels = ['panther', 'label']; const initialValues = { outputId: '123', @@ -51,6 +53,7 @@ const initialValues = { projectKey: 'key', issueType: 'Bug', userName: faker.internet.email(), + labels: ['panther', 'label'], }, }, defaultForSeverity: [severity], @@ -58,7 +61,7 @@ const initialValues = { describe('JiraDestinationForm', () => { it('renders the correct fields', () => { - const { getByLabelText, getByText } = render( + const { getByLabelText, getAllByLabelText, getByText } = render( {}} initialValues={emptyInitialValues} /> ); const displayNameField = getByLabelText('* Display Name'); @@ -68,6 +71,7 @@ describe('JiraDestinationForm', () => { const apiKeyField = getByLabelText('* Jira API Key'); const assigneeIdField = getByLabelText('Assignee ID'); const issueTypeField = getByLabelText('* Issue Type'); + const labelsField = getAllByLabelText('Labels')[0]; const submitButton = getByText('Add Destination'); expect(displayNameField).toBeInTheDocument(); expect(orgDomainField).toBeInTheDocument(); @@ -76,6 +80,7 @@ describe('JiraDestinationForm', () => { expect(apiKeyField).toBeInTheDocument(); expect(assigneeIdField).toBeInTheDocument(); expect(issueTypeField).toBeInTheDocument(); + expect(labelsField).toBeInTheDocument(); Object.values(SeverityEnum).forEach(sev => { expect(getByText(sev)).toBeInTheDocument(); }); @@ -84,7 +89,7 @@ describe('JiraDestinationForm', () => { }); it('has proper validation', async () => { - const { getByLabelText, getByText } = render( + const { getByLabelText, getAllByLabelText, getByText } = render( {}} initialValues={emptyInitialValues} /> ); const displayNameField = getByLabelText('* Display Name'); @@ -94,6 +99,7 @@ describe('JiraDestinationForm', () => { const apiKeyField = getByLabelText('* Jira API Key'); const assigneeIdField = getByLabelText('Assignee ID'); const issueTypeField = getByLabelText('* Issue Type'); + const labelsField = getAllByLabelText('Labels')[0]; const submitButton = getByText('Add Destination'); const criticalSeverityCheckBox = document.getElementById(severity); expect(criticalSeverityCheckBox).not.toBeNull(); @@ -117,6 +123,17 @@ describe('JiraDestinationForm', () => { expect(submitButton).toHaveAttribute('disabled'); fireEvent.change(issueTypeField, { target: { value: 'Bug' } }); await waitMs(50); + expect(submitButton).not.toHaveAttribute('disabled'); + // Labels is not required + labels.forEach(label => { + fireEvent.change(labelsField, { + target: { + value: label, + }, + }); + fireEvent.blur(labelsField); + }); + await waitMs(50); // Assignee ID is not required expect(submitButton).not.toHaveAttribute('disabled'); fireEvent.change(assigneeIdField, { target: { value: 'key' } }); @@ -126,7 +143,7 @@ describe('JiraDestinationForm', () => { it('should trigger submit successfully', async () => { const submitMockFunc = jest.fn(); - const { getByLabelText, getByText } = render( + const { getByLabelText, getAllByLabelText, getByText } = render( ); const jiraInput = buildJiraConfigInput({ @@ -139,6 +156,7 @@ describe('JiraDestinationForm', () => { const apiKeyField = getByLabelText('* Jira API Key'); const assigneeIdField = getByLabelText('Assignee ID'); const issueTypeField = getByLabelText('* Issue Type'); + const labelsField = getAllByLabelText('Labels')[0]; const submitButton = getByText('Add Destination'); const criticalSeverityCheckBox = document.getElementById(severity); expect(criticalSeverityCheckBox).not.toBeNull(); @@ -152,6 +170,14 @@ describe('JiraDestinationForm', () => { fireEvent.change(apiKeyField, { target: { value: jiraInput.apiKey } }); fireEvent.change(assigneeIdField, { target: { value: jiraInput.assigneeId } }); fireEvent.change(issueTypeField, { target: { value: jiraInput.issueType } }); + jiraInput.labels.forEach(label => { + fireEvent.change(labelsField, { + target: { + value: label, + }, + }); + fireEvent.blur(labelsField); + }); await waitMs(50); expect(submitButton).not.toHaveAttribute('disabled'); diff --git a/web/src/components/forms/JiraDestinationForm/JiraDestinationForm.tsx b/web/src/components/forms/JiraDestinationForm/JiraDestinationForm.tsx index 1ffc9255a0..7a67f1837b 100644 --- a/web/src/components/forms/JiraDestinationForm/JiraDestinationForm.tsx +++ b/web/src/components/forms/JiraDestinationForm/JiraDestinationForm.tsx @@ -27,6 +27,8 @@ import BaseDestinationForm, { defaultValidationSchema, } from 'Components/forms/BaseDestinationForm'; import { Box, FormHelperText, SimpleGrid } from 'pouncejs'; +import FormikMultiCombobox from 'Components/fields/MultiComboBox'; +import { hasNoWhitespaces } from 'Helpers/utils'; type JiraFieldValues = Pick; @@ -46,6 +48,7 @@ const JiraDestinationForm: React.FC = ({ onSubmit, ini projectKey: Yup.string().required(), assigneeId: Yup.string(), issueType: Yup.string().required(), + labels: Yup.array().of(Yup.string()), apiKey: existing ? Yup.string() : Yup.string().required(), }), }), @@ -83,7 +86,7 @@ const JiraDestinationForm: React.FC = ({ onSubmit, ini autoComplete="new-password" /> - + = ({ onSubmit, ini required={!existing} autoComplete="new-password" /> - - + + = ({ onSubmit, ini Can be Bug, Story, Task or any custom type + + + + + Add by pressing the {'<'}Enter{'>'} key + + ); diff --git a/web/src/components/wizards/CreateDestinationWizard/ConfigureDestinationPanel/ConfigureDestinationPanel.tsx b/web/src/components/wizards/CreateDestinationWizard/ConfigureDestinationPanel/ConfigureDestinationPanel.tsx index 463fee7032..65aadcd993 100644 --- a/web/src/components/wizards/CreateDestinationWizard/ConfigureDestinationPanel/ConfigureDestinationPanel.tsx +++ b/web/src/components/wizards/CreateDestinationWizard/ConfigureDestinationPanel/ConfigureDestinationPanel.tsx @@ -46,6 +46,7 @@ const initialValues: Omit = { apiKey: '', assigneeId: '', issueType: '', + labels: [], }, opsgenie: { apiKey: '', serviceRegion: OpsgenieServiceRegionEnum.Us }, slack: { webhookURL: '' }, diff --git a/web/src/graphql/fragments/DestinationFull.generated.ts b/web/src/graphql/fragments/DestinationFull.generated.ts index 19b4718e6f..0bc1aa2925 100644 --- a/web/src/graphql/fragments/DestinationFull.generated.ts +++ b/web/src/graphql/fragments/DestinationFull.generated.ts @@ -41,7 +41,7 @@ export type DestinationFull = { __typename: 'Destination' } & Pick< jira?: Types.Maybe< Pick< Types.JiraConfig, - 'orgDomain' | 'projectKey' | 'userName' | 'apiKey' | 'assigneeId' | 'issueType' + 'orgDomain' | 'projectKey' | 'userName' | 'apiKey' | 'assigneeId' | 'issueType' | 'labels' > >; opsgenie?: Types.Maybe>; @@ -82,6 +82,7 @@ export const DestinationFull = gql` apiKey assigneeId issueType + labels } opsgenie { apiKey diff --git a/web/src/graphql/fragments/DestinationFull.graphql b/web/src/graphql/fragments/DestinationFull.graphql index 149690c40c..2cf203e2a0 100644 --- a/web/src/graphql/fragments/DestinationFull.graphql +++ b/web/src/graphql/fragments/DestinationFull.graphql @@ -27,6 +27,7 @@ fragment DestinationFull on Destination { apiKey assigneeId issueType + labels } opsgenie { apiKey diff --git a/web/src/helpers/utils.tsx b/web/src/helpers/utils.tsx index d12c41a1f3..0891f30cbc 100644 --- a/web/src/helpers/utils.tsx +++ b/web/src/helpers/utils.tsx @@ -265,6 +265,12 @@ export function slugify(text: string) { export const isNumber = (value: string) => /^-{0,1}\d+$/.test(value); +/** + * A function that returns true if the string consists of only non-whitespace characters + * @param {string} value A string to test + */ +export const hasNoWhitespaces = (value: string) => /^\S+$/.test(value); + export const toStackNameFormat = (val: string) => val.replace(/ /g, '-').toLowerCase(); /* diff --git a/web/src/pages/CreateDestination/CreateDestination.test.tsx b/web/src/pages/CreateDestination/CreateDestination.test.tsx index 959c882825..89898540d8 100644 --- a/web/src/pages/CreateDestination/CreateDestination.test.tsx +++ b/web/src/pages/CreateDestination/CreateDestination.test.tsx @@ -201,6 +201,7 @@ describe('CreateDestination', () => { const userNameInput = getByLabelText('* Email'); const apiKeyInput = getByLabelText('* Jira API Key'); const issueInput = getByLabelText('* Issue Type'); + const labelsInput = getByLabelText('Labels', { selector: 'input' }); const assigneeInput = getByLabelText('Assignee ID'); const criticalSeverityCheckbox = getByLabelText(criticalSeverity); @@ -212,6 +213,14 @@ describe('CreateDestination', () => { fireEvent.change(apiKeyInput, { target: { value: jiraConfig.apiKey } }); fireEvent.change(issueInput, { target: { value: jiraConfig.issueType } }); fireEvent.change(assigneeInput, { target: { value: jiraConfig.assigneeId } }); + jiraConfig.labels.forEach(label => { + fireEvent.change(labelsInput, { + target: { + value: label, + }, + }); + fireEvent.blur(labelsInput); + }); fireEvent.click(criticalSeverityCheckbox); fireEvent.click(getByText('Add Destination')); // Expect success screen with proper redirect link diff --git a/web/src/pages/EditDestination/EditDestination.test.tsx b/web/src/pages/EditDestination/EditDestination.test.tsx index 4073931292..1a30ce6d92 100644 --- a/web/src/pages/EditDestination/EditDestination.test.tsx +++ b/web/src/pages/EditDestination/EditDestination.test.tsx @@ -189,6 +189,7 @@ describe('EditDestination', () => { apiKey: destination.outputConfig.jira.apiKey, assigneeId: destination.outputConfig.jira.assigneeId, issueType: destination.outputConfig.jira.issueType, + labels: destination.outputConfig.jira.labels, }, }, }, diff --git a/web/src/pages/ListDestinations/DestinationCards/JiraDestinationCard.tsx b/web/src/pages/ListDestinations/DestinationCards/JiraDestinationCard.tsx index fb5e7ecfca..1cfd8b9dc0 100644 --- a/web/src/pages/ListDestinations/DestinationCards/JiraDestinationCard.tsx +++ b/web/src/pages/ListDestinations/DestinationCards/JiraDestinationCard.tsx @@ -39,6 +39,10 @@ const JiraDestinationCard: React.FC = ({ destination } + { expect(getByText(jiraDestination.outputConfig.jira.assigneeId)).toBeInTheDocument(); expect(getByText(jiraDestination.outputConfig.jira.issueType)).toBeInTheDocument(); expect(getByText(jiraDestination.outputConfig.jira.orgDomain)).toBeInTheDocument(); + expect(getByText('Labels')).toBeInTheDocument(); expect(getByText('Date Created')).toBeInTheDocument(); expect(getByText('Last Updated')).toBeInTheDocument(); });