Skip to content

Commit

Permalink
Adding configurable description template for reporting issues (amunds…
Browse files Browse the repository at this point in the history
…en-io#1502)

Signed-off-by: Ozan Dogrultan <ozan.dogrultan@deliveryhero.com>
  • Loading branch information
kristenarmes authored and ozandogrultan committed Apr 28, 2022
1 parent 0822408 commit e3903df
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 35 deletions.
40 changes: 17 additions & 23 deletions frontend/amundsen_application/static/.betterer.results
Original file line number Diff line number Diff line change
Expand Up @@ -487,12 +487,12 @@ exports[`eslint`] = {
"js/components/Tags/index.tsx:3468508233": [
[38, 4, 21, "Must use destructuring props assignment", "4236634811"]
],
"js/config/config-default.ts:3310033717": [
"js/config/config-default.ts:3832904838": [
[2, 0, 72, "\`../interfaces\` import should occur before import of \`./config-types\`", "1449508543"],
[332, 6, 21, "\'partitionKey\' is defined but never used.", "399589312"],
[333, 6, 23, "\'partitionValue\' is defined but never used.", "793372348"]
[333, 6, 21, "\'partitionKey\' is defined but never used.", "399589312"],
[334, 6, 23, "\'partitionValue\' is defined but never used.", "793372348"]
],
"js/config/config-utils.ts:1551051440": [
"js/config/config-utils.ts:95111742": [
[11, 0, 45, "\`../interfaces\` import should occur before import of \`./config-types\`", "3885176344"]
],
"js/ducks/announcements/index.spec.ts:1898496537": [
Expand Down Expand Up @@ -900,25 +900,19 @@ exports[`eslint`] = {
[63, 25, 1, "\'_\' is defined but never used.", "177658"],
[66, 14, 204, "A control must be associated with a text label.", "3622841199"]
],
"js/pages/TableDetailPage/ReportTableIssue/index.spec.tsx:834656134": [
[7, 7, 9, "\'AppConfig\' is defined but never used.", "1320620174"],
[11, 2, 14, "\'ComponentProps\' is defined but never used.", "2554343108"],
[15, 2, 15, "\'mapStateToProps\' is defined but never used.", "1389821531"]
],
"js/pages/TableDetailPage/ReportTableIssue/index.tsx:3621699541": [
[67, 4, 22, "Must use destructuring props assignment", "2426007440"],
[82, 11, 19, "Must use destructuring props assignment", "987121924"],
[91, 19, 22, "Must use destructuring props assignment", "2201312961"],
[97, 14, 20, "Must use destructuring props assignment", "1598284208"],
[107, 9, 17, "Must use destructuring state assignment", "4230747098"],
[110, 29, 17, "Must use destructuring state assignment", "4230747098"],
[110, 29, 10, "Use callback in setState when referencing the previous state.", "4014904506"],
[118, 15, 20, "Script URL is a form of eval.", "3959800777"],
[124, 9, 17, "Must use destructuring state assignment", "4230747098"],
[138, 16, 7, "A form label must be associated with a control.", "2729454337"],
[139, 16, 160, "A control must be associated with a text label.", "1834626670"],
[147, 16, 7, "A form label must be associated with a control.", "2729454337"],
[148, 16, 197, "A control must be associated with a text label.", "968313273"]
"js/pages/TableDetailPage/ReportTableIssue/index.tsx:90979507": [
[68, 4, 22, "Must use destructuring props assignment", "2426007440"],
[83, 11, 19, "Must use destructuring props assignment", "987121924"],
[92, 19, 22, "Must use destructuring props assignment", "2201312961"],
[98, 14, 20, "Must use destructuring props assignment", "1598284208"],
[108, 9, 17, "Must use destructuring state assignment", "4230747098"],
[111, 29, 17, "Must use destructuring state assignment", "4230747098"],
[111, 29, 10, "Use callback in setState when referencing the previous state.", "4014904506"],
[119, 15, 20, "Script URL is a form of eval.", "3959800777"],
[125, 9, 17, "Must use destructuring state assignment", "4230747098"],
[139, 16, 7, "A form label must be associated with a control.", "2729454337"],
[140, 16, 160, "A control must be associated with a text label.", "1834626670"],
[148, 16, 7, "A form label must be associated with a control.", "2729454337"]
],
"js/pages/TableDetailPage/RequestDescriptionText/index.spec.tsx:2570104867": [
[7, 7, 11, "\'globalState\' is defined but never used.", "1130616505"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const configCustom: AppConfigCustom = {
userIdLabel: 'email address',
issueTracking: {
enabled: false,
issueDescriptionTemplate: '',
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const configDefault: AppConfig = {
userIdLabel: 'email address',
issueTracking: {
enabled: false,
issueDescriptionTemplate: '',
},
logoPath: null,
logoTitle: 'AMUNDSEN',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,12 @@ interface EditableTextConfig {
/**
* IssueTrackingConfig - configures whether to display the issue tracking feature
* that allows users to display tickets associated with a table and create ones
* linked to a table
* linked to a table, and allows a customized template that will be prepopulated
* in the description for reporting an issue
*/
interface IssueTrackingConfig {
enabled: boolean;
issueDescriptionTemplate: string;
}

export enum NumberStyle {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,15 @@ export function issueTrackingEnabled(): boolean {
return AppConfig.issueTracking.enabled;
}

/**
* Returns the string that will prepopulate the issue description
* text field with a template to suggest more detailed information
* to be provided by the user when an issue is reported
*/
export function getIssueDescriptionTemplate(): string {
return AppConfig.issueTracking.issueDescriptionTemplate;
}

/**
* Returns whether or not notification features should be enabled
*/
Expand Down
10 changes: 10 additions & 0 deletions frontend/amundsen_application/static/js/config/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,16 @@ describe('issueTrackingEnabled', () => {
});
});

describe('getIssueDescriptionTemplate', () => {
it('returns an issue description template string', () => {
AppConfig.issueTracking.issueDescriptionTemplate =
'This is a description template';
expect(ConfigUtils.getIssueDescriptionTemplate()).toBe(
AppConfig.issueTracking.issueDescriptionTemplate
);
});
});

describe('indexDashboardsEnabled', () => {
it('returns whether or not the indexDashboards feature is enabled', () => {
expect(ConfigUtils.indexDashboardsEnabled()).toBe(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@ import * as React from 'react';

import { shallow } from 'enzyme';

import AppConfig from 'config/config';
import globalState from 'fixtures/globalState';
import { NotificationType } from 'interfaces';
import {
ComponentProps,
ReportTableIssue,
ReportTableIssueProps,
mapDispatchToProps,
mapStateToProps,
} from '.';
import { ReportTableIssue, ReportTableIssueProps, mapDispatchToProps } from '.';

const globalAny: any = global;

let mockGetIssueDescriptionTemplate = 'This is a description template';
jest.mock('config/config-utils', () => {
const configUtilsModule = jest.requireActual('config/config-utils');
return {
...configUtilsModule,
getIssueDescriptionTemplate: () => mockGetIssueDescriptionTemplate,
};
});

const mockFormData = {
key: 'val1',
title: 'title',
Expand Down Expand Up @@ -88,6 +90,26 @@ describe('ReportTableIssue', () => {
expect(wrapper.find('.report-table-issue-modal')).toBeTruthy();
});

it('Renders description template', () => {
mockGetIssueDescriptionTemplate = 'This is a description template';
const { wrapper } = setup();
wrapper.setState({ isOpen: true });

expect(wrapper.find('textarea').props().children).toBe(
mockGetIssueDescriptionTemplate
);
});

it('Renders empty description template', () => {
mockGetIssueDescriptionTemplate = '';
const { wrapper } = setup();
wrapper.setState({ isOpen: true });

expect(wrapper.find('textarea').props().children).toBe(
mockGetIssueDescriptionTemplate
);
});

describe('toggle', () => {
it('calls setState with negation of state.isOpen', () => {
setStateSpy.mockClear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
NotificationPayload,
NotificationType,
} from 'interfaces';
import { getIssueDescriptionTemplate } from 'config/config-utils';
import * as Constants from './constants';

import './styles.scss';
Expand Down Expand Up @@ -152,7 +153,9 @@ export class ReportTableIssue extends React.Component<
rows={5}
required
maxLength={2000}
/>
>
{getIssueDescriptionTemplate()}
</textarea>
</div>
<button className="btn btn-primary submit" type="submit">
Submit
Expand Down
4 changes: 2 additions & 2 deletions frontend/docs/application_config.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,5 +316,5 @@ _TODO: Please add doc\*_

## Issue Tracking Features

In order to enable Issue Tracking set `IssueTrackingConfig.enabled` to `true` to see UI features. Further configuration
is required to fully enable the feature, please see this [entry](flask_config.md#issue-tracking-integration-features)
In order to enable Issue Tracking set `IssueTrackingConfig.enabled` to `true` to see UI features. If you wish to prepopulate the issue description text field with a template to suggest more detailed information to be provided by the user when an issue is reported, set `IssueTrackingConfig.issueDescriptionTemplate` with the desired string. Further configuration
is required to fully enable the feature, please see this [entry](flask_config.md#issue-tracking-integration-features).

0 comments on commit e3903df

Please sign in to comment.