Skip to content

Commit

Permalink
bugy#243 added possibility to aggregate scripts in groups on UI
Browse files Browse the repository at this point in the history
  • Loading branch information
bugy authored and antonellocaroli committed Aug 1, 2020
1 parent a080c94 commit 9275a12
Show file tree
Hide file tree
Showing 22 changed files with 789 additions and 143 deletions.
8 changes: 7 additions & 1 deletion src/model/model_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import utils.env_utils as env_utils
from config.constants import FILE_TYPE_DIR, FILE_TYPE_FILE
from utils.string_utils import is_blank

ENV_VAR_PREFIX = '$$'
SECURE_MASK = '*' * 6
Expand Down Expand Up @@ -134,7 +135,7 @@ def read_int_from_config(key, config_obj, *, default=None):
raise InvalidValueTypeException('Invalid %s value: integer expected, but was: %s' % (key, repr(value)))


def read_str_from_config(config_obj, key, *, default=None):
def read_str_from_config(config_obj, key, *, default=None, blank_to_none=False):
"""
Reads string value from a config by the key
If the value is missing, returns specified default value
Expand All @@ -143,9 +144,14 @@ def read_str_from_config(config_obj, key, *, default=None):
:param config_obj: where to read value from
:param key: key to read value from
:param default: default value, if config value is missing
:param blank_to_none: if value is blank, treat it as null
:return: config_obj[key] if non None, default otherwise
"""
value = config_obj.get(key)

if blank_to_none and isinstance(value, str) and is_blank(value):
value = None

if value is None:
return default

Expand Down
5 changes: 4 additions & 1 deletion src/model/script_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@

from auth.authorization import ANY_USER
from model import parameter_config
from model.model_helper import is_empty, fill_parameter_values, read_bool_from_config, InvalidValueException
from model.model_helper import is_empty, fill_parameter_values, read_bool_from_config, InvalidValueException, \
read_str_from_config
from model.parameter_config import ParameterModel
from react.properties import ObservableList, ObservableDict, observable_fields, Property
from utils import file_utils
Expand All @@ -19,6 +20,7 @@ class ShortConfig(object):
def __init__(self):
self.name = None
self.allowed_users = []
self.group = None


@observable_fields(
Expand Down Expand Up @@ -235,6 +237,7 @@ def read_short(file_path, json_object):

config.name = _read_name(file_path, json_object)
config.allowed_users = json_object.get('allowed_users')
config.group = read_str_from_config(json_object, 'group', blank_to_none=True)

hidden = read_bool_from_config('hidden', json_object, default=False)
if hidden:
Expand Down
23 changes: 18 additions & 5 deletions src/tests/config_service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ def test_list_configs_when_multiple(self):
conf_names = [config.name for config in configs]
self.assertCountEqual(['conf_x', 'conf_y', 'A B C'], conf_names)

def test_list_configs_with_groups(self):
_create_script_config_file('conf_x', group='g1')
_create_script_config_file('conf_y')
_create_script_config_file('A B C', group=' ')

configs = self.config_service.list_configs(self.user)
configs_dicts = [{'name': c.name, 'group': c.group} for c in configs]
self.assertCountEqual([
{'name': 'conf_x', 'group': 'g1'},
{'name': 'conf_y', 'group': None},
{'name': 'A B C', 'group': None}],
configs_dicts)

def test_list_configs_when_no(self):
configs = self.config_service.list_configs(self.user)
self.assertEqual([], configs)
Expand Down Expand Up @@ -92,26 +105,26 @@ def test_list_configs_when_no_constraints(self):
_create_script_config_file('a1')
_create_script_config_file('c2')

self.assert_list_configs(self.user1, ['a1', 'c2'])
self.assert_list_config_names(self.user1, ['a1', 'c2'])

def test_list_configs_when_user_allowed(self):
_create_script_config_file('a1', allowed_users=['user1'])
_create_script_config_file('c2', allowed_users=['user1'])

self.assert_list_configs(self.user1, ['a1', 'c2'])
self.assert_list_config_names(self.user1, ['a1', 'c2'])

def test_list_configs_when_one_not_allowed(self):
_create_script_config_file('a1', allowed_users=['XYZ'])
_create_script_config_file('b2')
_create_script_config_file('c3', allowed_users=['user1'])

self.assert_list_configs(self.user1, ['b2', 'c3'])
self.assert_list_config_names(self.user1, ['b2', 'c3'])

def test_list_configs_when_none_allowed(self):
_create_script_config_file('a1', allowed_users=['XYZ'])
_create_script_config_file('b2', allowed_users=['ABC'])

self.assert_list_configs(self.user1, [])
self.assert_list_config_names(self.user1, [])

def test_load_config_when_user_allowed(self):
_create_script_config_file('my_script', allowed_users=['ABC', 'user1', 'qwerty'])
Expand All @@ -125,7 +138,7 @@ def test_load_config_when_user_not_allowed(self):

self.assertRaises(ConfigNotAllowedException, self.config_service.load_config_model, 'my_script', self.user1)

def assert_list_configs(self, user, expected_names):
def assert_list_config_names(self, user, expected_names):
configs = self.config_service.list_configs(user)
conf_names = [config.name for config in configs]
self.assertCountEqual(expected_names, conf_names)
Expand Down
16 changes: 16 additions & 0 deletions src/tests/model_helper_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,22 @@ def test_text_with_whitespaces(self):
value = read_str_from_config({'key1': ' xyz \n'}, 'key1')
self.assertEquals(' xyz \n', value)

def test_text_when_blank_to_none_and_none(self):
value = read_str_from_config({'key1': None}, 'key1', blank_to_none=True)
self.assertIsNone(value)

def test_text_when_blank_to_none_and_empty(self):
value = read_str_from_config({'key1': ''}, 'key1', blank_to_none=True)
self.assertIsNone(value)

def test_text_when_blank_to_none_and_blank(self):
value = read_str_from_config({'key1': ' \t \n'}, 'key1', blank_to_none=True)
self.assertIsNone(value)

def test_text_when_blank_to_none_and_blank_and_default(self):
value = read_str_from_config({'key1': ' \t \n'}, 'key1', blank_to_none=True, default='abc')
self.assertEquals('abc', value)

def test_text_when_int(self):
self.assertRaisesRegex(InvalidValueTypeException, 'Invalid key1 value: string expected, but was: 5',
read_str_from_config, {'key1': 5}, 'key1')
34 changes: 30 additions & 4 deletions src/tests/web/server_test.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
import threading
import traceback
from asyncio import set_event_loop_policy
Expand All @@ -7,6 +8,8 @@
import requests
from tornado.ioloop import IOLoop

from auth.authorization import Authorizer, ANY_USER, EmptyGroupProvider
from config.config_service import ConfigService
from features.file_download_feature import FileDownloadFeature
from files.user_file_storage import UserFileStorage
from model.server_conf import ServerConfig
Expand Down Expand Up @@ -71,26 +74,46 @@ def test_init_when_windows_and_python_3_7(self):

raise

def test_get_scripts(self):
self.start_server(12345, '127.0.0.1')

test_utils.write_script_config({'name': 's1'}, 's1', self.runners_folder)
test_utils.write_script_config({'name': 's2', 'group': 'Xyz'}, 's2', self.runners_folder)
test_utils.write_script_config({'name': 's3'}, 's3', self.runners_folder)

response = self.request('GET', 'http://127.0.0.1:12345/scripts')
self.assertCountEqual([
{'name': 's1', 'group': None},
{'name': 's2', 'group': 'Xyz'},
{'name': 's3', 'group': None}],
response['scripts'])

def request(self, method, url):
response = requests.request(method, url)
self.assertEqual(200, response.status_code, 'Failed to execute request: ' + response.text)
return response.json()

def check_server_running(self):
response = requests.get('http://127.0.0.1:12345/conf')
self.assertEqual(response.status_code, 200)

def start_server(self, port, address):
file_download_feature = FileDownloadFeature(UserFileStorage(b'123456'), test_utils.temp_folder)
file_download_feature = FileDownloadFeature(UserFileStorage(b'some_secret'), test_utils.temp_folder)
config = ServerConfig()
config.port = port
config.address = address

authorizer = Authorizer(ANY_USER, [], [], EmptyGroupProvider())
server.init(config,
None,
authorizer,
None,
None,
None,
None,
ConfigService(authorizer, self.conf_folder),
None,
None,
file_download_feature,
None,
'cookie_secret',
None,
start_server=False)
self.start_loop()
Expand All @@ -107,6 +130,9 @@ def setUp(self) -> None:
self.requires_explicit_ioloop_factory = os_utils.is_win() and env_utils.is_min_version('3.8')
self.windows = os_utils.is_win()

self.conf_folder = test_utils.create_dir(os.path.join('conf'))
self.runners_folder = os.path.join(self.conf_folder, 'runners')

def tearDown(self) -> None:
super().tearDown()

Expand Down
4 changes: 2 additions & 2 deletions src/web/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ class GetScripts(BaseRequestHandler):
def get(self, user):
configs = self.application.config_service.list_configs(user)

names = [conf.name for conf in configs]
scripts = [{'name': conf.name, 'group': conf.group} for conf in configs]

self.write(json.dumps(names))
self.write(json.dumps({'scripts': scripts}))


class AdminUpdateScriptEndpoint(BaseRequestHandler):
Expand Down
2 changes: 1 addition & 1 deletion web-src/js/admin/AdminApp.vue
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
modules: {
'history': executions(),
scripts: scripts,
'script-config': scriptConfig
scriptConfig: scriptConfig
},
actions: {
setSubheader({commit}, subheader) {
Expand Down
4 changes: 2 additions & 2 deletions web-src/js/admin/scripts-config/ScriptConfig.vue
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@
},
methods: {
...mapActions('script-config', ['init', 'save'])
...mapActions('scriptConfig', ['init', 'save'])
},
computed: {
...mapState('script-config', {
...mapState('scriptConfig', {
scriptConfig: 'scriptConfig',
loadingError: 'error'
})
Expand Down
8 changes: 7 additions & 1 deletion web-src/js/admin/scripts-config/ScriptConfigForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<form class="script-config-form col">
<div class="row">
<TextField :config="nameField" class="col s6" v-model="newName"/>
<TextField :config="groupField" class="col s5 offset-s1" v-model="group"/>
</div>
<div class="row">
<TextField :config="scriptPathField" class="col s6" v-model="scriptPath"/>
Expand Down Expand Up @@ -33,13 +34,14 @@
import _ from 'lodash';
import {forEachKeyValue, isEmptyArray, isEmptyString, isNull} from '../../common';
import CheckBox from '../../components/checkbox'
import ChipsList from '../../components/ChipsList';
import TextArea from '../../components/TextArea';
import TextField from '../../components/textfield'
import ChipsList from '../../components/ChipsList';
import {
allowAllField,
bashFormattingField,
descriptionField,
groupField,
includeScriptField,
nameField,
requiresTerminalField,
Expand All @@ -62,6 +64,7 @@
return {
configCopy: null,
newName: null,
group: null,
scriptPath: null,
description: null,
workingDirectory: null,
Expand All @@ -71,6 +74,7 @@
allowedUsers: [],
allowAllUsers: true,
nameField,
groupField,
scriptPathField,
workDirField,
descriptionField,
Expand All @@ -84,6 +88,7 @@
mounted: function () {
const simpleFields = {
'newName': 'name',
'group': 'group',
'scriptPath': 'script_path',
'description': 'description',
'workingDirectory': 'working_directory',
Expand All @@ -108,6 +113,7 @@
immediate: true,
handler(config) {
this.newName = config.name;
this.group = config.group;
this.scriptPath = config['script_path'];
this.description = config['description'];
this.workingDirectory = config['working_directory'];
Expand Down
4 changes: 4 additions & 0 deletions web-src/js/admin/scripts-config/script-fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ export const nameField = {
name: 'Script name',
required: true
};
export const groupField = {
name: 'Group',
description: 'Aggregates scripts from the same group into the same category on UI'
};
export const scriptPathField = {
name: 'Script path',
required: true
Expand Down
13 changes: 9 additions & 4 deletions web-src/js/admin/scripts-config/scripts-module.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import axios from 'axios';


export const axiosInstance = axios.create();

export default {
state: {
scripts: [],
Expand All @@ -10,14 +13,16 @@ export default {
init({commit}) {
commit('SET_LOADING', true);

axios.get('scripts').then(({data: scripts}) => {
scripts.sort(function (name1, name2) {
axiosInstance.get('scripts').then(({data}) => {
const {scripts} = data;
let scriptNames = scripts.map(s => s.name);
scriptNames.sort(function (name1, name2) {
return name1.toLowerCase().localeCompare(name2.toLowerCase());
});

commit('SET_SCRIPTS', scripts);
commit('SET_SCRIPTS', scriptNames);
commit('SET_LOADING', false);
});
})
}
},
mutations: {
Expand Down
Loading

0 comments on commit 9275a12

Please sign in to comment.