Skip to content

Commit

Permalink
Remove features and feature_list usage in Notifications (#382)
Browse files Browse the repository at this point in the history
* Rename doesUserHas* access functions to doesUserHave*

Signed-off-by: Mohammad Qureshi <[email protected]>

* Upgrade to kotlin 1.6.10

Signed-off-by: Mohammad Qureshi <[email protected]>

* Remove usage of feature and feature_list in Notifications backend and fix compilation errors

Signed-off-by: Mohammad Qureshi <[email protected]>

* Remove usage of feature and feature_list in Notifications frontend

Signed-off-by: Mohammad Qureshi <[email protected]>
  • Loading branch information
qreshi authored Mar 18, 2022
1 parent aa26e98 commit 6a0e36b
Show file tree
Hide file tree
Showing 47 changed files with 107 additions and 688 deletions.
2 changes: 0 additions & 2 deletions dashboards-notifications/models/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { WebhookMethodType } from '../public/pages/Channels/types';
import {
CHANNEL_TYPE,
ENCRYPTION_TYPE,
NOTIFICATION_SOURCE,
} from '../public/utils/constants';

export interface ChannelStatus {
Expand All @@ -31,7 +30,6 @@ export type SenderType = 'smtp_account' | 'ses_account';

export interface ChannelItemType extends ConfigType {
config_type: keyof typeof CHANNEL_TYPE;
feature_list: Array<keyof typeof NOTIFICATION_SOURCE>;
is_enabled: boolean; // active or muted
slack?: {
url: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export function ChannelDetailsActions(props: ChannelDetailsActionsProps) {
try {
await servicesContext.notificationService.sendTestMessage(
props.channel.config_id,
props.channel.feature_list[0]
);
coreContext.notifications.toasts.addSuccess(
'Successfully sent a test message.'
Expand All @@ -62,7 +61,6 @@ export function ChannelDetailsActions(props: ChannelDetailsActionsProps) {
{
label: 'Send test message',
disabled:
props.channel.feature_list.length === 0 ||
!props.channel.config_id ||
!props.channel.is_enabled,
action: sendTestMessage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ export function CreateChannel(props: CreateChannelsProps) {
name,
description,
config_type: channelType,
feature_list: ['alerting'], // TODO: Remove this from config when the backend no longer requires it
is_enabled: isEnabled,
};
if (channelType === BACKEND_CHANNEL_TYPE.SLACK) {
Expand Down Expand Up @@ -327,7 +326,6 @@ export function CreateChannel(props: CreateChannelsProps) {

await servicesContext.notificationService.sendTestMessage(
tempChannelId,
config.feature_list[0] // for test message any source works
);
coreContext.notifications.toasts.addSuccess(
'Successfully sent a test message.'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ describe('creates sender and recipient groups as config object', () => {
expect(config).toEqual({
name: 'sender',
config_type: 'smtp_account',
feature_list: ['alerting', 'index_management', 'reports'],
is_enabled: true,
smtp_account: {
host: 'test.com',
Expand Down Expand Up @@ -52,7 +51,6 @@ describe('creates sender and recipient groups as config object', () => {
name: 'recipient group',
config_type: 'email_group',
description: 'test description',
feature_list: ['alerting', 'index_management', 'reports'],
is_enabled: true,
email_group: {
recipient_list: [
Expand Down
5 changes: 1 addition & 4 deletions dashboards-notifications/public/pages/Emails/utils/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { EuiComboBoxOptionOption } from '@elastic/eui';
import { ENCRYPTION_TYPE, NOTIFICATION_SOURCE } from '../../../utils/constants';
import { ENCRYPTION_TYPE } from '../../../utils/constants';

export const createSenderConfigObject = (
senderName: string,
Expand All @@ -16,7 +16,6 @@ export const createSenderConfigObject = (
return {
name: senderName,
config_type: 'smtp_account',
feature_list: Object.keys(NOTIFICATION_SOURCE),
is_enabled: true,
smtp_account: {
host,
Expand All @@ -36,7 +35,6 @@ export const createSesSenderConfigObject = (
return {
name: senderName,
config_type: 'ses_account',
feature_list: Object.keys(NOTIFICATION_SOURCE),
is_enabled: true,
ses_account: {
from_address: email,
Expand All @@ -55,7 +53,6 @@ export const createRecipientGroupConfigObject = (
name,
description,
config_type: 'email_group',
feature_list: Object.keys(NOTIFICATION_SOURCE),
is_enabled: true,
email_group: {
recipient_list: selectedEmailOptions.map((email) => ({
Expand Down
12 changes: 3 additions & 9 deletions dashboards-notifications/public/services/NotificationService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
SenderType,
SESSenderItemType,
} from '../../models/interfaces';
import {CHANNEL_TYPE, NOTIFICATION_SOURCE} from '../utils/constants';
import { CHANNEL_TYPE } from '../utils/constants';
import {
configListToChannels,
configListToRecipientGroups,
Expand Down Expand Up @@ -244,16 +244,10 @@ export default class NotificationService {
};

sendTestMessage = async (
configId: string,
feature: keyof typeof NOTIFICATION_SOURCE
configId: string
) => {
const response = await this.httpClient.get(
`${NODE_API.SEND_TEST_MESSAGE}/${configId}`,
{
query: {
feature,
},
}
`${NODE_API.SEND_TEST_MESSAGE}/${configId}`
);
if (response.event_id != null) {
await this.getNotification(response.event_id).then((response) => {
Expand Down
6 changes: 0 additions & 6 deletions dashboards-notifications/public/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,6 @@ export const BREADCRUMBS = Object.freeze({
EDIT_RECIPIENT_GROUP: { text: 'Edit recipient group' },
});

export const NOTIFICATION_SOURCE = Object.freeze({
alerting: 'Alerting',
index_management: 'ISM',
reports: 'Reporting',
});

export const BACKEND_CHANNEL_TYPE = Object.freeze({
SLACK: 'slack',
EMAIL: 'email',
Expand Down
5 changes: 0 additions & 5 deletions dashboards-notifications/server/routes/configRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ export function configRoutes(router: IRouter) {
schema.arrayOf(schema.string()),
schema.string(),
]),
feature_list: schema.maybe(
schema.oneOf([schema.arrayOf(schema.string()), schema.string()])
),
is_enabled: schema.maybe(schema.boolean()),
sort_field: schema.string(),
sort_order: schema.string(),
Expand All @@ -41,7 +38,6 @@ export function configRoutes(router: IRouter) {
},
async (context, request, response) => {
const config_type = joinRequestParams(request.query.config_type);
const feature_list = joinRequestParams(request.query.feature_list);
const config_id_list = joinRequestParams(request.query.config_id_list);
const encryption_method = joinRequestParams(
request.query['smtp_account.method']
Expand All @@ -61,7 +57,6 @@ export function configRoutes(router: IRouter) {
sort_field: request.query.sort_field,
sort_order: request.query.sort_order,
config_type,
...(feature_list && { feature_list }),
...(query && { text_query: query }), // text_query will exclude keyword fields
...(config_id_list && { config_id_list }),
...(encryption_method && {
Expand Down
4 changes: 0 additions & 4 deletions dashboards-notifications/server/routes/eventRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ export function eventRoutes(router: IRouter) {
params: schema.object({
configId: schema.string(),
}),
query: schema.object({
feature: schema.string(),
}),
},
},
async (context, request, response) => {
Expand All @@ -62,7 +59,6 @@ export function eventRoutes(router: IRouter) {
'notifications.sendTestMessage',
{
configId: request.params.configId,
feature: request.query.feature,
}
);
return response.ok({ body: resp });
Expand Down
6 changes: 0 additions & 6 deletions dashboards-notifications/test/mocks/mockData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const mockChime: ChannelItemType = {
name: 'Chime test channel',
description: 'test description',
config_type: 'chime',
feature_list: ['reports'],
is_enabled: true,
chime: {
url: 'https://chimehook',
Expand All @@ -28,7 +27,6 @@ const mockSlack: ChannelItemType = {
name: 'Slack test channel',
description: 'test description',
config_type: 'slack',
feature_list: ['reports'],
is_enabled: false,
slack: {
url: 'https://chimehook',
Expand All @@ -42,7 +40,6 @@ const mockEmail: ChannelItemType = {
name: 'Email test channel',
description: 'test description',
config_type: 'email',
feature_list: ['alerting'],
is_enabled: true,
email: {
email_account_id: 'dj8etXkBCzVy9Vy-nsiL',
Expand Down Expand Up @@ -76,7 +73,6 @@ const mockEmailWithSES: ChannelItemType = {
name: 'Email test channel',
description: 'test description',
config_type: 'email',
feature_list: ['alerting'],
is_enabled: true,
email: {
email_account_id: 'dj8etXkBCzVy9Vy-nsiL',
Expand Down Expand Up @@ -110,7 +106,6 @@ const mockWebhook: ChannelItemType = {
name: 'Webhook test channel',
description: 'test description',
config_type: 'webhook',
feature_list: ['alerting'],
is_enabled: true,
webhook: {
url: 'https://host:23/path?key1=%23%404&key2=&key3=value3',
Expand All @@ -133,7 +128,6 @@ const mockSNS: ChannelItemType = {
name: 'SNS test channel',
description: 'test description',
config_type: 'sns',
feature_list: ['alerting'],
is_enabled: true,
sns: {
topic_arn: 'arn:aws:sns:us-west-2:012345678912:notifications-test',
Expand Down
2 changes: 1 addition & 1 deletion notifications/build-tools/merged-coverage.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

allprojects {
plugins.withId('jacoco') {
jacoco.toolVersion = '0.8.5'
jacoco.toolVersion = '0.8.7'
// For some reason this dependency isn't getting setup automatically by the jacoco plugin
tasks.withType(JacocoReport) {
dependsOn tasks.withType(Test)
Expand Down
8 changes: 4 additions & 4 deletions notifications/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ buildscript {
// 1.0.0 -> 1.0.0.0, and 1.0.0-SNAPSHOT -> 1.0.0.0-SNAPSHOT
opensearch_build = opensearch_version.replaceAll(/(\.\d)([^\d]*)$/, '$1.0$2')
common_utils_version = System.getProperty("common_utils.version", opensearch_build)
kotlin_version = System.getProperty("kotlin.version", "1.4.32")
kotlin_version = System.getProperty("kotlin.version", "1.6.10")
junit_version = System.getProperty("junit.version", "5.7.2")
aws_version = System.getProperty("aws.version", "1.12.48")
}
Expand Down Expand Up @@ -55,10 +55,10 @@ allprojects {
}
group = "org.opensearch"
plugins.withId('java') {
sourceCompatibility = targetCompatibility = "1.8"
sourceCompatibility = targetCompatibility = "11"
}
plugins.withId('org.jetbrains.kotlin.jvm') {
compileKotlin.kotlinOptions.jvmTarget = compileTestKotlin.kotlinOptions.jvmTarget = "1.8"
compileKotlin.kotlinOptions.jvmTarget = compileTestKotlin.kotlinOptions.jvmTarget = "11"
compileKotlin.dependsOn ktlint
}
}
Expand All @@ -68,7 +68,7 @@ configurations {
}

dependencies {
add("ktlint", "com.pinterest:ktlint:0.41.0") {
add("ktlint", "com.pinterest:ktlint:0.44.0") {
attributes {
attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling, Bundling.EXTERNAL))
}
Expand Down
11 changes: 7 additions & 4 deletions notifications/core-spi/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,20 @@ dependencies {
compileOnly "org.opensearch:opensearch:${opensearch_version}"
compileOnly "org.jetbrains.kotlin:kotlin-stdlib:${kotlin_version}"
compileOnly "org.jetbrains.kotlin:kotlin-stdlib-common:${kotlin_version}"
compile "com.amazonaws:aws-java-sdk-ses:${aws_version}"
implementation "com.amazonaws:aws-java-sdk-core:${aws_version}"
implementation "com.amazonaws:aws-java-sdk-ses:${aws_version}"
implementation "org.apache.httpcomponents:httpcore:4.4.13"
implementation "org.apache.httpcomponents:httpclient:4.5.10"
implementation "com.github.seancfoley:ipaddress:5.3.3"

testImplementation(
'org.junit.jupiter:junit-jupiter-api:5.6.2',
"org.junit.jupiter:junit-jupiter-params:5.6.2",
'org.mockito:mockito-junit-jupiter:3.10.0',
)
testRuntime('org.junit.jupiter:junit-jupiter-engine:5.6.2')
testCompile "org.jetbrains.kotlin:kotlin-test:${kotlin_version}"
testCompile "org.opensearch.test:framework:${opensearch_version}"
testRuntimeOnly('org.junit.jupiter:junit-jupiter-engine:5.6.2')
testImplementation "org.jetbrains.kotlin:kotlin-test:${kotlin_version}"
testImplementation "org.opensearch.test:framework:${opensearch_version}"
}

configurations.all {
Expand Down
33 changes: 18 additions & 15 deletions notifications/core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,16 @@ compileKotlin { kotlinOptions.freeCompilerArgs = ['-Xjsr305=strict'] }

dependencies {
compileOnly "org.opensearch:opensearch:${opensearch_version}"
compile "org.jetbrains.kotlin:kotlin-stdlib:${kotlin_version}"
compile "org.jetbrains.kotlin:kotlin-stdlib-common:${kotlin_version}"
compile "org.apache.httpcomponents:httpcore:4.4.5"
compile "org.apache.httpcomponents:httpclient:4.5.10"
compile "com.amazonaws:aws-java-sdk-sns:${aws_version}"
compile "com.amazonaws:aws-java-sdk-sts:${aws_version}"
compile "com.amazonaws:aws-java-sdk-ses:${aws_version}"
compile "com.sun.mail:javax.mail:1.6.2"
implementation "org.jetbrains.kotlin:kotlin-stdlib:${kotlin_version}"
implementation "org.jetbrains.kotlin:kotlin-stdlib-common:${kotlin_version}"
implementation "org.apache.httpcomponents:httpcore:4.4.5"
implementation "org.apache.httpcomponents:httpclient:4.5.10"
implementation "com.amazonaws:aws-java-sdk-core:${aws_version}"
implementation "com.amazonaws:aws-java-sdk-sns:${aws_version}"
implementation "com.amazonaws:aws-java-sdk-sts:${aws_version}"
implementation "com.amazonaws:aws-java-sdk-ses:${aws_version}"
implementation "com.sun.mail:javax.mail:1.6.2"
implementation "javax.activation:activation:1.1"
testImplementation(
'org.assertj:assertj-core:3.16.1',
'org.junit.jupiter:junit-jupiter-api:5.6.2',
Expand All @@ -122,16 +124,17 @@ dependencies {
)
testImplementation 'org.springframework.integration:spring-integration-mail:5.5.0'
testImplementation 'org.springframework.integration:spring-integration-test-support:5.5.0'
testRuntime('org.junit.jupiter:junit-jupiter-engine:5.6.2')
testCompile "org.jetbrains.kotlin:kotlin-test:${kotlin_version}"
testCompile "org.mockito:mockito-core:4.3.1"
testCompile "org.opensearch.test:framework:${opensearch_version}"
testCompile "org.jetbrains.kotlin:kotlin-reflect:${kotlin_version}" // required by mockk
compile project(path: ":${rootProject.name}-core-spi", configuration: 'shadow')
testRuntimeOnly('org.junit.jupiter:junit-jupiter-engine:5.6.2')
testImplementation "org.jetbrains.kotlin:kotlin-test:${kotlin_version}"
testImplementation "net.bytebuddy:byte-buddy-agent:1.12.7"
testImplementation "org.mockito:mockito-core:4.3.1"
testImplementation "org.opensearch.test:framework:${opensearch_version}"
testImplementation "org.jetbrains.kotlin:kotlin-reflect:${kotlin_version}" // required by mockk
implementation project(path: ":${rootProject.name}-core-spi", configuration: 'shadow')
}

configurations {
testCompile {
testImplementation {
exclude group: 'org.elasticsearch', module: 'securemock' // resolve jarhell with mockito
}
}
Expand Down
23 changes: 12 additions & 11 deletions notifications/notifications/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ ext {
noticeFile = rootProject.file('NOTICE')
}

configurations.testCompile {
configurations.testImplementation {
exclude module: "securemock"
}

Expand All @@ -58,12 +58,12 @@ configurations.all {
dependencies {
compileOnly "${group}:opensearch:${opensearch_version}"
compileOnly "org.jetbrains.kotlin:kotlin-stdlib:${kotlin_version}"
compile "org.jetbrains.kotlin:kotlin-stdlib-common:${kotlin_version}"
compile ("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.4.3") {
implementation "org.jetbrains.kotlin:kotlin-stdlib-common:${kotlin_version}"
implementation ("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.4.3") {
exclude group: 'org.jetbrains', module: 'annotations' // resolve jarhell
} // ${kotlin_version} does not work for coroutines
compile "${group}:common-utils:${common_utils_version}"
compile "org.json:json:20180813"
implementation "${group}:common-utils:${common_utils_version}"
implementation "org.json:json:20180813"

testImplementation(
'org.assertj:assertj-core:3.19.0',
Expand All @@ -79,14 +79,15 @@ dependencies {
"org.junit.jupiter:junit-jupiter-api:${junit_version}"
)
testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${junit_version}")
testCompile "org.opensearch.test:framework:${opensearch_version}"
testCompile "org.jetbrains.kotlin:kotlin-test:${kotlin_version}"
testCompile "org.jetbrains.kotlin:kotlin-reflect:${kotlin_version}" // required by mockk
testCompile "org.mockito:mockito-core:4.3.1"
testCompile 'com.google.code.gson:gson:2.8.7'
testImplementation "org.opensearch.test:framework:${opensearch_version}"
testImplementation "org.jetbrains.kotlin:kotlin-test:${kotlin_version}"
testImplementation "org.jetbrains.kotlin:kotlin-reflect:${kotlin_version}" // required by mockk
testImplementation "net.bytebuddy:byte-buddy-agent:1.12.7"
testImplementation "org.mockito:mockito-core:4.3.1"
testImplementation 'com.google.code.gson:gson:2.8.7'
testImplementation 'org.springframework.integration:spring-integration-mail:5.5.0'
testImplementation 'org.springframework.integration:spring-integration-test-support:5.5.0'
compile group: 'com.github.wnameless', name: 'json-flattener', version: '0.1.0'
implementation group: 'com.github.wnameless', name: 'json-flattener', version: '0.1.0'
compileOnly project(path: ":${rootProject.name}-core-spi", configuration: 'shadow')
}

Expand Down
Loading

0 comments on commit 6a0e36b

Please sign in to comment.