Skip to content

Commit

Permalink
Merge pull request #1406 from bjoernricks/model-boolean-parsing
Browse files Browse the repository at this point in the history
Model boolean parsing
  • Loading branch information
bjoernricks authored May 15, 2019
2 parents 32fd507 + 28f94ac commit a8eb5d9
Show file tree
Hide file tree
Showing 12 changed files with 187 additions and 75 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Cleanup get_report function in gsad [#1263](https://github.com/greenbone/gsa/pull/1263)

### Fixed
- Fix checking if an entity is in use [#1406](https://github.com/greenbone/gsa/pull/1406)
- Fix "Invalid date" string for scan times [#1405](https://github.com/greenbone/gsa/pull/1405)
- Fix missing "Applied filter" message for "NVTs by Family" chart [#1404](https://github.com/greenbone/gsa/pull/1404)
- Load all filters and report formats at the report details page [#1401](https://github.com/greenbone/gsa/pull/1401)
Expand Down
39 changes: 39 additions & 0 deletions gsa/src/gmp/__tests__/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
parseCsv,
parseCvssBaseVector,
parseCvssBaseFromVector,
parseBoolean,
parseDate,
parseDuration,
parseEnvelopeMeta,
Expand Down Expand Up @@ -56,6 +57,11 @@ describe('parseInt tests', () => {
expect(parseInt('5.1')).toBe(5);
});

test('should parse int numbers', () => {
expect(parseInt(5)).toBe(5);
expect(parseInt(-5)).toBe(-5);
});

test('should cut float numbers', () => {
expect(parseInt(5.9999)).toBe(5);
expect(parseInt(5.1)).toBe(5);
Expand Down Expand Up @@ -792,4 +798,37 @@ describe('parseText tests', () => {
});
});

describe('parseBoolean tests', () => {
test('should parse int numbers', () => {
expect(parseBoolean(1)).toBe(true);
expect(parseBoolean(0)).toBe(false);
expect(parseBoolean(2)).toBe(true);
expect(parseBoolean(-2)).toBe(true);
});

test('should parse int number strings', () => {
expect(parseBoolean('1')).toBe(true);
expect(parseBoolean('0')).toBe(false);
expect(parseBoolean('2')).toBe(true);
expect(parseBoolean('-2')).toBe(true);
});

test('should parse undefined', () => {
expect(parseBoolean()).toBe(false);
});

test('should parse empty string', () => {
expect(parseBoolean('')).toBe(false);
});

test('should parse non number strings', () => {
expect(parseBoolean('foo')).toBe(false);
});

test('should parse boolean', () => {
expect(parseBoolean(false)).toBe(false);
expect(parseBoolean(true)).toBe(true);
});
});

// vim: set ts=2 sw=2 tw=80:
10 changes: 8 additions & 2 deletions gsa/src/gmp/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
parseProperties,
parseYesNo,
parseDate,
parseBoolean,
setProperties,
NO_VALUE,
YES_VALUE,
Expand Down Expand Up @@ -86,7 +87,7 @@ class Model {
copy.userTags = [];
}

const yes_no_props = ['in_use', 'writable', 'orphan', 'active', 'trash'];
const yes_no_props = ['writable', 'orphan', 'active', 'trash'];

for (const name of yes_no_props) {
const prop = elem[name];
Expand All @@ -95,6 +96,11 @@ class Model {
}
}

if (isDefined(elem.in_use)) {
copy.inUse = parseBoolean(elem.in_use);
delete copy.in_use;
}

if (isDefined(elem.owner) && isEmpty(elem.owner.name)) {
delete copy.owner;
}
Expand All @@ -107,7 +113,7 @@ class Model {
}

isInUse() {
return this.in_use === YES_VALUE;
return this.inUse === true;
}

isInTrash() {
Expand Down
2 changes: 1 addition & 1 deletion gsa/src/gmp/models/__tests__/alert.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import Alert, {
} from 'gmp/models/alert';
import {testModel} from 'gmp/models/testing';

testModel(Alert, 'alert');
testModel(Alert, 'alert', {testIsActive: false});

describe('Alert Model tests', () => {
test('should parse condition, event, and method', () => {
Expand Down
2 changes: 1 addition & 1 deletion gsa/src/gmp/models/__tests__/reportformat.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {testModel} from 'gmp/models/testing';

import {NO_VALUE, YES_VALUE} from 'gmp/parser';

testModel(ReportFormat, 'reportformat');
testModel(ReportFormat, 'reportformat', {testIsActive: false});

describe('ReportFormat model tests', () => {
test('should parse trust', () => {
Expand Down
34 changes: 1 addition & 33 deletions gsa/src/gmp/models/__tests__/task.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import Task, {
} from 'gmp/models/task';
import {testModelProperties} from '../testing';

testModelProperties(Task, 'task');
testModelProperties(Task, 'task', {testIsActive: false});

describe('Task model tests', () => {
test('should parse undefined hosts_ordering', () => {
Expand Down Expand Up @@ -58,38 +58,6 @@ describe('Task model tests', () => {
});

describe(`Task Model methods tests`, () => {
test('isInUse() should return correct true/false', () => {
const task1 = new Task({in_use: '1'});
const task2 = new Task({in_use: '0'});

expect(task1.isInUse()).toBe(true);
expect(task2.isInUse()).toBe(false);
});

test('isInTrash() should return correct true/false', () => {
const task1 = new Task({trash: '1'});
const task2 = new Task({trash: '0'});

expect(task1.isInTrash()).toBe(true);
expect(task2.isInTrash()).toBe(false);
});

test('isWritable() should return correct true/false', () => {
const task1 = new Task({writable: '1'});
const task2 = new Task({writable: '0'});

expect(task1.isWritable()).toBe(true);
expect(task2.isWritable()).toBe(false);
});

test('isOrphan() should return correct true/false', () => {
const task1 = new Task({orphan: '1'});
const task2 = new Task({orphan: '0'});

expect(task1.isOrphan()).toBe(true);
expect(task2.isOrphan()).toBe(false);
});

test('should be a container if target_id is not set', () => {
const task1 = new Task({});
const task2 = new Task({target: {_id: 'foo'}});
Expand Down
84 changes: 77 additions & 7 deletions gsa/src/gmp/models/testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ const testNvtId = modelClass => {
});
};

export const testModelProperties = (modelClass, type) => {
export const testModelProperties = (
modelClass,
type,
{testIsActive = true} = {},
) => {
describe(`${type} Model tests`, () => {
test('should unescape name when parsing', () => {
const elem = {name: `foo"<>&'/\`};
Expand Down Expand Up @@ -132,14 +136,80 @@ export const testModelProperties = (modelClass, type) => {

test('should parse props as YES_VALUE/NO_VALUE', () => {
const elem = {
in_use: '1',
writable: '0',
orphan: '1',
active: '0',
trash: '1',
};
const model = new modelClass(elem);

expect(model.in_use).toEqual(YES_VALUE);
expect(model.writable).toEqual(NO_VALUE);
expect(model.orphan).toEqual(YES_VALUE);
expect(model.active).toEqual(NO_VALUE);
expect(model.trash).toEqual(YES_VALUE);
});

test('should parse in_use', () => {
const model1 = new modelClass({in_use: '1'});
const model2 = new modelClass({in_use: '0'});
const model3 = new modelClass({in_use: '2'});
const model4 = new modelClass();

expect(model1.isInUse()).toBe(true);
expect(model2.isInUse()).toBe(false);
expect(model3.isInUse()).toBe(true);
expect(model4.isInUse()).toBe(false);
});

test('isInTrash() should return correct true/false', () => {
const model1 = new modelClass({trash: '1'});
const model2 = new modelClass({trash: '0'});
const model3 = new modelClass({trash: '2'});
const model4 = new modelClass();

expect(model1.isInTrash()).toBe(true);
expect(model2.isInTrash()).toBe(false);
expect(model3.isInTrash()).toBe(false);
expect(model4.isInTrash()).toBe(false);
});

test('isWritable() should return correct true/false', () => {
const model1 = new modelClass({writable: '1'});
const model2 = new modelClass({writable: '0'});
const model3 = new modelClass({writable: '2'});
const model4 = new modelClass();

expect(model1.isWritable()).toBe(true);
expect(model2.isWritable()).toBe(false);
expect(model3.isWritable()).toBe(false);
expect(model4.isWritable()).toBe(true);
});

test('isOrphan() should return correct true/false', () => {
const model1 = new modelClass({orphan: '1'});
const model2 = new modelClass({orphan: '0'});
const model3 = new modelClass({orphan: '2'});
const model4 = new modelClass();

expect(model1.isOrphan()).toBe(true);
expect(model2.isOrphan()).toBe(false);
expect(model3.isOrphan()).toBe(false);
expect(model4.isOrphan()).toBe(false);
});

if (testIsActive) {
test('isActive() should return correct true/false', () => {
const model1 = new modelClass({active: '1'});
const model2 = new modelClass({active: '0'});
const model3 = new modelClass({active: '2'});
const model4 = new modelClass();

expect(model1.isActive()).toBe(true);
expect(model2.isActive()).toBe(false);
expect(model3.isActive()).toBe(false);
expect(model4.isActive()).toBe(true);
});
}
});

describe(`${type} Model parse_properties function test`, () => {
Expand Down Expand Up @@ -229,14 +299,14 @@ export const testModelMethods = (modelClass, type) => {
});
};

export const testModel = (modelClass, type) => {
testModelProperties(modelClass, type);
export const testModel = (modelClass, type, options) => {
testModelProperties(modelClass, type, options);
testModelMethods(modelClass, type);
testId(modelClass);
};

export const testNvtModel = modelClass => {
testModelProperties(modelClass, 'nvt');
export const testNvtModel = (modelClass, options) => {
testModelProperties(modelClass, 'nvt', options);
testModelMethods(modelClass, 'nvt');
testNvtId(modelClass);
};
Expand Down
23 changes: 22 additions & 1 deletion gsa/src/gmp/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import 'core-js/fn/object/entries';
import 'core-js/fn/string/starts-with';

import {isDefined, isString} from './utils/identity';
import {isDefined, isString, isNumber} from './utils/identity';
import {isEmpty} from './utils/string';

import date, {duration} from './models/date';
Expand Down Expand Up @@ -412,4 +412,25 @@ export const parseDuration = value => {
return duration(value, 'seconds');
};

/**
* Parse Numbers, Number Strings and Boolean to Boolean
*
* Number Strings are converted to Numbers by using the parseInt function.
* A Number is considered true if the value is not equal zero.
* All other values are compared against true.
*
* @param {String, Number, Boolean} value Value to convert to boolean
*
* @returns true if value is considered true else false
*/
export const parseBoolean = value => {
if (isString(value)) {
value = parseInt(value);
}
if (isNumber(value)) {
return value !== 0;
}
return value === true;
};

// vim: set ts=2 sw=2 tw=80:
5 changes: 4 additions & 1 deletion gsa/src/web/pages/roles/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class RoleComponent extends React.Component {
this.setState({
allUsers,
dialogVisible: true,
in_use: role.isInUse(),
isInUse: role.isInUse(),
role,
title: _('Edit Role {{name}}', role),
});
Expand All @@ -85,6 +85,7 @@ class RoleComponent extends React.Component {
allUsers,
dialogVisible: true,
role,
isInUse: false,
title: _('New Role'),
});
}
Expand Down Expand Up @@ -193,6 +194,7 @@ class RoleComponent extends React.Component {
dialogVisible,
error,
groupId,
isInUse,
permissionName,
permissions,
role,
Expand Down Expand Up @@ -227,6 +229,7 @@ class RoleComponent extends React.Component {
allGroups={allGroups}
allPermissions={allPermissions}
error={error}
isInUse={isInUse}
groupId={groupId}
permissionName={permissionName}
permissions={permissions}
Expand Down
Loading

0 comments on commit a8eb5d9

Please sign in to comment.