Skip to content

Commit

Permalink
updated per code comment review
Browse files Browse the repository at this point in the history
  • Loading branch information
FrankHassanabad committed Mar 16, 2020
1 parent be27c91 commit ca24a96
Show file tree
Hide file tree
Showing 17 changed files with 19 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('feature_flags', () => {
);
});

test('it can be called twice as long as unSetFeatureFlagsForTestsOnly is called in-between', () => {
test('it can be called twice as long as setFeatureFlagsForTestsOnly is called in-between', () => {
setFeatureFlagsForTestsOnly();
unSetFeatureFlagsForTestsOnly();
setFeatureFlagsForTestsOnly();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

// TODO: Delete this file once the lists features are within the product and in a particular version
// TODO: (LIST-FEATURE) Delete this file once the lists features are within the product and in a particular version

// Very temporary file where we put our feature flags for detection lists.
// We need to use an environment variable and CANNOT use a kibana.dev.yml setting because some definitions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export const transformAlertToRule = (
last_success_at: ruleStatus?.attributes.lastSuccessAt,
last_failure_message: ruleStatus?.attributes.lastFailureMessage,
last_success_message: ruleStatus?.attributes.lastSuccessMessage,
// TODO: Remove hasListsFeature() check once we have lists available for a release
// TODO: (LIST-FEATURE) Remove hasListsFeature() check once we have lists available for a release
lists: hasListsFeature() ? alert.params.lists : null,
});
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1342,7 +1342,7 @@ describe('add prepackaged rules schema', () => {
});
});

// TODO: We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// TODO: (LIST-FEATURE) We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// on demand. Since they are per module, we have a an issue where the ENV variables do not take effect. It is better we change all the
// schema's to be function calls to avoid global side effects or just wait until the feature is available. If you want to test this early,
// you can remove the .skip and set your env variable of export ELASTIC_XPACK_SIEM_LISTS_FEATURE=true locally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,6 @@ export const addPrepackagedRulesSchema = Joi.object({
note: note.allow(''),
version: version.required(),

// TODO: Remove the hasListsFeatures once this is ready for release
// TODO: (LIST-FEATURE) Remove the hasListsFeatures once this is ready for release
lists: hasListsFeature() ? lists.default([]) : lists.forbidden().default([]),
});
Original file line number Diff line number Diff line change
Expand Up @@ -1324,7 +1324,7 @@ describe('create rules schema', () => {
).toBeFalsy();
});

// TODO: We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// TODO: (LIST-FEATURE) We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// on demand. Since they are per module, we have a an issue where the ENV variables do not take effect. It is better we change all the
// schema's to be function calls to avoid global side effects or just wait until the feature is available. If you want to test this early,
// you can remove the .skip and set your env variable of export ELASTIC_XPACK_SIEM_LISTS_FEATURE=true locally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,6 @@ export const createRulesSchema = Joi.object({
note: note.allow(''),
version: version.default(1),

// TODO: Remove the hasListsFeatures once this is ready for release
// TODO: (LIST-FEATURE) Remove the hasListsFeatures once this is ready for release
lists: hasListsFeature() ? lists.default([]) : lists.forbidden().default([]),
});
Original file line number Diff line number Diff line change
Expand Up @@ -1545,7 +1545,7 @@ describe('import rules schema', () => {
});
});

// TODO: We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// TODO: (LIST-FEATURE) We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// on demand. Since they are per module, we have a an issue where the ENV variables do not take effect. It is better we change all the
// schema's to be function calls to avoid global side effects or just wait until the feature is available. If you want to test this early,
// you can remove the .skip and set your env variable of export ELASTIC_XPACK_SIEM_LISTS_FEATURE=true locally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export const importRulesSchema = Joi.object({
created_by,
updated_by,

// TODO: Remove the hasListsFeatures once this is ready for release
// TODO: (LIST-FEATURE) Remove the hasListsFeatures once this is ready for release
lists: hasListsFeature() ? lists.default([]) : lists.forbidden().default([]),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,7 @@ describe('patch rules schema', () => {
});
});

// TODO: We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// TODO: (LIST-FEATURE) We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// on demand. Since they are per module, we have a an issue where the ENV variables do not take effect. It is better we change all the
// schema's to be function calls to avoid global side effects or just wait until the feature is available. If you want to test this early,
// you can remove the .skip and set your env variable of export ELASTIC_XPACK_SIEM_LISTS_FEATURE=true locally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,6 @@ export const patchRulesSchema = Joi.object({
note: note.allow(''),
version,

// TODO: Remove the hasListsFeatures once this is ready for release
// TODO: (LIST-FEATURE) Remove the hasListsFeatures once this is ready for release
lists: hasListsFeature() ? lists.default([]) : lists.forbidden().default([]),
}).xor('id', 'rule_id');
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ describe('rules_schema', () => {
expect(message.schema).toEqual({});
});

// TODO: Remove this test once the feature flag is deployed
// TODO: (LIST-FEATURE) Remove this test once the feature flag is deployed
test('it should remove lists when we need it to be removed because the feature is off but there exists a list in the data', () => {
const payload = getBaseResponsePayload();
const decoded = rulesSchema.decode(payload);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export const rulesSchema = new t.Type<
(input): Either<t.Errors, RulesWithoutTypeDependentsSchema> => {
const output = checkTypeDependents(input);
if (!hasListsFeature()) {
// TODO: Remove this after the lists feature is an accepted feature for a particular release
// TODO: (LIST-FEATURE) Remove this after the lists feature is an accepted feature for a particular release
return removeList(output);
} else {
return output;
Expand All @@ -154,7 +154,7 @@ export const rulesSchema = new t.Type<
t.identity
);

// TODO: Remove this after the lists feature is an accepted feature for a particular release
// TODO: (LIST-FEATURE) Remove this after the lists feature is an accepted feature for a particular release
export const removeList = (
decoded: Either<t.Errors, RequiredRulesSchema>
): Either<t.Errors, RequiredRulesSchema> => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export const note = t.string;
// NOTE: Experimental list support not being shipped currently and behind a feature flag
// TODO: Remove this comment once we lists have passed testing and is ready for the release
export const boolean_operator = t.keyof({ and: null, 'and not': null });
export const list_type = t.keyof({ value: null }); // TODO: Eventually this can include "list" when we support lists CRUD
export const list_type = t.keyof({ value: null }); // TODO: (LIST-FEATURE) Eventually this can include "list" when we support lists CRUD
export const list_value = t.exact(t.type({ name: t.string, type: list_type }));
export const list = t.exact(
t.type({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ export const version = Joi.number()
export const note = Joi.string();

// NOTE: Experimental list support not being shipped currently and behind a feature flag
// TODO: Remove this comment once we lists have passed testing and is ready for the release
// TODO: (LIST-FEATURE) Remove this comment once we lists have passed testing and is ready for the release
export const boolean_operator = Joi.string().valid('and', 'and not');
export const list_type = Joi.string().valid('value'); // TODO: Eventually this can be "list" when we support list types
export const list_type = Joi.string().valid('value'); // TODO: (LIST-FEATURE) Eventually this can be "list" when we support list types
export const list_value = Joi.object({ name: Joi.string().required(), type: list_type.required() });
export const list = Joi.object({
field: Joi.string().required(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ describe('create rules schema', () => {
});
});

// TODO: We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// TODO: (LIST-FEATURE) We can enable this once we change the schema's to not be global per module but rather functions that can create the schema
// on demand. Since they are per module, we have a an issue where the ENV variables do not take effect. It is better we change all the
// schema's to be function calls to avoid global side effects or just wait until the feature is available. If you want to test this early,
// you can remove the .skip and set your env variable of export ELASTIC_XPACK_SIEM_LISTS_FEATURE=true locally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,6 @@ export const updateRulesSchema = Joi.object({
note: note.allow(''),
version,

// TODO: Remove the hasListsFeatures once this is ready for release
// TODO: (LIST-FEATURE) Remove the hasListsFeatures once this is ready for release
lists: hasListsFeature() ? lists.default([]) : lists.forbidden().default([]),
}).xor('id', 'rule_id');

0 comments on commit ca24a96

Please sign in to comment.