From ff84547e2afcaadf23547ab7dabe98cc4be72106 Mon Sep 17 00:00:00 2001 From: Tim Garrels Date: Thu, 26 Sep 2019 16:27:57 +0200 Subject: [PATCH 01/20] Resolve "Allow using existing db user in setup_database.sh" --- utility/setup_database.sh | 82 +++++++++++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 12 deletions(-) diff --git a/utility/setup_database.sh b/utility/setup_database.sh index 6aad9ce1..fd1ccf25 100755 --- a/utility/setup_database.sh +++ b/utility/setup_database.sh @@ -6,25 +6,83 @@ echo "***********************************************" # while loops to ensure input not empty +# ------------- +# Database Name +# ------------- while [[ -z "$db_name" ]] do echo -n "Database Name: " read db_name done -while [[ -z "$username" ]] -do - echo -n "Username: " - read username -done +# ---------------- +# User Name Choice +# ---------------- -while [[ -z "$password" ]] +# Choice to create a new user or use and existing one +while [ "$use_existing_user" != "y" ] && [ "$use_existing_user" != "n" ] do - echo -n "Password: " - read -s password + echo -n "Do you want to use an existing db-user? [y|n] " + read use_existing_user done -sudo su -c "psql -c \"CREATE DATABASE $db_name;\"" postgres -sudo su -c "psql -c \"CREATE USER $username WITH PASSWORD '$password';\"" postgres -sudo su -c "psql -c \"GRANT ALL PRIVILEGES ON DATABASE $db_name to $username;\"" postgres -sudo su -c "psql -c \"ALTER USER $username CREATEDB;\"" postgres +# Existing user +if [ "$use_existing_user" == "y" ] +then + # Get and parse existing users + + # All users in the psql database + users="$( sudo su - postgres -c "psql -c \"SELECT u.usename FROM pg_catalog.pg_user u;\"")" + # Regex to remove table header and row count + regex="usename\ -+ \K(.*)(?=\ \(\d*\ rows?\))" + # Use grep to execute regex + users=$(echo $users | grep -Po "$regex") + # Create user array by + # Splitting users string at spaces + IFS=' ' + read -ra user_array <<< "$users" + + # Choose existing user + echo "" + choice=-1 + # Choice has to be in range and an integer + while (( $choice < 0 )) || (( $choice > ${#user_array[@] - 1} )) || ! [ "$choice" -eq "$choice" ] + do + echo "Choose an existing user" + idx=0 + for user in "${user_array[@]}"; do # access each element of array + echo "[$idx] $user" + idx=$(( idx + 1 )) + done + echo -n "> " + read choice + done + username=${user_array[$choice]} + +# New user +else + while [[ -z "$username" ]] + do + echo -n "New username: " + read username + done + + # ---------------- + # Password Choice + # ---------------- + while [[ -z "$password" ]] + do + echo -n "Password: " + read -s password + done + echo "" +fi + + +sudo su - postgres -c "psql -c \"CREATE DATABASE $db_name;\"" +if [ "$use_existing_user" == "n" ] +then + sudo su - postgres -c "psql -c \"CREATE USER $username WITH PASSWORD '$password';\"" +fi +sudo su - postgres -c "psql -c \"GRANT ALL PRIVILEGES ON DATABASE $db_name to $username;\"" +sudo su - postgres -c "psql -c \"ALTER USER $username CREATEDB;\"" From 0fe6a12e4ecdc3a31f6e26aa23f5318b57cdf674 Mon Sep 17 00:00:00 2001 From: Tim Garrels Date: Fri, 27 Sep 2019 14:26:49 +0200 Subject: [PATCH 02/20] add management command for adding remote sites (#314) --- CHANGELOG.rst | 1 + docs/source/app_projectroles_usage.rst | 7 + .../management/commands/addremotesite.py | 150 ++++++++++++++++++ 3 files changed, 158 insertions(+) create mode 100644 projectroles/management/commands/addremotesite.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6fe2946f..836b34ce 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -35,6 +35,7 @@ Added - Link for copying remote site secret token in remote site list (#332) - Project ownership transfer from member list (#287) - UI notification for disabled member management on target sites (#301) + - Management command ``addremotesite`` for adding remote sites (#314) - **Timeline** - Display event extra data as JSON (#6) - **Userprofile** diff --git a/docs/source/app_projectroles_usage.rst b/docs/source/app_projectroles_usage.rst index 0c1c1dc0..63ef4797 100644 --- a/docs/source/app_projectroles_usage.rst +++ b/docs/source/app_projectroles_usage.rst @@ -296,6 +296,13 @@ As Target Site The source site should be set up as above using the *Set Source Site* link, using the provided secret string as the access token. +Alternatively to creating the site in the UI, the following management command can be used +with additional arguments to create a remote site: + +.. code-block:: console + + $ ./manage.py addremotesite + After creating the source site, remote project metadata and member roles (for which access has been granted) can be accessed using the *Synchronize* link. Additionaly if the remote Source site is synchronized with multiple Target Sites, diff --git a/projectroles/management/commands/addremotesite.py b/projectroles/management/commands/addremotesite.py new file mode 100644 index 00000000..039f0b87 --- /dev/null +++ b/projectroles/management/commands/addremotesite.py @@ -0,0 +1,150 @@ +import logging +import re + +from django.contrib import auth +from django.core.management.base import BaseCommand +from django.db import transaction + +from projectroles.models import RemoteSite, SODAR_CONSTANTS + + +User = auth.get_user_model() +logger = logging.getLogger(__name__) + + +# SODAR constants +SITE_MODE_TARGET = SODAR_CONSTANTS['SITE_MODE_TARGET'] +SITE_MODE_SOURCE = SODAR_CONSTANTS['SITE_MODE_SOURCE'] +SITE_MODE_PEER = SODAR_CONSTANTS['SITE_MODE_PEER'] + + +class Command(BaseCommand): + help = 'Creates a remote site from given arguments' + + def add_arguments(self, parser): + # --------------------------- + # RemoteSite Model Argumments + # --------------------------- + parser.add_argument( + '-n', + '--name', + dest='name', + type=str, + required=True, + help='Name of the remote site', + ) + parser.add_argument( + '-u', + '--url', + dest='url', + type=str, + required=True, + help='Url of the remote site. Can be provided without protocol ' + 'prefix, defaults to http in that case', + ) + parser.add_argument( + '-m', + '--mode', + dest='mode', + type=str, + required=True, + help='Mode of the remote site', + ) + parser.add_argument( + '-d', + '--description', + dest='description', + type=str, + required=False, + default='', + help='Description of the remote site', + ) + parser.add_argument( + '-t', + '--token', + dest='secret', + type=str, + required=True, + help='Secret token of the remote site', + ) + parser.add_argument( + '-ud', + '--user-display', + dest='user_display', + default=True, + required=False, + type=bool, + help='User display of the remote site', + ) + # -------------------- + # Additional Arguments + # -------------------- + parser.add_argument( + '-s', + '--suppress-error', + dest='suppress_error', + required=False, + default=False, + action='store_true', + help='Suppresses error if site already exists', + ) + + def handle(self, *args, **options): + logger.info('Creating new Remote Site..') + + name = options['name'] + url = options['url'] + # Validate url + if not url.startswith("http://") and not url.startswith("https://"): + url = "".join(["http://", url]) + pattern = re.compile(r"(http|https)\:\/\/.*\..*") + if not pattern.match(url): + logger.error("Provided url '{}' seems not be a url".format(url)) + return + + mode = options['mode'].upper() + # Validate mode + if mode not in [SITE_MODE_SOURCE, SITE_MODE_TARGET]: + if mode in [SITE_MODE_PEER]: + logger.error("You cant create peer mode sites!") + else: + logger.error("Unkown mode '{}'".format(mode)) + return + + description = options['description'] + secret = options['secret'] + user_diplsay = options['user_display'] + suppress_error = options['suppress_error'] + + # Validate whether site exists + name_exists = bool(len(RemoteSite.objects.filter(name=name))) + url_exists = bool(len(RemoteSite.objects.filter(url=url))) + if name_exists or url_exists: + log_string = ( + "A site named '{}' already exists!".format(name) + if name_exists + else "A site with url '{}' already exists!".format(url) + ) + + if not suppress_error: + logger.error(log_string) + else: + logger.info(log_string) + return + + with transaction.atomic(): + create_values = { + 'name': name, + 'url': url, + 'mode': mode, + 'description': description, + 'secret': secret, + 'user_display': user_diplsay, + } + site = RemoteSite.objects.create(**create_values) + + logger.info( + 'Created a {name} remote site in {mode} mode'.format( + name=site.name, mode=site.mode + ) + ) From 1e5c0b64b952e3e00db9a1c9e42fb74797b8b70e Mon Sep 17 00:00:00 2001 From: Hendrik Bomhardt Date: Fri, 27 Sep 2019 15:41:37 +0200 Subject: [PATCH 03/20] add JSON support in app settings (#268) --- CHANGELOG.rst | 1 + example_project_app/plugins.py | 25 ++ projectroles/app_settings.py | 73 ++++-- projectroles/forms.py | 89 ++++++-- projectroles/models.py | 7 +- .../static/projectroles/css/projectroles.css | 6 +- projectroles/tests/test_app_settings_api.py | 213 +++++++++++------- projectroles/tests/test_models.py | 65 +++++- projectroles/tests/test_views.py | 120 ++++++++++ userprofile/forms.py | 24 +- userprofile/tests/test_views.py | 29 +++ 11 files changed, 512 insertions(+), 140 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 836b34ce..112b76d8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -36,6 +36,7 @@ Added - Project ownership transfer from member list (#287) - UI notification for disabled member management on target sites (#301) - Management command ``addremotesite`` for adding remote sites (#314) + - JSON support for app settings (#268) - **Timeline** - Display event extra data as JSON (#6) - **Userprofile** diff --git a/example_project_app/plugins.py b/example_project_app/plugins.py index 159c997b..de056f92 100644 --- a/example_project_app/plugins.py +++ b/example_project_app/plugins.py @@ -32,6 +32,18 @@ class ProjectAppPlugin(ProjectAppPluginPoint): 'description': 'Example project setting', 'user_modifiable': True, }, + 'project_json_setting': { + 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_PROJECT'], + 'type': 'JSON', + 'default': { + 'Example': 'Value', + 'list': [1, 2, 3, 4, 5], + 'level_6': False, + }, + 'description': 'Example project setting for JSON. Will accept ' + 'anything that json.dumps() can.', + 'user_modifiable': True, + }, 'project_hidden_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_PROJECT'], 'type': 'STRING', @@ -40,6 +52,19 @@ class ProjectAppPlugin(ProjectAppPluginPoint): 'description': 'Should not be displayed in forms', 'user_modifiable': False, }, + 'user_json_setting': { + 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_USER'], + 'type': 'JSON', + 'label': 'Json example', + 'default': { + 'Example': 'Value', + 'list': [1, 2, 3, 4, 5], + 'level_6': False, + }, + 'description': 'Example project setting for JSON. Will accept ' + 'anything that json.dumps() can.', + 'user_modifiable': True, + }, 'user_str_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_USER'], 'type': 'STRING', diff --git a/projectroles/app_settings.py b/projectroles/app_settings.py index 05bc5197..108edf43 100644 --- a/projectroles/app_settings.py +++ b/projectroles/app_settings.py @@ -1,4 +1,5 @@ """Project and user settings API""" +import json from projectroles.models import AppSetting, APP_SETTING_TYPES, SODAR_CONSTANTS from projectroles.plugins import get_app_plugin, get_active_plugins @@ -173,13 +174,16 @@ def set_app_setting( user=user, ) - if setting.value == value: + if setting.value == value or setting.value_json == value: return False if validate: cls.validate_setting(setting.type, value) - setting.value = value + if setting.type == 'JSON': + setting.value_json = value + else: + setting.value = value setting.save() return True @@ -204,15 +208,26 @@ def set_app_setting( if validate: cls.validate_setting(s_type, value) - setting = AppSetting( - app_plugin=app_plugin.get_model(), - project=project, - user=user, - name=setting_name, - type=s_type, - value=value, - user_modifiable=s_mod, - ) + if type == 'JSON': + setting = AppSetting( + app_plugin=app_plugin.get_model(), + project=project, + user=user, + name=setting_name, + type=s_type, + value_json=value, + user_modifiable=s_mod, + ) + else: + setting = AppSetting( + app_plugin=app_plugin.get_model(), + project=project, + user=user, + name=setting_name, + type=s_type, + value=value, + user_modifiable=s_mod, + ) setting.save() return True @@ -228,19 +243,31 @@ def validate_setting(cls, setting_type, setting_value): if setting_type not in APP_SETTING_TYPES: raise ValueError('Invalid setting type "{}"'.format(setting_type)) - if setting_type == 'BOOLEAN' and not isinstance(setting_value, bool): - raise ValueError( - 'Please enter a valid boolean value ({})'.format(setting_value) - ) - - if setting_type == 'INTEGER' and ( - not isinstance(setting_value, int) - and not str(setting_value).isdigit() - ): - raise ValueError( - 'Please enter a valid integer value ({})'.format(setting_value) - ) + elif setting_type == 'BOOLEAN': + if not isinstance(setting_value, bool): + raise ValueError( + 'Please enter a valid boolean value ({})'.format( + setting_value + ) + ) + elif setting_type == 'INTEGER': + if ( + not isinstance(setting_value, int) + and not str(setting_value).isdigit() + ): + raise ValueError( + 'Please enter a valid integer value ({})'.format( + setting_value + ) + ) + elif setting_type == 'JSON': + try: + json.dumps(setting_value) + except TypeError: + raise ValueError( + 'Please enter valid json ({})'.format(setting_value) + ) return True @classmethod diff --git a/projectroles/forms.py b/projectroles/forms.py index 82a8ef50..8542fb58 100644 --- a/projectroles/forms.py +++ b/projectroles/forms.py @@ -1,3 +1,5 @@ +import json + from django import forms from django.conf import settings from django.contrib import auth @@ -126,31 +128,61 @@ def __init__(self, project=None, current_user=None, *args, **kwargs): 'help_text': s_val['description'], } - if s_val['type'] == 'STRING': + if s_val['type'] == 'JSON': self.fields[s_field] = forms.CharField( - max_length=APP_SETTING_VAL_MAXLENGTH, **setting_kwargs - ) - - elif s_val['type'] == 'INTEGER': - self.fields[s_field] = forms.IntegerField(**setting_kwargs) - - elif s_val['type'] == 'BOOLEAN': - self.fields[s_field] = forms.BooleanField(**setting_kwargs) - - # Set initial value - if self.instance.pk: - self.initial[s_field] = self.app_settings.get_app_setting( - app_name=plugin.name, - setting_name=s_key, - project=self.instance, + widget=forms.Textarea( + attrs={'class': 'sodar-json-input'} + ), + **setting_kwargs ) - + if self.instance.pk: + self.initial[s_field] = json.dumps( + self.app_settings.get_app_setting( + app_name=plugin.name, + setting_name=s_key, + project=self.instance, + ) + ) + + else: + self.initial[s_field] = json.dumps( + self.app_settings.get_default_setting( + app_name=plugin.name, setting_name=s_key + ) + ) else: - self.initial[ - s_field - ] = self.app_settings.get_default_setting( - app_name=plugin.name, setting_name=s_key - ) + if s_val['type'] == 'STRING': + self.fields[s_field] = forms.CharField( + max_length=APP_SETTING_VAL_MAXLENGTH, + **setting_kwargs + ) + + elif s_val['type'] == 'INTEGER': + self.fields[s_field] = forms.IntegerField( + **setting_kwargs + ) + + elif s_val['type'] == 'BOOLEAN': + self.fields[s_field] = forms.BooleanField( + **setting_kwargs + ) + + # Set initial value + if self.instance.pk: + self.initial[ + s_field + ] = self.app_settings.get_app_setting( + app_name=plugin.name, + setting_name=s_key, + project=self.instance, + ) + + else: + self.initial[ + s_field + ] = self.app_settings.get_default_setting( + app_name=plugin.name, setting_name=s_key + ) # Access parent project if present parent_project = None @@ -289,6 +321,19 @@ def clean(self): s = p_settings[s_key] s_field = 'settings.{}.{}'.format(plugin.name, s_key) + if s['type'] == 'JSON': + # for some reason, there is a distinct possiblity, that the initial value has been discarded and we get '' as value. Seems to only happen in automated tests. Will catch that here. + if self.cleaned_data[s_field] == '': + self.cleaned_data[s_field] = '{}' + try: + self.cleaned_data[s_field] = json.loads( + self.cleaned_data[s_field] + ) + except json.JSONDecodeError as err: + raise forms.ValidationError( + 'Couldn\'t encode json.\n' + str(err) + ) + if not self.app_settings.validate_setting( setting_type=s['type'], setting_value=self.cleaned_data.get(s_field), diff --git a/projectroles/models.py b/projectroles/models.py index 392503a9..8dbf21da 100644 --- a/projectroles/models.py +++ b/projectroles/models.py @@ -26,11 +26,12 @@ # Local constants PROJECT_TYPE_CHOICES = [('CATEGORY', 'Category'), ('PROJECT', 'Project')] -APP_SETTING_TYPES = ['BOOLEAN', 'INTEGER', 'STRING'] +APP_SETTING_TYPES = ['BOOLEAN', 'INTEGER', 'STRING', 'JSON'] APP_SETTING_TYPE_CHOICES = [ ('BOOLEAN', 'Boolean'), ('INTEGER', 'Integer'), ('STRING', 'String'), + ('JSON', 'Json'), ] APP_SETTING_VAL_MAXLENGTH = 255 PROJECT_SEARCH_TYPES = ['project'] @@ -552,7 +553,6 @@ class AppSetting(models.Model): help_text='Value of the setting', ) - # TODO: Implement the use of this in release v0.7 (see #268) #: Optional JSON value for the setting value_json = JSONField( default=dict, help_text='Optional JSON value for the setting' @@ -611,6 +611,9 @@ def get_value(self): elif self.type == 'BOOLEAN': return bool(int(self.value)) + elif self.type == 'JSON': + return self.value_json + return self.value diff --git a/projectroles/static/projectroles/css/projectroles.css b/projectroles/static/projectroles/css/projectroles.css index 3e7a170c..5d2066a9 100644 --- a/projectroles/static/projectroles/css/projectroles.css +++ b/projectroles/static/projectroles/css/projectroles.css @@ -550,8 +550,10 @@ a.sodar-nav-link-last { border-bottom-left-radius: 0; } -/* Pagedown textarea height modification ("rows" attribute does not work) */ -textarea.wmd-input { +/* Pagedown and JSON setting textarea height modification */ +/* ("rows" attribute does not work) */ +textarea.wmd-input, +textarea.sodar-json-input { height: 175px; min-height: 175px; resize: vertical; diff --git a/projectroles/tests/test_app_settings_api.py b/projectroles/tests/test_app_settings_api.py index c6ec34e9..bc1b30dd 100644 --- a/projectroles/tests/test_app_settings_api.py +++ b/projectroles/tests/test_app_settings_api.py @@ -7,7 +7,6 @@ from ..app_settings import AppSettingAPI from .test_models import ProjectMixin, RoleAssignmentMixin, AppSettingMixin - # SODAR constants PROJECT_ROLE_OWNER = SODAR_CONSTANTS['PROJECT_ROLE_OWNER'] PROJECT_ROLE_DELEGATE = SODAR_CONSTANTS['PROJECT_ROLE_DELEGATE'] @@ -25,7 +24,6 @@ EXISTING_SETTING = 'project_bool_setting' EXAMPLE_APP_NAME = 'example_project_app' - # App settings API app_settings = AppSettingAPI() @@ -53,40 +51,67 @@ def setUp(self): ) # Init test setting - self.setting_str = self._make_setting( - app_name=EXAMPLE_APP_NAME, - project=self.project, - name='str_setting', - setting_type='STRING', - value='test', - ) - - # Init integer setting - self.setting_int = self._make_setting( - app_name=EXAMPLE_APP_NAME, - project=self.project, - name='int_setting', - setting_type='INTEGER', - value=170, - ) - - # Init boolean setting - self.setting_bool = self._make_setting( - app_name=EXAMPLE_APP_NAME, - project=self.project, - name='bool_setting', - setting_type='BOOLEAN', - value=True, - ) + setting_str_kwarg = { + 'app_name': EXAMPLE_APP_NAME, + 'project': self.project, + 'name': 'str_setting', + 'setting_type': 'STRING', + 'value': 'test', + 'update_value': 'better test', + 'non_valid_value': False, + } + setting_int_kwarg = { + 'app_name': EXAMPLE_APP_NAME, + 'project': self.project, + 'name': 'int_setting', + 'setting_type': 'INTEGER', + 'value': 210, + 'update_value': 420, + 'non_valid_value': 'Nan', + } + setting_bool_kwarg = { + 'app_name': EXAMPLE_APP_NAME, + 'project': self.project, + 'name': 'bool_setting', + 'setting_type': 'BOOLEAN', + 'value': True, + 'update_value': False, + 'non_valid_value': 69, + } + setting_json_kwarg = { + 'app_name': EXAMPLE_APP_NAME, + 'project': self.project, + 'name': 'json_setting', + 'setting_type': 'JSON', + 'value': {'Test': 'often'}, + 'update_value': {'Test_more': 'often_always'}, + 'non_valid_value': self.project, + } + self.settings = [ + setting_int_kwarg, + setting_json_kwarg, + setting_str_kwarg, + setting_bool_kwarg, + ] + for s in self.settings: + self._make_setting( + app_name=s['app_name'], + name=s['name'], + setting_type=s['setting_type'], + value=s['value'], + value_json=s['value'], + project=s['project'], + ) def test_get_project_setting(self): """Test get_app_setting()""" - val = app_settings.get_app_setting( - app_name=EXAMPLE_APP_NAME, - setting_name='str_setting', - project=self.project, - ) - self.assertEqual(self.setting_str.value, val) + for setting in self.settings: + val = app_settings.get_app_setting( + app_name=setting['app_name'], + setting_name=setting['name'], + project=setting['project'], + ) + self.assertEqual(setting['value'], val) def test_get_project_setting_default(self): """Test get_app_setting() with default value for existing setting""" @@ -113,58 +138,40 @@ def test_get_project_setting_nonexisting(self): def test_set_project_setting(self): """Test set_app_setting()""" - # Assert postcondition - val = app_settings.get_app_setting( - app_name=EXAMPLE_APP_NAME, - setting_name='str_setting', - project=self.project, - ) - self.assertEqual('test', val) - - ret = app_settings.set_app_setting( - app_name=EXAMPLE_APP_NAME, - setting_name='str_setting', - value='updated', - project=self.project, - ) - - self.assertEqual(ret, True) + for setting in self.settings: + ret = app_settings.set_app_setting( + app_name=setting['app_name'], + setting_name=setting['name'], + project=setting['project'], + value=setting['update_value'], + ) + self.assertEqual(ret, True) - # Assert postcondition - val = app_settings.get_app_setting( - app_name=EXAMPLE_APP_NAME, - setting_name='str_setting', - project=self.project, - ) - self.assertEqual('updated', val) + val = app_settings.get_app_setting( + app_name=setting['app_name'], + setting_name=setting['name'], + project=setting['project'], + ) + self.assertEqual(val, setting['update_value']) def test_set_project_setting_unchanged(self): """Test set_app_setting() with an unchnaged value""" - # Assert postcondition - val = app_settings.get_app_setting( - app_name=EXAMPLE_APP_NAME, - setting_name='str_setting', - project=self.project, - ) - self.assertEqual('test', val) - - ret = app_settings.set_app_setting( - app_name=EXAMPLE_APP_NAME, - setting_name='str_setting', - value='test', - project=self.project, - ) - - self.assertEqual(ret, False) + for setting in self.settings: + ret = app_settings.set_app_setting( + app_name=setting['app_name'], + setting_name=setting['name'], + project=setting['project'], + value=setting['value'], + ) + self.assertEqual(ret, False) - # Assert postcondition - val = app_settings.get_app_setting( - app_name=EXAMPLE_APP_NAME, - setting_name='str_setting', - project=self.project, - ) - self.assertEqual('test', val) + val = app_settings.get_app_setting( + app_name=setting['app_name'], + setting_name=setting['name'], + project=setting['project'], + ) + self.assertEqual(val, setting['value']) def test_set_project_setting_new(self): """Test set_app_setting() with a new but defined setting""" @@ -210,12 +217,21 @@ def test_set_project_setting_undefined(self): project=self.project, ) - def test_validate_project_setting_bool(self): + def test_validator(self): """Test validate_setting() with type BOOLEAN""" - self.assertEqual(app_settings.validate_setting('BOOLEAN', True), True) - - with self.assertRaises(ValueError): - app_settings.validate_setting('BOOLEAN', 'not a boolean') + for setting in self.settings: + self.assertEqual( + app_settings.validate_setting( + setting['setting_type'], setting['value'] + ), + True, + ) + if setting['setting_type'] == 'STRING': + continue + with self.assertRaises(ValueError): + app_settings.validate_setting( + setting['setting_type'], setting['non_valid_value'] + ) def test_validate_project_setting_int(self): """Test validate_setting() with type INTEGER""" @@ -242,6 +258,18 @@ def test_get_setting_defs_project(self): 'description': 'Example project setting', 'user_modifiable': True, }, + 'project_json_setting': { + 'scope': APP_SETTING_SCOPE_PROJECT, + 'type': 'JSON', + 'default': { + 'Example': 'Value', + 'list': [1, 2, 3, 4, 5], + 'level_6': False, + }, + 'description': 'Example project setting for JSON. Will accept ' + 'anything that json.dumps() can.', + 'user_modifiable': True, + }, 'project_hidden_setting': { 'scope': APP_SETTING_SCOPE_PROJECT, 'type': 'STRING', @@ -282,6 +310,19 @@ def test_get_setting_defs_user(self): 'default': False, 'user_modifiable': True, }, + 'user_json_setting': { + 'scope': APP_SETTING_SCOPE_USER, + 'type': 'JSON', + 'label': 'Json example', + 'default': { + 'Example': 'Value', + 'list': [1, 2, 3, 4, 5], + 'level_6': False, + }, + 'description': 'Example project setting for JSON. Will accept ' + 'anything that json.dumps() can.', + 'user_modifiable': True, + }, 'user_hidden_setting': { 'scope': APP_SETTING_SCOPE_USER, 'type': 'STRING', @@ -300,11 +341,11 @@ def test_get_setting_defs_modifiable(self): defs = app_settings.get_setting_defs( app_plugin, APP_SETTING_SCOPE_PROJECT ) - self.assertEqual(len(defs), 2) + self.assertEqual(len(defs), 3) defs = app_settings.get_setting_defs( app_plugin, APP_SETTING_SCOPE_PROJECT, user_modifiable=True ) - self.assertEqual(len(defs), 1) + self.assertEqual(len(defs), 2) def test_get_setting_defs_invalid(self): """Test get_setting_defs() with an invalid scope""" diff --git a/projectroles/tests/test_models.py b/projectroles/tests/test_models.py index d3293260..a5fe1d6e 100644 --- a/projectroles/tests/test_models.py +++ b/projectroles/tests/test_models.py @@ -120,6 +120,7 @@ def _make_setting( name, setting_type, value, + value_json={}, user_modifiable=True, project=None, user=None, @@ -131,7 +132,7 @@ def _make_setting( 'name': name, 'type': setting_type, 'value': value, - 'value_json': {}, # TODO: Implement usage in v0.7 + 'value_json': value_json, 'user_modifiable': user_modifiable, 'user': user, } @@ -817,6 +818,16 @@ def setUp(self): project=self.project, ) + # Init json setting + self.setting_json = self._make_setting( + app_name=EXAMPLE_APP_NAME, + name='json_setting', + setting_type='JSON', + value=None, + value_json={'Testing': 'good'}, + project=self.project, + ) + def test_initialization(self): """Test AppSetting initialization""" expected = { @@ -849,6 +860,22 @@ def test_initialization_integer(self): } self.assertEqual(model_to_dict(self.setting_int), expected) + def test_initialization_json(self): + """Test initialization with integer value""" + expected = { + 'id': self.setting_json.pk, + 'app_plugin': get_app_plugin(EXAMPLE_APP_NAME).get_model().pk, + 'project': self.project.pk, + 'name': 'json_setting', + 'type': 'JSON', + 'user': None, + 'value': None, + 'value_json': {'Testing': 'good'}, + 'user_modifiable': True, + 'sodar_uuid': self.setting_json.sodar_uuid, + } + self.assertEqual(model_to_dict(self.setting_json), expected) + def test__str__(self): """Test AppSetting __str__()""" expected = 'TestProject: {} / str_setting'.format(EXAMPLE_APP_NAME) @@ -879,6 +906,11 @@ def test_get_value_bool(self): self.assertIsInstance(val, bool) self.assertEqual(val, True) + def test_get_value_json(self): + """Test get_value() with type BOOLEAN""" + val = self.setting_json.get_value() + self.assertEqual(val, {'Testing': 'good'}) + class TestUserSetting( ProjectMixin, RoleAssignmentMixin, AppSettingMixin, TestCase @@ -917,6 +949,16 @@ def setUp(self): user=self.user, ) + # Init json setting + self.setting_json = self._make_setting( + app_name=EXAMPLE_APP_NAME, + name='json_setting', + setting_type='JSON', + value=None, + value_json={'Testing': 'good'}, + user=self.user, + ) + def test_initialization(self): """Test AppSetting initialization""" expected = { @@ -949,6 +991,22 @@ def test_initialization_integer(self): } self.assertEqual(model_to_dict(self.setting_int), expected) + def test_initialization_json(self): + """Test initialization with integer value""" + expected = { + 'id': self.setting_json.pk, + 'app_plugin': get_app_plugin(EXAMPLE_APP_NAME).get_model().pk, + 'project': None, + 'name': 'json_setting', + 'type': 'JSON', + 'user': self.user.pk, + 'value': None, + 'value_json': {'Testing': 'good'}, + 'user_modifiable': True, + 'sodar_uuid': self.setting_json.sodar_uuid, + } + self.assertEqual(model_to_dict(self.setting_json), expected) + def test__str__(self): """Test AppSetting __str__()""" expected = 'owner: {} / str_setting'.format(EXAMPLE_APP_NAME) @@ -979,6 +1037,11 @@ def test_get_value_bool(self): self.assertIsInstance(val, bool) self.assertEqual(val, True) + def test_get_value_json(self): + """Test get_value() with type BOOLEAN""" + val = self.setting_json.get_value() + self.assertEqual(val, {'Testing': 'good'}) + # TODO: Test manager diff --git a/projectroles/tests/test_views.py b/projectroles/tests/test_views.py index 5c85ac35..94e5332b 100644 --- a/projectroles/tests/test_views.py +++ b/projectroles/tests/test_views.py @@ -14,6 +14,7 @@ from test_plus.test import TestCase +from projectroles.app_settings import AppSettingAPI from .. import views from ..models import ( Project, @@ -41,6 +42,7 @@ ProjectUserTagMixin, RemoteSiteMixin, RemoteProjectMixin, + AppSettingMixin, ) from projectroles.utils import get_user_display_name @@ -78,6 +80,11 @@ SODAR_API_VERSION = '0.1' SODAR_API_VERSION_INVALID = '9.9' +EXAMPLE_APP_NAME = 'example_project_app' + +# App settings API +app_settings = AppSettingAPI() + # TODO: Refactor or remove this (API should be enough?) class ProjectSettingMixin: @@ -693,6 +700,119 @@ def test_update_project_owner(self): self.assertEqual(new_owner_ra.role, self.role_owner) +class TestProjectSettingsForm( + AppSettingMixin, TestViewsBase, ProjectMixin, RoleAssignmentMixin +): + """Tests for the project settings form.""" + + # NOTE: This assumes an example app is available + def setUp(self): + super().setUp() + # Init user & role + self.project = self._make_project( + 'TestProject', PROJECT_TYPE_PROJECT, None + ) + self.owner_as = self._make_assignment( + self.project, self.user, self.role_owner + ) + + # Init boolean setting + self.setting_bool = self._make_setting( + app_name=EXAMPLE_APP_NAME, + name='project_bool_setting', + setting_type='BOOLEAN', + value=True, + project=self.project, + ) + + # Init json setting + self.setting_json = self._make_setting( + app_name=EXAMPLE_APP_NAME, + name='project_json_setting', + setting_type='JSON', + value=None, + value_json={'Test': 'More'}, + project=self.project, + ) + + def testGet(self): + with self.login(self.user): + response = self.client.get( + reverse( + 'projectroles:update', + kwargs={'project': self.project.sodar_uuid}, + ) + ) + self.assertEqual(response.status_code, 200) + self.assertIsNotNone(response.context['form']) + self.assertIsNotNone( + response.context['form'].fields.get( + 'settings.%s.project_bool_setting' % EXAMPLE_APP_NAME + ) + ) + self.assertIsNotNone( + response.context['form'].fields.get( + 'settings.%s.project_json_setting' % EXAMPLE_APP_NAME + ) + ) + + def testPost(self): + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'project_bool_setting', project=self.project + ), + True, + ) + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'project_json_setting', project=self.project + ), + {'Test': 'More'}, + ) + + values = { + 'settings.%s.project_bool_setting' % EXAMPLE_APP_NAME: False, + 'settings.%s.project_json_setting' + % EXAMPLE_APP_NAME: '{"Test": "Less"}', + 'owner': self.user.sodar_uuid, + 'title': 'TestProject', + 'type': PROJECT_TYPE_PROJECT, + } + + with self.login(self.user): + response = self.client.post( + reverse( + 'projectroles:update', + kwargs={'project': self.project.sodar_uuid}, + ), + values, + ) + + # Assert redirect + with self.login(self.user): + self.assertRedirects( + response, + reverse( + 'projectroles:detail', + kwargs={'project': self.project.sodar_uuid}, + ), + ) + + # Assert settings state after update + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'project_bool_setting', project=self.project + ), + False, + ) + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'project_json_setting', project=self.project + ), + {'Test': 'Less'}, + ) + + class TestProjectRoleView(ProjectMixin, RoleAssignmentMixin, TestViewsBase): """Tests for project roles view""" diff --git a/userprofile/forms.py b/userprofile/forms.py index d29c618c..3942dcd5 100644 --- a/userprofile/forms.py +++ b/userprofile/forms.py @@ -1,3 +1,5 @@ +import json + from django import forms from projectroles.app_settings import AppSettingAPI @@ -61,6 +63,12 @@ def __init__(self, *args, **kwargs): elif s_val['type'] == 'BOOLEAN': self.fields[s_field] = forms.BooleanField(**field_kwarg) + elif s_val['type'] == 'JSON': + widget_attrs.update({'class': 'sodar-json-input'}) + self.fields[s_field] = forms.CharField( + widget=forms.Textarea(attrs=widget_attrs), **field_kwarg + ) + # Set initial value self.initial[s_field] = app_settings.get_app_setting( app_name=plugin.name, setting_name=s_key, user=self.user @@ -76,11 +84,19 @@ def clean(self): for s_key, s_val in p_settings.items(): s_field = 'settings.{}.{}'.format(plugin.name, s_key) - - if not app_settings.validate_setting( + if s_val['type'] == 'JSON': + try: + self.cleaned_data[s_field] = json.loads( + self.cleaned_data[s_field] + ) + except json.JSONDecodeError as err: + raise forms.ValidationError( + 'Couldn\'t encode json.\n' + str(err) + ) + + app_settings.validate_setting( setting_type=s_val['type'], setting_value=self.cleaned_data.get(s_field), - ): - self.add_error(s_field, 'Invalid value') + ) return self.cleaned_data diff --git a/userprofile/tests/test_views.py b/userprofile/tests/test_views.py index dec2ea6a..7d5f3c58 100644 --- a/userprofile/tests/test_views.py +++ b/userprofile/tests/test_views.py @@ -76,6 +76,16 @@ def setUp(self): user=self.user, ) + # Init json setting + self.setting_json = self._make_setting( + app_name=EXAMPLE_APP_NAME, + name='user_json_setting', + setting_type='JSON', + value=None, + value_json={'Test': 'More'}, + user=self.user, + ) + def testGet(self): with self.login(self.user): response = self.client.get(reverse('userprofile:settings_update')) @@ -96,6 +106,11 @@ def testGet(self): 'settings.%s.user_bool_setting' % EXAMPLE_APP_NAME ) ) + self.assertIsNotNone( + response.context['form'].fields.get( + 'settings.%s.user_json_setting' % EXAMPLE_APP_NAME + ) + ) def testPost(self): self.assertEqual( @@ -116,11 +131,19 @@ def testPost(self): ), True, ) + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'user_json_setting', user=self.user + ), + {'Test': 'More'}, + ) values = { 'settings.%s.user_str_setting' % EXAMPLE_APP_NAME: 'another-text', 'settings.%s.user_int_setting' % EXAMPLE_APP_NAME: '123', 'settings.%s.user_bool_setting' % EXAMPLE_APP_NAME: False, + 'settings.%s.user_json_setting' + % EXAMPLE_APP_NAME: '{"Test": "Less"}', } with self.login(self.user): @@ -151,3 +174,9 @@ def testPost(self): ), False, ) + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'user_json_setting', user=self.user + ), + {'Test': 'Less'}, + ) From 9f6e9e6954225a5097a5a8ced41bef64b9c49f1a Mon Sep 17 00:00:00 2001 From: "mikko.nieminen" Date: Fri, 27 Sep 2019 16:42:44 +0200 Subject: [PATCH 04/20] fix missing owner value in project update form (#355) --- projectroles/forms.py | 1 + 1 file changed, 1 insertion(+) diff --git a/projectroles/forms.py b/projectroles/forms.py index 8542fb58..eac3d142 100644 --- a/projectroles/forms.py +++ b/projectroles/forms.py @@ -234,6 +234,7 @@ def __init__(self, project=None, current_user=None, *args, **kwargs): # Set hidden project field for autocomplete self.initial['project'] = self.instance + self.initial['owner'] = self.instance.get_owner().user.sodar_uuid self.fields['owner'].widget = forms.HiddenInput() # Set initial value for parent From da3e64be34e8b697f4ba0e0238cddf4134d90286 Mon Sep 17 00:00:00 2001 From: "mikko.nieminen" Date: Mon, 30 Sep 2019 18:07:03 +0200 Subject: [PATCH 05/20] refactor AppSettingAPI, fix JSON app setting saving (#354), fix app settings tests, remove ProjectSettingMixin (#357), add run_taskflow.sh --- CHANGELOG.rst | 2 + docs/source/breaking_changes.rst | 7 + example_project_app/plugins.py | 64 +++++---- projectroles/app_settings.py | 96 ++++++++++--- projectroles/forms.py | 25 ++-- projectroles/tests/test_app_settings_api.py | 132 +++++++++++------ projectroles/tests/test_models.py | 6 +- projectroles/tests/test_views.py | 151 +++++++++++++------- projectroles/tests/test_views_taskflow.py | 12 +- projectroles/views.py | 24 +++- run.sh | 3 - run_taskflow.sh | 5 + userprofile/forms.py | 26 +++- 13 files changed, 387 insertions(+), 166 deletions(-) create mode 100755 run_taskflow.sh diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 112b76d8..5193305a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -56,6 +56,7 @@ Changed - Make sidebar hiding dynamic by content height (#316) - Replace ``login_and_redirect()`` in UI tests with a faster cookie based function (#323) - Refactor remote project display on details page (#196) + - Refactor AppSettingAPI (#268) - **Timeline** - Use common pagination template (#336) @@ -79,6 +80,7 @@ Removed - Duplicate database indexes from ``RoleAssignment`` (#285) - Deprecated ``get_setting()`` tag from ``projectroles_common_tags`` (#283) - Project owner change from project updating form (#287) + - ``ProjectSettingMixin`` from ``projectoles.tests.test_views`` (#357) v0.6.2 (2019-06-21) diff --git a/docs/source/breaking_changes.rst b/docs/source/breaking_changes.rst index 7b48c0a4..bda38d2f 100644 --- a/docs/source/breaking_changes.rst +++ b/docs/source/breaking_changes.rst @@ -34,6 +34,13 @@ The deprecated ``get_setting()`` template tag has been removed from ``projectroles_common_tags``. Please use ``get_django_setting()`` in your templates instead. +ProjectSettingMixin Removed +--------------------------- + +In ``projectroles.tests.test_views``, the deprecated ``ProjectSettingMixin`` +was removed. If you need to populate app settings in your tests, use the +``AppSettingAPI`` instead. + v0.6.2 (2019-06-21) =================== diff --git a/example_project_app/plugins.py b/example_project_app/plugins.py index de056f92..b0dec428 100644 --- a/example_project_app/plugins.py +++ b/example_project_app/plugins.py @@ -25,74 +25,90 @@ class ProjectAppPlugin(ProjectAppPluginPoint): #: Project and user settings app_settings = { + 'project_string_setting': { + 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_PROJECT'], + 'type': 'STRING', + 'label': 'String Setting', + 'default': '', + 'description': 'Example string project setting', + 'user_modifiable': True, + }, + 'project_int_setting': { + 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_PROJECT'], + 'type': 'INTEGER', + 'label': 'Integer Setting', + 'default': 0, + 'description': 'Example integer project setting', + 'user_modifiable': True, + }, 'project_bool_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_PROJECT'], 'type': 'BOOLEAN', + 'label': 'Boolean Setting', 'default': False, - 'description': 'Example project setting', + 'description': 'Example boolean project setting', 'user_modifiable': True, }, 'project_json_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_PROJECT'], 'type': 'JSON', + 'label': 'JSON Setting', 'default': { 'Example': 'Value', 'list': [1, 2, 3, 4, 5], 'level_6': False, }, - 'description': 'Example project setting for JSON. Will accept ' - 'anything that json.dumps() can.', + 'description': 'Example JSON project setting', 'user_modifiable': True, }, 'project_hidden_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_PROJECT'], 'type': 'STRING', - 'label': 'Hidden project setting', 'default': '', - 'description': 'Should not be displayed in forms', + 'description': 'Example hidden project setting', 'user_modifiable': False, }, - 'user_json_setting': { - 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_USER'], - 'type': 'JSON', - 'label': 'Json example', - 'default': { - 'Example': 'Value', - 'list': [1, 2, 3, 4, 5], - 'level_6': False, - }, - 'description': 'Example project setting for JSON. Will accept ' - 'anything that json.dumps() can.', - 'user_modifiable': True, - }, 'user_str_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_USER'], 'type': 'STRING', - 'label': 'String example', + 'label': 'String Setting', 'default': '', - 'description': 'Example user setting', + 'description': 'Example string user setting', 'user_modifiable': True, }, 'user_int_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_USER'], 'type': 'INTEGER', - 'label': 'Int example', + 'label': 'Integer Setting', 'default': 0, + 'description': 'Example integer user setting', 'user_modifiable': True, }, 'user_bool_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_USER'], 'type': 'BOOLEAN', - 'label': 'Bool Example', + 'label': 'Boolean Setting', 'default': False, + 'description': 'Example boolean user setting', + 'user_modifiable': True, + }, + 'user_json_setting': { + 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_USER'], + 'type': 'JSON', + 'label': 'JSON Setting', + 'default': { + 'Example': 'Value', + 'list': [1, 2, 3, 4, 5], + 'level_6': False, + }, + 'description': 'Example JSON project setting', 'user_modifiable': True, }, 'user_hidden_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_USER'], 'type': 'STRING', - 'label': 'Hidden user setting', 'default': '', - 'description': 'Should not be displayed in forms', + 'description': 'Example hidden user setting', 'user_modifiable': False, }, } diff --git a/projectroles/app_settings.py b/projectroles/app_settings.py index 108edf43..6534119a 100644 --- a/projectroles/app_settings.py +++ b/projectroles/app_settings.py @@ -43,19 +43,69 @@ def _check_scope(cls, scope): raise ValueError('Invalid scope "{}"'.format(scope)) @classmethod - def get_default_setting(cls, app_name, setting_name): + def _get_json_value(cls, value): + """ + Return JSON value as dict regardless of input type + + :param value: Original value (string or dict) + :raise: json.decoder.JSONDecodeError if string value is not valid JSON + :raise: ValueError if value type is not recognized + :return: dict + """ + if not value: + return {} + + elif isinstance(value, str): + return json.loads(value) + + elif isinstance(value, dict): + return value + + raise ValueError( + 'Invalid type for value: "{}" ({})'.format(value, type(value)) + ) + + @classmethod + def _compare_value(cls, setting_obj, input_value): + """ + Compare input value to value in an AppSetting object + + :param setting_obj: AppSetting object + :param input_value: Input value (string, int, bool or dict) + :return: Bool + """ + if setting_obj.type == 'JSON': + return setting_obj.value_json == cls._get_json_value(input_value) + + elif setting_obj.type == 'BOOLEAN': + # TODO: Also do conversion on input value here if necessary + return bool(int(setting_obj.value)) == input_value + + return setting_obj.value == str(input_value) + + @classmethod + def get_default_setting(cls, app_name, setting_name, post_safe=False): """ Get default setting value from an app plugin. :param app_name: App name (string, must correspond to "name" in app plugin) :param setting_name: Setting name (string) + :param post_safe: Whether a POST safe value should be returned (bool) :return: Setting value (string, integer or boolean) :raise: KeyError if nothing is found with setting_name """ app_plugin = get_app_plugin(app_name) if setting_name in app_plugin.app_settings: + if ( + post_safe + and app_plugin.app_settings[setting_name]['type'] == 'JSON' + ): + return json.dumps( + app_plugin.app_settings[setting_name]['default'] + ) + return app_plugin.app_settings[setting_name]['default'] raise KeyError( @@ -65,7 +115,9 @@ def get_default_setting(cls, app_name, setting_name): ) @classmethod - def get_app_setting(cls, app_name, setting_name, project=None, user=None): + def get_app_setting( + cls, app_name, setting_name, project=None, user=None, post_safe=False + ): """ Return app setting value for a project or an user. If not set, return default. @@ -75,27 +127,33 @@ def get_app_setting(cls, app_name, setting_name, project=None, user=None): :param setting_name: Setting name (string) :param project: Project object (can be None) :param user: User object (can be None) + :param post_safe: Whether a POST safe value should be returned (bool) :return: String or None :raise: KeyError if nothing is found with setting_name """ try: - return AppSetting.objects.get_setting_value( + val = AppSetting.objects.get_setting_value( app_name, setting_name, project=project, user=user ) except AppSetting.DoesNotExist: - pass + val = cls.get_default_setting(app_name, setting_name, post_safe) + + # Handle post_safe for dict values (JSON) + if post_safe and isinstance(val, dict): + return json.dumps(val) - return cls.get_default_setting(app_name, setting_name) + return val @classmethod - def get_all_settings(cls, project=None, user=None): + def get_all_settings(cls, project=None, user=None, post_safe=False): """ Return all setting values. If the value is not found, return the default. :param project: Project object (can be None) :param user: User object (can be None) + :param post_safe: Whether POST safe values should be returned (bool) :return: Dict :raise: ValueError if neither project nor user are set """ @@ -110,17 +168,20 @@ def get_all_settings(cls, project=None, user=None): for s_key in p_settings: ret[ 'settings.{}.{}'.format(plugin.name, s_key) - ] = cls.get_app_setting(plugin.name, s_key, project, user) + ] = cls.get_app_setting( + plugin.name, s_key, project, user, post_safe + ) return ret @classmethod - def get_all_defaults(cls, scope): + def get_all_defaults(cls, scope, post_safe=False): """ Get all default settings for a scope. - :param scope: - :return: + :param scope: Setting scope (PROJECT or USER) + :param post_safe: Whether POST safe values should be returned (bool) + :return: Dict """ cls._check_scope(scope) @@ -133,7 +194,7 @@ def get_all_defaults(cls, scope): for s_key in p_settings: ret[ 'settings.{}.{}'.format(plugin.name, s_key) - ] = cls.get_default_setting(plugin.name, s_key) + ] = cls.get_default_setting(plugin.name, s_key, post_safe) return ret @@ -174,16 +235,18 @@ def set_app_setting( user=user, ) - if setting.value == value or setting.value_json == value: + if cls._compare_value(setting, value): return False if validate: cls.validate_setting(setting.type, value) if setting.type == 'JSON': - setting.value_json = value + setting.value_json = cls._get_json_value(value) + else: setting.value = value + setting.save() return True @@ -208,14 +271,15 @@ def set_app_setting( if validate: cls.validate_setting(s_type, value) - if type == 'JSON': + # TODO: Simplfy by creating with **values instead + if s_type == 'JSON': setting = AppSetting( app_plugin=app_plugin.get_model(), project=project, user=user, name=setting_name, type=s_type, - value_json=value, + value_json=cls._get_json_value(value), user_modifiable=s_mod, ) else: @@ -266,7 +330,7 @@ def validate_setting(cls, setting_type, setting_value): json.dumps(setting_value) except TypeError: raise ValueError( - 'Please enter valid json ({})'.format(setting_value) + 'Please enter valid JSON ({})'.format(setting_value) ) return True diff --git a/projectroles/forms.py b/projectroles/forms.py index eac3d142..e91e7a4b 100644 --- a/projectroles/forms.py +++ b/projectroles/forms.py @@ -167,7 +167,7 @@ def __init__(self, project=None, current_user=None, *args, **kwargs): **setting_kwargs ) - # Set initial value + # Set initial value if self.instance.pk: self.initial[ s_field @@ -234,6 +234,7 @@ def __init__(self, project=None, current_user=None, *args, **kwargs): # Set hidden project field for autocomplete self.initial['project'] = self.instance + # Set owner value but hide the field (updating via member form) self.initial['owner'] = self.instance.get_owner().user.sodar_uuid self.fields['owner'].widget = forms.HiddenInput() @@ -318,25 +319,31 @@ def clean(self): plugin, APP_SETTING_SCOPE_PROJECT, user_modifiable=True ) - for s_key in p_settings: - s = p_settings[s_key] + for s_key, s_val in p_settings.items(): s_field = 'settings.{}.{}'.format(plugin.name, s_key) - if s['type'] == 'JSON': - # for some reason, there is a distinct possiblity, that the initial value has been discarded and we get '' as value. Seems to only happen in automated tests. Will catch that here. - if self.cleaned_data[s_field] == '': + if s_val['type'] == 'JSON': + # for some reason, there is a distinct possiblity, that the + # initial value has been discarded and we get '' as value. + # Seems to only happen in automated tests. Will catch that + # here. + if not self.cleaned_data.get(s_field): self.cleaned_data[s_field] = '{}' + try: + self.cleaned_data[s_field] = json.loads( - self.cleaned_data[s_field] + self.cleaned_data.get(s_field) ) + except json.JSONDecodeError as err: + # TODO: Shouldn't we use add_error() instead? raise forms.ValidationError( - 'Couldn\'t encode json.\n' + str(err) + 'Couldn\'t encode JSON\n' + str(err) ) if not self.app_settings.validate_setting( - setting_type=s['type'], + setting_type=s_val['type'], setting_value=self.cleaned_data.get(s_field), ): self.add_error(s_field, 'Invalid value') diff --git a/projectroles/tests/test_app_settings_api.py b/projectroles/tests/test_app_settings_api.py index bc1b30dd..ded57372 100644 --- a/projectroles/tests/test_app_settings_api.py +++ b/projectroles/tests/test_app_settings_api.py @@ -51,55 +51,59 @@ def setUp(self): ) # Init test setting - setting_str_kwarg = { + self.setting_str_values = { 'app_name': EXAMPLE_APP_NAME, 'project': self.project, - 'name': 'str_setting', + 'name': 'project_string_setting', 'setting_type': 'STRING', 'value': 'test', 'update_value': 'better test', 'non_valid_value': False, } - setting_int_kwarg = { + self.setting_int_values = { 'app_name': EXAMPLE_APP_NAME, 'project': self.project, - 'name': 'int_setting', + 'name': 'project_int_setting', 'setting_type': 'INTEGER', - 'value': 210, - 'update_value': 420, + 'value': 0, + 'update_value': 170, 'non_valid_value': 'Nan', } - setting_bool_kwarg = { + self.setting_bool_values = { 'app_name': EXAMPLE_APP_NAME, 'project': self.project, - 'name': 'bool_setting', + 'name': 'project_bool_setting', 'setting_type': 'BOOLEAN', - 'value': True, - 'update_value': False, - 'non_valid_value': 69, + 'value': False, + 'update_value': True, + 'non_valid_value': 170, } - setting_json_kwarg = { + self.setting_json_values = { 'app_name': EXAMPLE_APP_NAME, 'project': self.project, - 'name': 'json_setting', + 'name': 'project_json_setting', 'setting_type': 'JSON', - 'value': {'Test': 'often'}, + 'value': { + 'Example': 'Value', + 'list': [1, 2, 3, 4, 5], + 'level_6': False, + }, 'update_value': {'Test_more': 'often_always'}, 'non_valid_value': self.project, } self.settings = [ - setting_int_kwarg, - setting_json_kwarg, - setting_str_kwarg, - setting_bool_kwarg, + self.setting_int_values, + self.setting_json_values, + self.setting_str_values, + self.setting_bool_values, ] for s in self.settings: self._make_setting( app_name=s['app_name'], name=s['name'], setting_type=s['setting_type'], - value=s['value'], - value_json=s['value'], + value=s['value'] if s['setting_type'] != 'JSON' else '', + value_json=s['value'] if s['setting_type'] == 'JSON' else {}, project=s['project'], ) @@ -111,7 +115,7 @@ def test_get_project_setting(self): setting_name=setting['name'], project=setting['project'], ) - self.assertEqual(setting['value'], val) + self.assertEqual(val, setting['value']) def test_get_project_setting_default(self): """Test get_app_setting() with default value for existing setting""" @@ -135,6 +139,16 @@ def test_get_project_setting_nonexisting(self): project=self.project, ) + def test_get_project_setting_post_safe(self): + """Test get_app_setting() with JSON setting and post_safe=True""" + val = app_settings.get_app_setting( + app_name=self.setting_json_values['app_name'], + setting_name=self.setting_json_values['name'], + project=self.setting_json_values['project'], + post_safe=True, + ) + self.assertEqual(type(val), str) + def test_set_project_setting(self): """Test set_app_setting()""" @@ -164,25 +178,37 @@ def test_set_project_setting_unchanged(self): project=setting['project'], value=setting['value'], ) - self.assertEqual(ret, False) + self.assertEqual( + ret, + False, + msg='setting={}.{}'.format( + setting['app_name'], setting['name'] + ), + ) val = app_settings.get_app_setting( app_name=setting['app_name'], setting_name=setting['name'], project=setting['project'], ) - self.assertEqual(val, setting['value']) + self.assertEqual( + val, + setting['value'], + msg='setting={}.{}'.format( + setting['app_name'], setting['name'] + ), + ) def test_set_project_setting_new(self): """Test set_app_setting() with a new but defined setting""" # Assert precondition - with self.assertRaises(AppSetting.DoesNotExist): - AppSetting.objects.get( - app_plugin=get_app_plugin(EXAMPLE_APP_NAME).get_model(), - project=self.project, - name=EXISTING_SETTING, - ) + val = AppSetting.objects.get( + app_plugin=get_app_plugin(EXAMPLE_APP_NAME).get_model(), + project=self.project, + name=EXISTING_SETTING, + ).value + self.assertEqual(bool(int(val)), False) ret = app_settings.set_app_setting( app_name=EXAMPLE_APP_NAME, @@ -251,31 +277,47 @@ def test_get_setting_defs_project(self): """Test get_setting_defs() with the PROJECT scope""" app_plugin = get_app_plugin(EXAMPLE_APP_NAME) expected = { + 'project_string_setting': { + 'scope': APP_SETTING_SCOPE_PROJECT, + 'type': 'STRING', + 'label': 'String Setting', + 'default': '', + 'description': 'Example string project setting', + 'user_modifiable': True, + }, + 'project_int_setting': { + 'scope': APP_SETTING_SCOPE_PROJECT, + 'type': 'INTEGER', + 'label': 'Integer Setting', + 'default': 0, + 'description': 'Example integer project setting', + 'user_modifiable': True, + }, 'project_bool_setting': { 'scope': APP_SETTING_SCOPE_PROJECT, 'type': 'BOOLEAN', + 'label': 'Boolean Setting', 'default': False, - 'description': 'Example project setting', + 'description': 'Example boolean project setting', 'user_modifiable': True, }, 'project_json_setting': { 'scope': APP_SETTING_SCOPE_PROJECT, 'type': 'JSON', + 'label': 'JSON Setting', 'default': { 'Example': 'Value', 'list': [1, 2, 3, 4, 5], 'level_6': False, }, - 'description': 'Example project setting for JSON. Will accept ' - 'anything that json.dumps() can.', + 'description': 'Example JSON project setting', 'user_modifiable': True, }, 'project_hidden_setting': { 'scope': APP_SETTING_SCOPE_PROJECT, 'type': 'STRING', - 'label': 'Hidden project setting', 'default': '', - 'description': 'Should not be displayed in forms', + 'description': 'Example hidden project setting', 'user_modifiable': False, }, } @@ -291,44 +333,44 @@ def test_get_setting_defs_user(self): 'user_str_setting': { 'scope': APP_SETTING_SCOPE_USER, 'type': 'STRING', - 'label': 'String example', + 'label': 'String Setting', 'default': '', - 'description': 'Example user setting', + 'description': 'Example string user setting', 'user_modifiable': True, }, 'user_int_setting': { 'scope': APP_SETTING_SCOPE_USER, 'type': 'INTEGER', - 'label': 'Int example', + 'label': 'Integer Setting', 'default': 0, + 'description': 'Example integer user setting', 'user_modifiable': True, }, 'user_bool_setting': { 'scope': APP_SETTING_SCOPE_USER, 'type': 'BOOLEAN', - 'label': 'Bool Example', + 'label': 'Boolean Setting', 'default': False, + 'description': 'Example boolean user setting', 'user_modifiable': True, }, 'user_json_setting': { 'scope': APP_SETTING_SCOPE_USER, 'type': 'JSON', - 'label': 'Json example', + 'label': 'JSON Setting', 'default': { 'Example': 'Value', 'list': [1, 2, 3, 4, 5], 'level_6': False, }, - 'description': 'Example project setting for JSON. Will accept ' - 'anything that json.dumps() can.', + 'description': 'Example JSON project setting', 'user_modifiable': True, }, 'user_hidden_setting': { 'scope': APP_SETTING_SCOPE_USER, 'type': 'STRING', - 'label': 'Hidden user setting', 'default': '', - 'description': 'Should not be displayed in forms', + 'description': 'Example hidden user setting', 'user_modifiable': False, }, } @@ -341,11 +383,11 @@ def test_get_setting_defs_modifiable(self): defs = app_settings.get_setting_defs( app_plugin, APP_SETTING_SCOPE_PROJECT ) - self.assertEqual(len(defs), 3) + self.assertEqual(len(defs), 5) defs = app_settings.get_setting_defs( app_plugin, APP_SETTING_SCOPE_PROJECT, user_modifiable=True ) - self.assertEqual(len(defs), 2) + self.assertEqual(len(defs), 4) def test_get_setting_defs_invalid(self): """Test get_setting_defs() with an invalid scope""" diff --git a/projectroles/tests/test_models.py b/projectroles/tests/test_models.py index a5fe1d6e..47c8404c 100644 --- a/projectroles/tests/test_models.py +++ b/projectroles/tests/test_models.py @@ -818,7 +818,7 @@ def setUp(self): project=self.project, ) - # Init json setting + # Init JSON setting self.setting_json = self._make_setting( app_name=EXAMPLE_APP_NAME, name='json_setting', @@ -861,7 +861,7 @@ def test_initialization_integer(self): self.assertEqual(model_to_dict(self.setting_int), expected) def test_initialization_json(self): - """Test initialization with integer value""" + """Test initialization with JSON value""" expected = { 'id': self.setting_json.pk, 'app_plugin': get_app_plugin(EXAMPLE_APP_NAME).get_model().pk, @@ -907,7 +907,7 @@ def test_get_value_bool(self): self.assertEqual(val, True) def test_get_value_json(self): - """Test get_value() with type BOOLEAN""" + """Test get_value() with type JSON""" val = self.setting_json.get_value() self.assertEqual(val, {'Testing': 'good'}) diff --git a/projectroles/tests/test_views.py b/projectroles/tests/test_views.py index 94e5332b..3eb6e90a 100644 --- a/projectroles/tests/test_views.py +++ b/projectroles/tests/test_views.py @@ -59,6 +59,7 @@ SUBMIT_STATUS_PENDING_TASKFLOW = SODAR_CONSTANTS['SUBMIT_STATUS_PENDING'] SITE_MODE_TARGET = SODAR_CONSTANTS['SITE_MODE_TARGET'] SITE_MODE_SOURCE = SODAR_CONSTANTS['SITE_MODE_SOURCE'] +APP_SETTING_SCOPE_PROJECT = SODAR_CONSTANTS['APP_SETTING_SCOPE_PROJECT'] # Local constants INVITE_EMAIL = 'test@example.com' @@ -86,33 +87,6 @@ app_settings = AppSettingAPI() -# TODO: Refactor or remove this (API should be enough?) -class ProjectSettingMixin: - """Helper mixin for Project settings""" - - @classmethod - def _get_settings(cls): - """Get settings""" - ret = {} - - app_plugins = sorted( - [ - p - for p in ProjectAppPluginPoint.get_plugins() - if p.project_settings - ], - key=lambda x: x.name, - ) - - for p in app_plugins: - for s_key in p.project_settings: - ret[ - 'settings.{}.{}'.format(p.name, s_key) - ] = p.project_settings[s_key]['default'] - - return ret - - class KnoxAuthMixin: """Helper mixin for API views with Knox token authorization""" @@ -350,7 +324,7 @@ def test_render(self): class TestProjectCreateView( - ProjectMixin, RoleAssignmentMixin, ProjectSettingMixin, TestViewsBase + ProjectMixin, RoleAssignmentMixin, TestViewsBase ): """Tests for Project creation view""" @@ -409,8 +383,7 @@ def test_render_sub(self): self.assertIsInstance(form.fields['parent'].widget, HiddenInput) def test_render_sub_project(self): - """Test rendering of Project creation form if creating a subproject - under a project (should fail with redirect)""" + """Test rendering of Project creation form if creating a subproject under a project (should fail with redirect)""" self.project = self._make_project( 'TestProject', PROJECT_TYPE_PROJECT, None ) @@ -448,7 +421,11 @@ def test_create_top_level_category(self): } # Add settings values - values.update(self._get_settings()) + values.update( + app_settings.get_all_defaults( + APP_SETTING_SCOPE_PROJECT, post_safe=True + ) + ) with self.login(self.user): response = self.client.post(reverse('projectroles:create'), values) @@ -501,9 +478,8 @@ def test_create_top_level_category(self): ) def test_create_project(self): - """Test Project creation with taskflow""" - # create category (no assertions, cause that is covered by other testcase) - + """Test Project creation""" + # Create category # Issue POST request values = { 'title': 'TestCategory', @@ -515,7 +491,11 @@ def test_create_project(self): } # Add settings values - values.update(self._get_settings()) + values.update( + app_settings.get_all_defaults( + APP_SETTING_SCOPE_PROJECT, post_safe=True + ) + ) with self.login(self.user): self.client.post(reverse('projectroles:create'), values) @@ -532,7 +512,11 @@ def test_create_project(self): } # Add settings values - values.update(self._get_settings()) + values.update( + app_settings.get_all_defaults( + APP_SETTING_SCOPE_PROJECT, post_safe=True + ) + ) with self.login(self.user): self.client.post( @@ -581,7 +565,7 @@ def test_create_project(self): class TestProjectUpdateView( - ProjectMixin, RoleAssignmentMixin, ProjectSettingMixin, TestViewsBase + ProjectMixin, RoleAssignmentMixin, TestViewsBase ): """Tests for Project updating view""" @@ -628,7 +612,9 @@ def test_update_project(self): values['owner'] = self.user.sodar_uuid # NOTE: Must add owner # Add settings values - values.update(self._get_settings()) + values.update( + app_settings.get_all_settings(project=self.project, post_safe=True) + ) with self.login(self.user): response = self.client.post( @@ -673,15 +659,16 @@ def test_update_project(self): ) def test_update_project_owner(self): - """Light version of test_update_project. Only to test if a mail is send, - when updating the owner of a project and that the project owner is updated correctly""" + """Test email sending on project owner update""" new_owner = self.make_user('New Owner') values = model_to_dict(self.project) values['owner'] = new_owner.sodar_uuid # NOTE: Must add owner # Add settings values - values.update(self._get_settings()) + values.update( + app_settings.get_all_settings(project=self.project, post_safe=True) + ) with self.login(self.user): self.client.post( @@ -703,7 +690,7 @@ def test_update_project_owner(self): class TestProjectSettingsForm( AppSettingMixin, TestViewsBase, ProjectMixin, RoleAssignmentMixin ): - """Tests for the project settings form.""" + """Tests for project settings in the project create/update view""" # NOTE: This assumes an example app is available def setUp(self): @@ -716,12 +703,30 @@ def setUp(self): self.project, self.user, self.role_owner ) + # Init string setting + self.setting_bool = self._make_setting( + app_name=EXAMPLE_APP_NAME, + name='project_string_setting', + setting_type='STRING', + value='', + project=self.project, + ) + + # Init integer setting + self.setting_bool = self._make_setting( + app_name=EXAMPLE_APP_NAME, + name='project_int_setting', + setting_type='INTEGER', + value='0', + project=self.project, + ) + # Init boolean setting self.setting_bool = self._make_setting( app_name=EXAMPLE_APP_NAME, name='project_bool_setting', setting_type='BOOLEAN', - value=True, + value=False, project=self.project, ) @@ -731,11 +736,16 @@ def setUp(self): name='project_json_setting', setting_type='JSON', value=None, - value_json={'Test': 'More'}, + value_json={ + 'Example': 'Value', + 'list': [1, 2, 3, 4, 5], + 'level_6': False, + }, project=self.project, ) - def testGet(self): + def test_get(self): + """Test rendering the settings values""" with self.login(self.user): response = self.client.get( reverse( @@ -745,6 +755,16 @@ def testGet(self): ) self.assertEqual(response.status_code, 200) self.assertIsNotNone(response.context['form']) + self.assertIsNotNone( + response.context['form'].fields.get( + 'settings.%s.project_string_setting' % EXAMPLE_APP_NAME + ) + ) + self.assertIsNotNone( + response.context['form'].fields.get( + 'settings.%s.project_int_setting' % EXAMPLE_APP_NAME + ) + ) self.assertIsNotNone( response.context['form'].fields.get( 'settings.%s.project_bool_setting' % EXAMPLE_APP_NAME @@ -756,24 +776,39 @@ def testGet(self): ) ) - def testPost(self): + def test_post(self): + """Test modifying the settings values""" + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'project_string_setting', project=self.project + ), + '', + ) + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'project_int_setting', project=self.project + ), + 0, + ) self.assertEqual( app_settings.get_app_setting( EXAMPLE_APP_NAME, 'project_bool_setting', project=self.project ), - True, + False, ) self.assertEqual( app_settings.get_app_setting( EXAMPLE_APP_NAME, 'project_json_setting', project=self.project ), - {'Test': 'More'}, + {'Example': 'Value', 'list': [1, 2, 3, 4, 5], 'level_6': False}, ) values = { - 'settings.%s.project_bool_setting' % EXAMPLE_APP_NAME: False, + 'settings.%s.project_string_setting' % EXAMPLE_APP_NAME: 'updated', + 'settings.%s.project_int_setting' % EXAMPLE_APP_NAME: 170, + 'settings.%s.project_bool_setting' % EXAMPLE_APP_NAME: True, 'settings.%s.project_json_setting' - % EXAMPLE_APP_NAME: '{"Test": "Less"}', + % EXAMPLE_APP_NAME: '{"Test": "Updated"}', 'owner': self.user.sodar_uuid, 'title': 'TestProject', 'type': PROJECT_TYPE_PROJECT, @@ -799,17 +834,29 @@ def testPost(self): ) # Assert settings state after update + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'project_string_setting', project=self.project + ), + 'updated', + ) + self.assertEqual( + app_settings.get_app_setting( + EXAMPLE_APP_NAME, 'project_int_setting', project=self.project + ), + 170, + ) self.assertEqual( app_settings.get_app_setting( EXAMPLE_APP_NAME, 'project_bool_setting', project=self.project ), - False, + True, ) self.assertEqual( app_settings.get_app_setting( EXAMPLE_APP_NAME, 'project_json_setting', project=self.project ), - {'Test': 'Less'}, + {'Test': 'Updated'}, ) diff --git a/projectroles/tests/test_views_taskflow.py b/projectroles/tests/test_views_taskflow.py index acce97d9..f1823191 100644 --- a/projectroles/tests/test_views_taskflow.py +++ b/projectroles/tests/test_views_taskflow.py @@ -76,7 +76,9 @@ def _make_project_taskflow(self, title, type, parent, owner, description): 'sodar_url': self.live_server_url, } # HACK: Override callback URL values.update( - app_settings.get_all_defaults(APP_SETTING_SCOPE_PROJECT) + app_settings.get_all_defaults( + APP_SETTING_SCOPE_PROJECT, post_safe=True + ) ) # Add default settings post_kwargs = {'project': parent.sodar_uuid} if parent else {} @@ -166,7 +168,9 @@ def setUp(self): 'description': 'description', } values.update( - app_settings.get_all_defaults(APP_SETTING_SCOPE_PROJECT) + app_settings.get_all_defaults( + APP_SETTING_SCOPE_PROJECT, post_safe=True + ) ) # Add default settings with self.login(self.user): @@ -260,7 +264,7 @@ def test_update_project(self): values['owner'] = self.user.sodar_uuid # NOTE: Must add owner values['readme'] = 'updated readme' values.update( - app_settings.get_all_settings(project=self.project) + app_settings.get_all_settings(project=self.project, post_safe=True) ) # Add default settings values['sodar_url'] = self.live_server_url # HACK @@ -317,7 +321,7 @@ def test_update_project_new_owner(self): values['owner'] = new_user.sodar_uuid values['readme'] = 'updated readme' values.update( - app_settings.get_all_settings(project=self.project) + app_settings.get_all_settings(project=self.project, post_safe=True) ) # Add default settings values['sodar_url'] = self.live_server_url # HACK diff --git a/projectroles/views.py b/projectroles/views.py index fe591db4..d1fabe00 100644 --- a/projectroles/views.py +++ b/projectroles/views.py @@ -631,7 +631,12 @@ def _get_project_update_data(old_data, project, owner, project_settings): ) if old_v != v: - extra_data[k] = v + if isinstance(v, dict): + extra_data[k] = v + + else: + extra_data[k] = '' + upd_fields.append(k) return extra_data, upd_fields @@ -795,7 +800,14 @@ def form_valid(self, form): for s_key, s_val in p_settings.items(): s_name = 'settings.{}.{}'.format(plugin.name, s_key) - project_settings[s_name] = form.cleaned_data.get(s_name) + + if s_val['type'] == 'JSON': + project_settings[s_name] = json.dumps( + form.cleaned_data.get(s_name) + ) + + else: + project_settings[s_name] = form.cleaned_data.get(s_name) if timeline: if form_action == 'create': @@ -2652,9 +2664,15 @@ def post(self, request): except Project.DoesNotExist as ex: return Response(str(ex), status=404) + project_settings = app_settings.get_all_settings(project) + + for k, v in project_settings.items(): + if isinstance(v, dict): + project_settings[k] = json.dumps(v) + ret_data = { 'project_uuid': project.sodar_uuid, - 'settings': app_settings.get_all_settings(project), + 'settings': project_settings, } return Response(ret_data, status=200) diff --git a/run.sh b/run.sh index 58801b6d..e592bd52 100755 --- a/run.sh +++ b/run.sh @@ -1,5 +1,2 @@ #!/usr/bin/env bash -if [ $# -gt 0 ] && [ $1 = "sync" ]; then - ./manage.py synctaskflow -fi ./manage.py runserver --settings=config.settings.local diff --git a/run_taskflow.sh b/run_taskflow.sh new file mode 100755 index 00000000..1190bbce --- /dev/null +++ b/run_taskflow.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +if [ $# -gt 0 ] && [ $1 = "sync" ]; then + ./manage.py synctaskflow --settings=config.settings.local_taskflow +fi +./manage.py runserver --settings=config.settings.local_taskflow diff --git a/userprofile/forms.py b/userprofile/forms.py index 3942dcd5..67a71b00 100644 --- a/userprofile/forms.py +++ b/userprofile/forms.py @@ -70,9 +70,19 @@ def __init__(self, *args, **kwargs): ) # Set initial value - self.initial[s_field] = app_settings.get_app_setting( - app_name=plugin.name, setting_name=s_key, user=self.user - ) + if s_val['type'] != 'JSON': + self.initial[s_field] = app_settings.get_app_setting( + app_name=plugin.name, setting_name=s_key, user=self.user + ) + + else: + self.initial[s_field] = json.dumps( + app_settings.get_app_setting( + app_name=plugin.name, + setting_name=s_key, + user=self.user, + ) + ) def clean(self): """Function for custom form validation and cleanup""" @@ -87,16 +97,18 @@ def clean(self): if s_val['type'] == 'JSON': try: self.cleaned_data[s_field] = json.loads( - self.cleaned_data[s_field] + self.cleaned_data.get(s_field) ) except json.JSONDecodeError as err: + # TODO: Shouldn't we use add_error() instead? raise forms.ValidationError( - 'Couldn\'t encode json.\n' + str(err) + 'Couldn\'t encode JSON\n' + str(err) ) - app_settings.validate_setting( + if not app_settings.validate_setting( setting_type=s_val['type'], setting_value=self.cleaned_data.get(s_field), - ) + ): + self.add_error(s_field, 'Invalid value') return self.cleaned_data From 873024b0153da4de017b7daf4d7840e8fea1fcac Mon Sep 17 00:00:00 2001 From: "mikko.nieminen" Date: Tue, 1 Oct 2019 12:30:49 +0200 Subject: [PATCH 06/20] improve project extra data logging (#358, #359), fix cosmetic timeline issues (#360, #361), add get_setting_def() to app settings api, refactor example app settings, cleanup --- CHANGELOG.rst | 2 + example_project_app/plugins.py | 2 +- projectroles/app_settings.py | 34 ++++++++++ projectroles/tests/test_app_settings_api.py | 66 ++++++++++++++++++- projectroles/tests/test_views.py | 25 +++---- projectroles/views.py | 30 ++++++--- .../templates/timeline/_extra_data_modal.html | 8 ++- timeline/templatetags/timeline_tags.py | 4 +- 8 files changed, 139 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5193305a..e36fe9a9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -37,6 +37,8 @@ Added - UI notification for disabled member management on target sites (#301) - Management command ``addremotesite`` for adding remote sites (#314) - JSON support for app settings (#268) + - ``get_setting_def()`` in app settings API + - Timeline logging of app settings in project creation (#359) - **Timeline** - Display event extra data as JSON (#6) - **Userprofile** diff --git a/example_project_app/plugins.py b/example_project_app/plugins.py index b0dec428..ae7df973 100644 --- a/example_project_app/plugins.py +++ b/example_project_app/plugins.py @@ -25,7 +25,7 @@ class ProjectAppPlugin(ProjectAppPluginPoint): #: Project and user settings app_settings = { - 'project_string_setting': { + 'project_str_setting': { 'scope': SODAR_CONSTANTS['APP_SETTING_SCOPE_PROJECT'], 'type': 'STRING', 'label': 'String Setting', diff --git a/projectroles/app_settings.py b/projectroles/app_settings.py index 6534119a..f26de49f 100644 --- a/projectroles/app_settings.py +++ b/projectroles/app_settings.py @@ -334,6 +334,40 @@ def validate_setting(cls, setting_type, setting_value): ) return True + @classmethod + def get_setting_def(cls, name, plugin=None, app_name=None): + """ + Return definition for a single app setting, either based on an app name + or the plugin object. + + :param name: Setting name + :param app_name: Name of the app plugin (string) + :param name: Plugin object extending ProjectAppPluginPoint + :return: Dict + :raise: ValueError if neither app_name or plugin are set or if setting + is not found in plugin + """ + if not plugin and not app_name: + raise ValueError('Plugin and app name both unset') + + elif not plugin: + plugin = get_app_plugin(app_name) + + if not plugin: + raise ValueError( + 'Plugin not found with app name "{}"'.format(app_name) + ) + + if name not in plugin.app_settings: + raise ValueError( + 'App setting not found in app "{}" with name "{}"'.format( + plugin.name, name + ) + ) + + return plugin.app_settings[name] + + # TODO: Refactor to also take app name instead of plugin @classmethod def get_setting_defs(cls, plugin, scope, user_modifiable=False): """ diff --git a/projectroles/tests/test_app_settings_api.py b/projectroles/tests/test_app_settings_api.py index ded57372..f2c7235b 100644 --- a/projectroles/tests/test_app_settings_api.py +++ b/projectroles/tests/test_app_settings_api.py @@ -54,7 +54,7 @@ def setUp(self): self.setting_str_values = { 'app_name': EXAMPLE_APP_NAME, 'project': self.project, - 'name': 'project_string_setting', + 'name': 'project_str_setting', 'setting_type': 'STRING', 'value': 'test', 'update_value': 'better test', @@ -273,11 +273,73 @@ def test_validate_project_setting_invalid(self): with self.assertRaises(ValueError): app_settings.validate_setting('INVALID_TYPE', 'value') + def test_get_setting_def_plugin(self): + """Test get_setting_def() with a plugin""" + app_plugin = get_app_plugin(EXAMPLE_APP_NAME) + expected = { + 'scope': APP_SETTING_SCOPE_PROJECT, + 'type': 'STRING', + 'label': 'String Setting', + 'default': '', + 'description': 'Example string project setting', + 'user_modifiable': True, + } + s_def = app_settings.get_setting_def( + 'project_str_setting', plugin=app_plugin + ) + self.assertEqual(s_def, expected) + + def test_get_setting_def_app_name(self): + """Test get_setting_def() with an app name""" + expected = { + 'scope': APP_SETTING_SCOPE_PROJECT, + 'type': 'STRING', + 'label': 'String Setting', + 'default': '', + 'description': 'Example string project setting', + 'user_modifiable': True, + } + s_def = app_settings.get_setting_def( + 'project_str_setting', app_name=EXAMPLE_APP_NAME + ) + self.assertEqual(s_def, expected) + + def test_get_setting_def_user(self): + """Test get_setting_def() with a user setting""" + expected = { + 'scope': APP_SETTING_SCOPE_USER, + 'type': 'STRING', + 'label': 'String Setting', + 'default': '', + 'description': 'Example string user setting', + 'user_modifiable': True, + } + s_def = app_settings.get_setting_def( + 'user_str_setting', app_name=EXAMPLE_APP_NAME + ) + self.assertEqual(s_def, expected) + + def test_get_setting_def_invalid(self): + """Test get_setting_def() with innvalid input""" + with self.assertRaises(ValueError): + app_settings.get_setting_def( + 'non_existing_setting', app_name=EXAMPLE_APP_NAME + ) + + with self.assertRaises(ValueError): + app_settings.get_setting_def( + 'project_str_setting', app_name='non_existing_app_name' + ) + + # Both app_name and plugin unset + with self.assertRaises(ValueError): + app_settings.get_setting_def('project_str_setting') + def test_get_setting_defs_project(self): """Test get_setting_defs() with the PROJECT scope""" app_plugin = get_app_plugin(EXAMPLE_APP_NAME) expected = { - 'project_string_setting': { + 'project_str_setting': { 'scope': APP_SETTING_SCOPE_PROJECT, 'type': 'STRING', 'label': 'String Setting', diff --git a/projectroles/tests/test_views.py b/projectroles/tests/test_views.py index 3eb6e90a..c3676ac4 100644 --- a/projectroles/tests/test_views.py +++ b/projectroles/tests/test_views.py @@ -27,12 +27,7 @@ SODAR_CONSTANTS, PROJECT_TAG_STARRED, ) -from ..plugins import ( - change_plugin_status, - get_backend_api, - get_active_plugins, - ProjectAppPluginPoint, -) +from ..plugins import change_plugin_status, get_backend_api, get_active_plugins from ..remote_projects import RemoteProjectAPI from ..utils import build_secret, get_display_name from .test_models import ( @@ -323,9 +318,7 @@ def test_render(self): self.assertEqual(response.context['object'].pk, self.project.pk) -class TestProjectCreateView( - ProjectMixin, RoleAssignmentMixin, TestViewsBase -): +class TestProjectCreateView(ProjectMixin, RoleAssignmentMixin, TestViewsBase): """Tests for Project creation view""" def test_render_top(self): @@ -564,9 +557,7 @@ def test_create_project(self): self.assertEqual(model_to_dict(owner_as), expected) -class TestProjectUpdateView( - ProjectMixin, RoleAssignmentMixin, TestViewsBase -): +class TestProjectUpdateView(ProjectMixin, RoleAssignmentMixin, TestViewsBase): """Tests for Project updating view""" def setUp(self): @@ -706,7 +697,7 @@ def setUp(self): # Init string setting self.setting_bool = self._make_setting( app_name=EXAMPLE_APP_NAME, - name='project_string_setting', + name='project_str_setting', setting_type='STRING', value='', project=self.project, @@ -757,7 +748,7 @@ def test_get(self): self.assertIsNotNone(response.context['form']) self.assertIsNotNone( response.context['form'].fields.get( - 'settings.%s.project_string_setting' % EXAMPLE_APP_NAME + 'settings.%s.project_str_setting' % EXAMPLE_APP_NAME ) ) self.assertIsNotNone( @@ -780,7 +771,7 @@ def test_post(self): """Test modifying the settings values""" self.assertEqual( app_settings.get_app_setting( - EXAMPLE_APP_NAME, 'project_string_setting', project=self.project + EXAMPLE_APP_NAME, 'project_str_setting', project=self.project ), '', ) @@ -804,7 +795,7 @@ def test_post(self): ) values = { - 'settings.%s.project_string_setting' % EXAMPLE_APP_NAME: 'updated', + 'settings.%s.project_str_setting' % EXAMPLE_APP_NAME: 'updated', 'settings.%s.project_int_setting' % EXAMPLE_APP_NAME: 170, 'settings.%s.project_bool_setting' % EXAMPLE_APP_NAME: True, 'settings.%s.project_json_setting' @@ -836,7 +827,7 @@ def test_post(self): # Assert settings state after update self.assertEqual( app_settings.get_app_setting( - EXAMPLE_APP_NAME, 'project_string_setting', project=self.project + EXAMPLE_APP_NAME, 'project_str_setting', project=self.project ), 'updated', ) diff --git a/projectroles/views.py b/projectroles/views.py index d1fabe00..815a891b 100644 --- a/projectroles/views.py +++ b/projectroles/views.py @@ -626,17 +626,16 @@ def _get_project_update_data(old_data, project, owner, project_settings): # Settings for k, v in project_settings.items(): - old_v = app_settings.get_app_setting( - k.split('.')[1], k.split('.')[2], project - ) - - if old_v != v: - if isinstance(v, dict): - extra_data[k] = v + a_name = k.split('.')[1] + s_name = k.split('.')[2] + s_def = app_settings.get_setting_def(s_name, app_name=a_name) + old_v = app_settings.get_app_setting(a_name, s_name, project) - else: - extra_data[k] = '' + if s_def['type'] == 'JSON': + v = json.loads(v) + if old_v != v: + extra_data[k] = v upd_fields.append(k) return extra_data, upd_fields @@ -821,6 +820,19 @@ def form_valid(self, form): 'readme': project.readme.raw, } + # Add settings to extra data + for k, v in project_settings.items(): + a_name = k.split('.')[1] + s_name = k.split('.')[2] + s_def = app_settings.get_setting_def( + s_name, app_name=a_name + ) + + if s_def['type'] == 'JSON': + v = json.loads(v) + + extra_data[k] = v + else: # Update tl_desc = 'update ' + type_str.lower() extra_data, upd_fields = self._get_project_update_data( diff --git a/timeline/templates/timeline/_extra_data_modal.html b/timeline/templates/timeline/_extra_data_modal.html index a27ef615..5d78b9a6 100644 --- a/timeline/templates/timeline/_extra_data_modal.html +++ b/timeline/templates/timeline/_extra_data_modal.html @@ -2,6 +2,10 @@ {% block css %} + +{{ block.super }} + + {% endblock css %} {% block projectroles_extend %} - {% has_perm 'projectroles.update_project_owner' request.user project as can_update_owner %} - {% has_perm 'projectroles.update_project_members' request.user project as can_update_members %} - {% has_perm 'projectroles.update_project_delegate' request.user project as can_update_delegate %} - {% has_perm 'projectroles.invite_users' request.user project as can_invite %} - -
-

- {% get_display_name project.type title=True %} Members -

- {% if project.is_remote %} -
- -
- {% else %} - {% if can_update_members or can_invite %} - {% include 'projectroles/_role_list_buttons.html' with project=project can_update_members=can_update_members can_invite=can_invite %} - {% else %} -
- -
- {% endif %} - {% endif %} -
+{% has_perm 'projectroles.update_project_owner' request.user project as can_update_owner %} +{% has_perm 'projectroles.update_project_members' request.user project as can_update_members %} +{% has_perm 'projectroles.update_project_delegate' request.user project as can_update_delegate %} +{% has_perm 'projectroles.invite_users' request.user project as can_invite %} -
-
-
- - - - - - - - - - - - - - - - - - - {% for delegate in delegates %} - - - - - - - - {% endfor %} - {% for member in members %} - - - - - - - - {% endfor %} - -
UserNameEmailRole
{{ owner.user.username }}{{ owner.user.name }}{{ owner.user.email }}project owner - {% if not project.is_remote and can_update_owner %} - {% include 'projectroles/_project_roles_owner_dropdown.html' with user=owner.user assignment_id=owner.sodar_uuid %} - {% endif %} -
{{ delegate.user.username }}{{ delegate.user.name }}{{ delegate.user.email }}project delegate - - {% if not project.is_remote and can_update_delegate %} - {% include 'projectroles/_project_roles_buttons.html' with user=delegate.user assignment_id=delegate.sodar_uuid %} - {% endif %} -
{{ member.user.username }}{{ member.user.name }}{{ member.user.email }}{{ member.role.name }} - {% if not project.is_remote and can_update_members %} - {% include 'projectroles/_project_roles_buttons.html' with user=member.user assignment_id=member.sodar_uuid %} - {% endif %} -
-
-
+
+

{% get_display_name project.type title=True %} Members

+ {% if can_update_members or can_invite %} + {% include 'projectroles/_role_list_buttons.html' with project=project can_update_members=can_update_members can_invite=can_invite %} + {% endif %} +
+ +
+ {% if project.is_remote %} + {% if can_update_members or can_invite %} +
+ This is a remote project. You can only update or invite members on the + source site of this project. +
+ {% endif %} + {% endif %} + +
+
+ + + + + + + + + + + + + + + + + + + {% for delegate in delegates %} + + + + + + + + {% endfor %} + {% for member in members %} + + + + + + + + {% endfor %} + +
UserNameEmailRole
{{ owner.user.username }}{{ owner.user.name }}{{ owner.user.email }}project owner + {% if not project.is_remote and can_update_owner %} + {% include 'projectroles/_project_roles_owner_dropdown.html' with user=owner.user assignment_id=owner.sodar_uuid %} + {% endif %} +
{{ delegate.user.username }}{{ delegate.user.name }}{{ delegate.user.email }}project delegate + {% if not project.is_remote and can_update_delegate %} + {% include 'projectroles/_project_roles_buttons.html' with user=delegate.user assignment_id=delegate.sodar_uuid %} + {% endif %} +
{{ member.user.username }}{{ member.user.name }}{{ member.user.email }}{{ member.role.name }} + {% if not project.is_remote and can_update_members %} + {% include 'projectroles/_project_roles_buttons.html' with user=member.user assignment_id=member.sodar_uuid %} + {% endif %} +
+
+
{% endblock projectroles_extend %} {% block javascript %} - {{ block.super }} + {{ block.super }} + + {# Tour content #} + + if ($('div.pr_role_buttons').length) { + tour.addStep('role_menu', { + title: 'Member Editing Menu', + text: 'Individual memberships can be updated or removed from these ' + + 'menues.', + attachTo: 'div.sodar-pr-btn-grp-role left', + advanceOn: '.docs-link click', + showCancelLink: true, + scrollTo: true + }); + } + {% endblock javascript %} diff --git a/projectroles/templatetags/projectroles_common_tags.py b/projectroles/templatetags/projectroles_common_tags.py index d61b02e1..95da7787 100644 --- a/projectroles/templatetags/projectroles_common_tags.py +++ b/projectroles/templatetags/projectroles_common_tags.py @@ -264,10 +264,14 @@ def get_remote_icon(project, request): project=project, site__mode=SITE_MODE_SOURCE ) return ( - '' - ''.format(remote_project.site.name) + ''.format( + 'text-danger' if project.is_revoked() else 'text-info', + 'REVOKED remote' if project.is_revoked() else 'Remote', + remote_project.site.name, + ) ) except RemoteProject.DoesNotExist: diff --git a/projectroles/templatetags/projectroles_tags.py b/projectroles/templatetags/projectroles_tags.py index 34307fdf..a2569119 100644 --- a/projectroles/templatetags/projectroles_tags.py +++ b/projectroles/templatetags/projectroles_tags.py @@ -24,6 +24,10 @@ else 7 ) +# SODAR Constants +REMOTE_LEVEL_NONE = SODAR_CONSTANTS['REMOTE_LEVEL_NONE'] +REMOTE_LEVEL_REVOKED = SODAR_CONSTANTS['REMOTE_LEVEL_REVOKED'] + # Local constants INDENT_PX = 25 @@ -381,7 +385,20 @@ def get_target_project_select(site, project): for level in ACTIVE_LEVEL_TYPES: selected = False - legend = SODAR_CONSTANTS['REMOTE_ACCESS_LEVELS'][level] + + if ( + level == REMOTE_LEVEL_NONE + and current_level + and current_level != REMOTE_LEVEL_NONE + ): + legend = SODAR_CONSTANTS['REMOTE_ACCESS_LEVELS'][ + REMOTE_LEVEL_REVOKED + ] + level_val = REMOTE_LEVEL_REVOKED + + else: + legend = SODAR_CONSTANTS['REMOTE_ACCESS_LEVELS'][level] + level_val = level if level == current_level or ( level == SODAR_CONSTANTS['REMOTE_LEVEL_NONE'] and not current_level @@ -389,7 +406,7 @@ def get_target_project_select(site, project): selected = True ret += '\n'.format( - level, 'selected' if selected else '', legend + level_val, 'selected' if selected else '', legend ) ret += '\n' diff --git a/projectroles/tests/test_models.py b/projectroles/tests/test_models.py index 47c8404c..c8366fdf 100644 --- a/projectroles/tests/test_models.py +++ b/projectroles/tests/test_models.py @@ -351,6 +351,10 @@ def test_is_remote(self): """Test Project.is_remote() without remote projects""" self.assertEqual(self.project_sub.is_remote(), False) + def test_is_revoked(self): + """Test Project.is_revoked() without remote projects""" + self.assertEqual(self.project_sub.is_revoked(), False) + def test_validate_parent(self): """Test parent ForeignKey validation: project can't be its own parent""" @@ -1256,6 +1260,16 @@ def test_get_source_site_target(self): self.site.save() self.assertEqual(self.project.get_source_site(), self.site) + @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) + def test_is_revoked_target(self): + """Test Project.is_revoked() as target""" + self.site.mode = SITE_MODE_SOURCE + self.site.save() + self.assertEqual(self.project.is_revoked(), False) + self.remote_project.level = SODAR_CONSTANTS['REMOTE_LEVEL_REVOKED'] + self.remote_project.save() + self.assertEqual(self.project.is_revoked(), True) + @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) @override_settings(PROJECTROLES_DELEGATE_LIMIT=1) def test_validate_remote_delegates(self): diff --git a/projectroles/tests/test_permissions.py b/projectroles/tests/test_permissions.py index 387c5342..7de2bcdd 100644 --- a/projectroles/tests/test_permissions.py +++ b/projectroles/tests/test_permissions.py @@ -789,6 +789,7 @@ def test_user_autocomplete_api(self): self.assertFalse(data['results']) +@override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) class TestTargetProjectViews( RemoteSiteMixin, RemoteProjectMixin, TestProjectPermissionBase ): @@ -820,7 +821,6 @@ def setUp(self): level=SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES'], ) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_update(self): """Test access to project updating as target""" url = reverse( @@ -837,7 +837,6 @@ def test_update(self): ] self.assert_redirect(url, bad_users) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_create_top_allowed(self): """Test access to top level project creation as target""" url = reverse('projectroles:create') @@ -853,7 +852,6 @@ def test_create_top_allowed(self): self.assert_render200_ok(url, good_users) self.assert_redirect(url, bad_users) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_create_sub(self): """Test access to subproject creation as target""" url = reverse( @@ -870,10 +868,7 @@ def test_create_sub(self): self.assert_render200_ok(url, good_users) self.assert_redirect(url, bad_users) - @override_settings( - PROJECTROLES_SITE_MODE=SITE_MODE_TARGET, - PROJECTROLES_TARGET_CREATE=False, - ) + @override_settings(PROJECTROLES_TARGET_CREATE=False) def test_create_sub_disallowed(self): """Test access to subproject creation with creation disallowed as target""" url = reverse( @@ -890,7 +885,6 @@ def test_create_sub_disallowed(self): ] self.assert_redirect(url, bad_users) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_role_create(self): """Test access to role creation as target""" url = reverse( @@ -908,7 +902,6 @@ def test_role_create(self): ] self.assert_redirect(url, bad_users) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_role_update(self): """Test access to role updating as target""" url = reverse( @@ -926,7 +919,6 @@ def test_role_update(self): ] self.assert_redirect(url, bad_users) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_role_delete(self): """Test access to role deletion as target""" url = reverse( @@ -944,7 +936,6 @@ def test_role_delete(self): ] self.assert_redirect(url, bad_users) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_role_update_delegate(self): """Test access to delegate role update as target""" url = reverse( @@ -962,7 +953,6 @@ def test_role_update_delegate(self): ] self.assert_redirect(url, bad_users) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_role_delete_delegate(self): """Test access to role deletion for delegate as target""" url = reverse( @@ -980,7 +970,6 @@ def test_role_delete_delegate(self): ] self.assert_redirect(url, bad_users) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_role_invite_create(self): """Test access to role invite creation as target""" url = reverse( @@ -998,7 +987,6 @@ def test_role_invite_create(self): ] self.assert_redirect(url, bad_users) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_role_invite_list(self): """Test access to role invite list as target""" url = reverse( @@ -1016,6 +1004,69 @@ def test_role_invite_list(self): self.assert_redirect(url, bad_users) +@override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) +class TestRevokedRemoteProject( + RemoteSiteMixin, RemoteProjectMixin, TestProjectPermissionBase +): + """Tests for views for a revoked project on a TARGET site""" + + def setUp(self): + super().setUp() + + # Create site + self.site = self._make_site( + name=REMOTE_SITE_NAME, + url=REMOTE_SITE_URL, + mode=SODAR_CONSTANTS['SITE_MODE_SOURCE'], + description='', + secret=REMOTE_SITE_SECRET, + ) + + # Create RemoteProject objects + self.remote_category = self._make_remote_project( + project_uuid=self.category.sodar_uuid, + project=self.category, + site=self.site, + level=SODAR_CONSTANTS['REMOTE_LEVEL_READ_INFO'], + ) + self.remote_project = self._make_remote_project( + project_uuid=self.project.sodar_uuid, + project=self.project, + site=self.site, + level=SODAR_CONSTANTS['REMOTE_LEVEL_REVOKED'], + ) + + def test_project_details(self): + """Test access to REVOKED project detail page as target""" + url = reverse( + 'projectroles:detail', kwargs={'project': self.project.sodar_uuid} + ) + good_users = [self.superuser, self.as_owner.user, self.as_delegate.user] + bad_users = [ + self.anonymous, + self.as_contributor.user, + self.as_guest.user, + self.user_no_roles, + ] + self.assert_response(url, good_users, 200) + self.assert_redirect(url, bad_users) + + def test_role_list(self): + """Test access to REVOKED project's role list as target""" + url = reverse( + 'projectroles:roles', kwargs={'project': self.project.sodar_uuid} + ) + good_users = [self.superuser, self.as_owner.user, self.as_delegate.user] + bad_users = [ + self.anonymous, + self.as_contributor.user, + self.as_guest.user, + self.user_no_roles, + ] + self.assert_response(url, good_users, 200) + self.assert_redirect(url, bad_users) + + class TestRemoteSiteApp(RemoteSiteMixin, TestPermissionBase): """Tests for remote site management views""" diff --git a/projectroles/tests/test_remote_projects_api.py b/projectroles/tests/test_remote_projects_api.py index 64781f0c..2e62015f 100644 --- a/projectroles/tests/test_remote_projects_api.py +++ b/projectroles/tests/test_remote_projects_api.py @@ -47,6 +47,7 @@ REMOTE_LEVEL_VIEW_AVAIL = SODAR_CONSTANTS['REMOTE_LEVEL_VIEW_AVAIL'] REMOTE_LEVEL_READ_INFO = SODAR_CONSTANTS['REMOTE_LEVEL_READ_INFO'] REMOTE_LEVEL_READ_ROLES = SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES'] +REMOTE_LEVEL_REVOKED = SODAR_CONSTANTS['REMOTE_LEVEL_REVOKED'] # Local constants SOURCE_SITE_NAME = 'Test source site' @@ -295,7 +296,6 @@ def test_read_roles(self): site=self.target_site, level=REMOTE_LEVEL_READ_ROLES, ) - self._make_remote_project( project_uuid=self.project.sodar_uuid, site=self.peer_site, @@ -358,6 +358,64 @@ def test_read_roles(self): self.assertEqual(sync_data, expected) + def test_revoked(self): + """Test get data with project level of REVOKED""" + user_source_new = self.make_user('new_source_user') + self._make_assignment(self.project, user_source_new, self.role_guest) + self._make_remote_project( + project_uuid=self.project.sodar_uuid, + site=self.target_site, + level=REMOTE_LEVEL_REVOKED, + ) + self._make_remote_project( + project_uuid=self.project.sodar_uuid, + site=self.peer_site, + level=REMOTE_LEVEL_REVOKED, + ) + + sync_data = self.remote_api.get_target_data(self.target_site) + + expected = { + 'users': { + str(self.user_source.sodar_uuid): { + 'username': self.user_source.username, + 'name': self.user_source.name, + 'first_name': self.user_source.first_name, + 'last_name': self.user_source.last_name, + 'email': self.user_source.email, + 'groups': [SOURCE_USER_GROUP], + } + }, + 'projects': { + str(self.category.sodar_uuid): { + 'title': self.category.title, + 'type': PROJECT_TYPE_CATEGORY, + 'level': REMOTE_LEVEL_READ_INFO, + 'parent_uuid': None, + 'description': self.category.description, + 'readme': self.category.readme.raw, + }, + str(self.project.sodar_uuid): { + 'title': self.project.title, + 'type': PROJECT_TYPE_PROJECT, + 'level': REMOTE_LEVEL_REVOKED, + 'description': self.project.description, + 'readme': self.project.readme.raw, + 'parent_uuid': str(self.category.sodar_uuid), + 'roles': { + str(self.project_owner_as.sodar_uuid): { + 'user': self.project_owner_as.user.username, + 'role': self.project_owner_as.role.name, + } # NOTE: Another user should not be synced + }, + 'remote_sites': [], + }, + }, + 'peer_sites': {}, + } + + self.assertEqual(sync_data, expected) + def test_no_access(self): """Test get data with no project access set in the source site""" sync_data = self.remote_api.get_target_data(self.target_site) @@ -367,6 +425,7 @@ def test_no_access(self): self.assertEqual(sync_data, expected) +@override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) class TestSyncSourceData( ProjectMixin, RoleAssignmentMixin, @@ -458,7 +517,6 @@ def setUp(self): }, } - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_create(self): """Test sync with non-existing project data and READ_ROLE access""" @@ -610,7 +668,6 @@ def test_create(self): self.assertEqual(remote_data, expected) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_create_multiple(self): """Test sync with non-existing project data and multiple projects""" @@ -699,7 +756,6 @@ def test_create_multiple(self): self.assertEqual(remote_data, expected) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_create_local_owner(self): """Test sync with non-existing project data and a local owner""" @@ -736,7 +792,6 @@ def test_create_local_owner(self): project_obj = Project.objects.get(sodar_uuid=SOURCE_PROJECT_UUID) self.assertEqual(project_obj.get_owner().user, self.admin_user) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_update(self): """Test sync with existing project data and READ_ROLE access""" @@ -989,7 +1044,123 @@ def test_update(self): self.assertEqual(remote_data, expected) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) + def test_update_revoke(self): + """Test sync with existing project data and REVOKED access""" + + # Set up target category and project + category_obj = self._make_project( + title='NewCategoryTitle', + type=PROJECT_TYPE_CATEGORY, + parent=None, + description='New description', + readme='New readme', + sodar_uuid=SOURCE_CATEGORY_UUID, + ) + project_obj = self._make_project( + title='NewProjectTitle', + type=PROJECT_TYPE_PROJECT, + parent=category_obj, + description='New description', + readme='New readme', + sodar_uuid=SOURCE_PROJECT_UUID, + ) + + # Set up users and roles + target_user = self._make_sodar_user( + username=SOURCE_USER_USERNAME, + name='NewFirstName NewLastName', + first_name='NewFirstName', + last_name='NewLastName', + email='newemail@example.com', + ) + new_user_username = 'newuser@' + SOURCE_USER_DOMAIN + target_user2 = self._make_sodar_user( + username=new_user_username, + name='Some OtherName', + first_name='Some', + last_name='OtherName', + email='othername@example.com', + ) + self._make_assignment(category_obj, target_user, self.role_owner) + self._make_assignment(project_obj, target_user, self.role_owner) + self._make_assignment(project_obj, target_user2, self.role_contributor) + + # Set up RemoteProject objects + self._make_remote_project( + project_uuid=category_obj.sodar_uuid, + project=category_obj, + site=self.source_site, + level=REMOTE_LEVEL_READ_ROLES, + ) + self._make_remote_project( + project_uuid=project_obj.sodar_uuid, + project=project_obj, + site=self.source_site, + level=REMOTE_LEVEL_READ_ROLES, + ) + + # Set up Peer Objects + peer_site = RemoteSite.objects.create( + **{ + 'name': PEER_SITE_NAME, + 'url': PEER_SITE_URL, + 'mode': SITE_MODE_PEER, + 'description': PEER_SITE_DESC, + 'secret': None, + 'sodar_uuid': PEER_SITE_UUID, + 'user_display': PEER_SITE_USER_DISPLAY, + } + ) + + self._make_remote_project( + project_uuid=project_obj.sodar_uuid, + project=project_obj, + site=peer_site, + level=SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES'], + ) + + # Assert preconditions + self.assertEqual(Project.objects.all().count(), 2) + self.assertEqual(RoleAssignment.objects.all().count(), 3) + self.assertEqual(User.objects.all().count(), 3) + self.assertEqual(RemoteProject.objects.all().count(), 3) + self.assertEqual(RemoteSite.objects.all().count(), 2) + + remote_data = self.default_data + + # Revoke access to project + remote_data['projects'][SOURCE_PROJECT_UUID][ + 'level' + ] = REMOTE_LEVEL_REVOKED + remote_data['projects'][SOURCE_PROJECT_UUID]['remote_sites'] = [] + + # Do sync + self.remote_api.sync_source_data(self.source_site, remote_data) + + # Assert database status + self.assertEqual(Project.objects.all().count(), 2) + self.assertEqual(RoleAssignment.objects.all().count(), 2) + self.assertEqual(User.objects.all().count(), 3) + self.assertEqual(RemoteProject.objects.all().count(), 2) + self.assertEqual(RemoteSite.objects.all().count(), 2) + + new_user = User.objects.get(username=new_user_username) + + # Assert removal of role assignment + with self.assertRaises(RoleAssignment.DoesNotExist): + RoleAssignment.objects.get( + project__sodar_uuid=SOURCE_PROJECT_UUID, + user=new_user, + role__name=PROJECT_ROLE_CONTRIBUTOR, + ) + + # Assert update_data changes + self.assertEqual( + remote_data['projects'][SOURCE_PROJECT_UUID]['level'], + REMOTE_LEVEL_REVOKED, + ) + self.assertNotIn(str(new_user.sodar_uuid), remote_data['users'].keys()) + def test_delete_role(self): """Test sync with existing project data and a removed role""" @@ -1098,7 +1269,6 @@ def test_delete_role(self): self.assertEqual(remote_data, expected) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_update_no_changes(self): """Test sync with existing project data and no changes""" @@ -1300,7 +1470,6 @@ def test_update_no_changes(self): # Assert no changes between update_data and remote_data self.assertEqual(original_data, remote_data) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_create_no_access(self): """Test sync with no READ_ROLE access set""" @@ -1328,7 +1497,6 @@ def test_create_no_access(self): # Assert no changes between update_data and remote_data self.assertEqual(original_data, remote_data) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_create_local_user(self): """Test sync with a local non-owner user""" @@ -1400,7 +1568,6 @@ def test_create_local_user_allow(self): self.assertEqual(RemoteProject.objects.all().count(), 3) self.assertEqual(RemoteSite.objects.all().count(), 2) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) @override_settings(PROJECTROLES_ALLOW_LOCAL_USERS=True) def test_create_local_user_allow_unavailable(self): """Test sync with a non-existent local user with local users allowed""" @@ -1435,7 +1602,6 @@ def test_create_local_user_allow_unavailable(self): self.assertEqual(RemoteProject.objects.all().count(), 3) self.assertEqual(RemoteSite.objects.all().count(), 2) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) @override_settings(PROJECTROLES_ALLOW_LOCAL_USERS=True) def test_create_local_owner_allow(self): """Test sync with a local owner with local users allowed""" @@ -1479,7 +1645,6 @@ def test_create_local_owner_allow(self): new_project = Project.objects.get(sodar_uuid=SOURCE_PROJECT_UUID) self.assertEqual(new_project.get_owner().user, new_user) - @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) @override_settings(PROJECTROLES_ALLOW_LOCAL_USERS=True) def test_create_local_owner_allow_unavailable(self): """Test sync with an unavailable local owner""" diff --git a/projectroles/tests/test_ui.py b/projectroles/tests/test_ui.py index 5ec71d03..f94a1420 100644 --- a/projectroles/tests/test_ui.py +++ b/projectroles/tests/test_ui.py @@ -738,10 +738,9 @@ def test_peer_project_source_invisible(self): @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_peer_project_target_visible(self): - """Checks visibility of peer projects on TARGET site (user_display=False)""" + """Test visibility of peer projects on TARGET site (user_display=False)""" # There needs to be a source mode remote project as master project # otherwise peer project logic wont be reached - # TODO: Clean up when implementing #196 (refactoring project_detail.html logic) source_site = self._make_site( name='Second Remote Site', url='second_remote.site', @@ -750,14 +749,12 @@ def test_peer_project_target_visible(self): secret=build_secret(), user_display=True, ) - self._make_remote_project( project_uuid=self.project.sodar_uuid, site=source_site, level=SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES'], project=self.project, ) - self.setup_remote_project(site_mode=SITE_MODE_PEER) url = reverse( @@ -773,26 +770,24 @@ def test_peer_project_target_visible(self): for user in users: self.login_and_redirect(user, url) - - projects_on_other_sites = self.selenium.find_element_by_id( - 'sodar-pr-details-card-remote' + remote_links = self.selenium.find_elements_by_class_name( + 'sodar-pr-link-remote' ) - - details = projects_on_other_sites.find_elements_by_css_selector('a') - self.assertEqual(len(details), 2) - self.assertEqual( - details[0].text, source_site.name + ' (Master Project)' + self.assertEqual(len(remote_links), 2) + self.assertIn( + 'sodar-pr-link-remote-master', + remote_links[0].get_attribute('class'), ) - self.assertEqual( - details[1].text, self.remote_site.name + ' (Peer Project)' + self.assertIn( + 'sodar-pr-link-remote-peer', + remote_links[1].get_attribute('class'), ) @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) def test_peer_project_target_invisible(self): - """Checks invisibility of peer projects on TARGET site for different users (user_display=False)""" + """Test invisibility of peer projects on TARGET site for users (user_display=False)""" # There needs to be a source mode remote project as master project # otherwise peer project logic wont be reached - # TODO: Clean up when implementing #196 (refactoring project_detail.html logic) source_site = self._make_site( name='Second Remote Site', url='second_remote.site', @@ -825,29 +820,28 @@ def test_peer_project_target_invisible(self): for user in expected_true: self.login_and_redirect(user, url) - - projects_on_other_sites = self.selenium.find_element_by_id( - 'sodar-pr-details-card-remote' + remote_links = self.selenium.find_elements_by_class_name( + 'sodar-pr-link-remote' ) - details = projects_on_other_sites.find_elements_by_css_selector('a') - self.assertEqual(len(details), 2) - self.assertEqual( - details[0].text, source_site.name + ' (Master Project)' + self.assertEqual(len(remote_links), 2) + self.assertIn( + 'sodar-pr-link-remote-master', + remote_links[0].get_attribute('class'), ) - self.assertEqual( - details[1].text, self.remote_site.name + ' (Peer Project)' + self.assertIn( + 'sodar-pr-link-remote-peer', + remote_links[1].get_attribute('class'), ) for user in expected_false: self.login_and_redirect(user, url) - - projects_on_other_sites = self.selenium.find_element_by_id( - 'sodar-pr-details-card-remote' + remote_links = self.selenium.find_elements_by_class_name( + 'sodar-pr-link-remote' ) - details = projects_on_other_sites.find_elements_by_css_selector('a') - self.assertEqual(len(details), 1) - self.assertEqual( - details[0].text, source_site.name + ' (Master Project)' + self.assertEqual(len(remote_links), 1) + self.assertIn( + 'sodar-pr-link-remote-master', + remote_links[0].get_attribute('class'), ) @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) @@ -915,24 +909,17 @@ class TestProjectRoles(RemoteTargetMixin, TestUIBase): def test_list_buttons(self): """Test visibility of role list button group according to user permissions""" - users_with_perm = [ - self.superuser, - self.as_owner.user, - self.as_delegate.user, - ] - - users_without_perm = [self.as_contributor.user, self.as_guest.user] - + good_users = [self.superuser, self.as_owner.user, self.as_delegate.user] + bad_users = [self.as_contributor.user, self.as_guest.user] url = reverse( 'projectroles:roles', kwargs={'project': self.project.sodar_uuid} ) self.assert_element_exists( - users_with_perm, url, 'sodar-pr-btn-perm', True + good_users, url, 'sodar-pr-btn-role-op', True ) - self.assert_element_exists( - users_without_perm, url, 'sodar-pr-btn-no-perm', True + bad_users, url, 'sodar-pr-btn-role-op', False ) @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET) @@ -942,8 +929,7 @@ def test_list_buttons_target(self): # Set up site as target self._set_up_as_target(projects=[self.category, self.project]) - users = [ - self.superuser, + non_superusers = [ self.as_owner.user, self.as_delegate.user, self.as_contributor.user, @@ -953,7 +939,13 @@ def test_list_buttons_target(self): 'projectroles:roles', kwargs={'project': self.project.sodar_uuid} ) - self.assert_element_exists(users, url, 'sodar-pr-btn-remote', True) + self.login_and_redirect(self.superuser, url) + btn = self.selenium.find_element_by_id('sodar-pr-btn-role-op') + self.assertEqual(btn.is_enabled(), False) + + self.assert_element_exists( + non_superusers, url, 'sodar-pr-btn-role-op', False + ) def test_role_list_invite_button(self): """Test visibility of role invite button according to user diff --git a/projectroles/views.py b/projectroles/views.py index 04de43d2..467ec621 100644 --- a/projectroles/views.py +++ b/projectroles/views.py @@ -242,9 +242,10 @@ def get_permission_object(self): return self.get_project() def has_permission(self): - """Disable project app access for categories""" + """Overrides for project permission access""" project = self.get_project() + # Disable project app access for categories if project and project.type == PROJECT_TYPE_CATEGORY: request_url = resolve(self.request.get_full_path()) @@ -254,10 +255,22 @@ def has_permission(self): ): return False + # Disable access for non-owner/delegate if remote project is revoked + if project and project.is_revoked(): + role_as = RoleAssignment.objects.filter( + project=project, user=self.request.user + ).first() + + if role_as and role_as.role.name not in [ + PROJECT_ROLE_OWNER, + PROJECT_ROLE_DELEGATE, + ]: + return False + return super().has_permission() def get_queryset(self, *args, **kwargs): - """Override ``get_query_set()`` to filter down to the currently selected + """Override ``get_queryset()`` to filter down to the currently selected object.""" qs = super().get_queryset(*args, **kwargs) @@ -448,7 +461,11 @@ def get_context_data(self, *args, **kwargs): class ProjectDetailView( - LoginRequiredMixin, LoggedInPermissionMixin, ProjectContextMixin, DetailView + LoginRequiredMixin, + LoggedInPermissionMixin, + ProjectPermissionMixin, + ProjectContextMixin, + DetailView, ): """Project details view""" From 825550cbb3f6029aa6e3c11f3949fbf6bbe93c0d Mon Sep 17 00:00:00 2001 From: "mikko.nieminen" Date: Tue, 8 Oct 2019 16:43:49 +0200 Subject: [PATCH 13/20] fix minor layout issues (#365, #367, #371) --- CHANGELOG.rst | 2 ++ .../static/projectroles/css/projectroles.css | 2 ++ .../static/projectroles/js/projectroles.js | 32 ++++++++++++------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6597afac..cdadd99c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -83,6 +83,8 @@ Fixed - Remote sync of parent category roles could fail with multiple subprojects - ``RemoteProject`` modifications not saved during sync update - Timeline events not created in remote project sync (#370) + - DAL select modifying HTML body width (#365) + - Footer overflow breaking layout (#367) - **Timeline** - Crash from exception raised by ``get_object_link()`` in a plugin (#328) diff --git a/projectroles/static/projectroles/css/projectroles.css b/projectroles/static/projectroles/css/projectroles.css index 5d2066a9..a7ff00a6 100644 --- a/projectroles/static/projectroles/css/projectroles.css +++ b/projectroles/static/projectroles/css/projectroles.css @@ -13,6 +13,7 @@ body { margin: 0; + overflow-x: hidden; /* Fix for DAL select bug (issue #365) */ } body, @@ -160,6 +161,7 @@ div.sodar-page-container { line-height: 36px; margin: 0; background-color: #f5f5f5; + overflow-y: hidden; } diff --git a/projectroles/static/projectroles/js/projectroles.js b/projectroles/static/projectroles/js/projectroles.js index deefe431..a2fe45bb 100644 --- a/projectroles/static/projectroles/js/projectroles.js +++ b/projectroles/static/projectroles/js/projectroles.js @@ -398,23 +398,31 @@ $(document).ready(function () { } }); -/* Hide Sidebar based on its element count */ + +/* Hide sidebar based on element count -------------------------------------- */ + + +function toggleSidebar() { + if (!window.sidebar.is(':visible')) { + if (window.sidebarMinWindowHeight < window.innerHeight && window.innerWidth > 1000) { + window.sidebar.show(); + window.sidebar_alt_btn.hide(); + } + } else if (window.sidebarMinWindowHeight > window.innerHeight || window.innerWidth < 1000) { + window.sidebar_alt_btn.show(); + window.sidebar.hide(); + } +} + $(document).ready(function () { - // remember sidebar total height + // Remember sidebar total height window.sidebar = $('#sodar-pr-sidebar'); window.sidebar_alt_btn = $('#sodar-pr-sidebar-alt-btn'); let sidebarContent = $('#sodar-pr-sidebar-navbar').get(0); window.sidebarMinWindowHeight = sidebarContent.scrollHeight + sidebarContent.getBoundingClientRect().top; + toggleSidebar(); }); $(window).on('resize', function () { - if (!window.sidebar.is(':visible')) { - if (window.sidebarMinWindowHeight < window.innerHeight && window.innerWidth > 1000) { - window.sidebar.collapse('show'); - window.sidebar_alt_btn.collapse('hide'); - } - } else if (window.sidebarMinWindowHeight > window.innerHeight || window.innerWidth < 1000) { - window.sidebar_alt_btn.collapse('show'); - window.sidebar.collapse('hide'); - } -}); \ No newline at end of file + toggleSidebar(); +}); From 905a14eba0a097cfd5ecc6717e4c1f1cf9a52fab Mon Sep 17 00:00:00 2001 From: "mikko.nieminen" Date: Tue, 8 Oct 2019 18:28:28 +0200 Subject: [PATCH 14/20] cleanup for v0.7.0 release --- AUTHORS.rst | 7 +- CHANGELOG.rst | 2 + .../templates/adminalerts/alert_list.html | 342 ++++++++---------- adminalerts/tests/test_models.py | 1 - adminalerts/tests/test_permissions.py | 6 + adminalerts/tests/test_views.py | 10 +- adminalerts/urls.py | 2 +- adminalerts/views.py | 42 ++- docs/source/app_projectroles_usage.rst | 14 +- docs/source/dev_general.rst | 13 + .../example_backend_app/css/greeting.css | 2 +- filesfolders/tests/test_models.py | 4 +- .../project_roles_transfer_owner.html | 62 ++-- projectroles/tests/test_templatetags.py | 2 +- projectroles/tests/test_ui.py | 28 +- .../templates/timeline/_extra_data_modal.html | 3 +- userprofile/tests/test_views.py | 6 +- 17 files changed, 278 insertions(+), 268 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 66cfd0c3..e3ee8221 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -4,6 +4,7 @@ Credits * Mikko Nieminen * Manuel Holtgrewe * Franziska Schumann -* Raunak Agarwal -* Hendrik Bomhardt -* Tim Garrels +* Oliver Stolpe +* Hendrik Bomhardt +* Tim Garrels +* Raunak Agarwal diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cdadd99c..2878d482 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -58,6 +58,8 @@ Changed - Use ``CurrentUserFormMixin`` instead of repeated code (#12) - Run tests in parallel where applicable - Upgrade minimum Django version to 1.11.25 (#346) +- **Adminalerts** + - Use common pagination template - **Projectroles** - Improve user name placeholder in ``login.html`` (#294) - Backend app Javascript and CSS included on-demand instead of for all templates (#261) diff --git a/adminalerts/templates/adminalerts/alert_list.html b/adminalerts/templates/adminalerts/alert_list.html index 633b9c2b..4ab84a1c 100644 --- a/adminalerts/templates/adminalerts/alert_list.html +++ b/adminalerts/templates/adminalerts/alert_list.html @@ -7,200 +7,176 @@ {% block title %}Admin Alerts{% endblock %} {% block css %} - {{ block.super }} - + {{ block.super }} + {% endblock css %} {% block javascript %} - {{ block.super }} - -{% endblock %} + }); + +{% endblock javascript %} {% block projectroles %} -
-

Admin Alerts

- - - Create Alert - -
- -
- - {% if object_list.count > 0 %} -
-
- - - - - - - - - - - - - {% for a in object_list %} - - - - - - - - - {% endfor %} - -
MessageUserCreatedExpiryStatus
{{ a.message }}{% get_user_html a.user as user_html %}{{ user_html|safe }}{{ a.date_created | date:'Y-m-d H:i:s' }}{{ a.date_expire | date:'Y-m-d' }} - - - -
-
-
- - {% if is_paginated %} -
-
- {% if page_obj.has_previous %} - - Newer - - {% else %} - - Newer - - {% endif %} - {% if page_obj.has_next %} - - Older - - {% else %} - - Older - - {% endif %} +
+

Admin Alerts

+ + + Create Alert + +
+ +
+ + {% if object_list.count > 0 %} +
+
+ + + + + + + + + + + + + {% for a in object_list %} + + + + + + + + + {% endfor %} + +
MessageUserCreatedExpiryStatus
{{ a.message }}{% get_user_html a.user as user_html %}{{ user_html|safe }}{{ a.date_created | date:'Y-m-d H:i:s' }}{{ a.date_expire | date:'Y-m-d' }} + + + - {% endif %} + +
+
+
- {% else %} -
- -
- {% endif %} + {% if is_paginated %} + {% include 'projectroles/_pagination.html' with pg_small=False %} + {% endif %} + {% else %} {# if object_list.count == 0 #} +
+
+ {% endif %} + +
{% endblock projectroles %} diff --git a/adminalerts/tests/test_models.py b/adminalerts/tests/test_models.py index fae3db2b..c8c657ac 100644 --- a/adminalerts/tests/test_models.py +++ b/adminalerts/tests/test_models.py @@ -40,7 +40,6 @@ class TestAdminAlert(AdminAlertMixin, TestCase): """Tests for AdminAlert model""" def setUp(self): - # Create superuser self.superuser = self.make_user('superuser') self.superuser.is_superuser = True diff --git a/adminalerts/tests/test_permissions.py b/adminalerts/tests/test_permissions.py index a91eb6ab..08bceeef 100644 --- a/adminalerts/tests/test_permissions.py +++ b/adminalerts/tests/test_permissions.py @@ -32,6 +32,7 @@ def setUp(self): ) def test_alert_create(self): + """Test permissions for AdminAlert creation""" url = reverse('adminalerts:create') good_users = [self.superuser] bad_users = [self.anonymous, self.regular_user] @@ -39,6 +40,7 @@ def test_alert_create(self): self.assert_redirect(url, bad_users) def test_alert_update(self): + """Test permissions for AdminAlert updating""" url = reverse( 'adminalerts:update', kwargs={'uuid': self.alert.sodar_uuid} ) @@ -48,6 +50,7 @@ def test_alert_update(self): self.assert_redirect(url, bad_users) def test_alert_delete(self): + """Test permissions for AdminAlert deletion""" url = reverse( 'adminalerts:delete', kwargs={'uuid': self.alert.sodar_uuid} ) @@ -57,6 +60,7 @@ def test_alert_delete(self): self.assert_redirect(url, bad_users) def test_alert_list(self): + """Test permissions for AdminAlert list""" url = reverse('adminalerts:list') good_users = [self.superuser] bad_users = [self.anonymous, self.regular_user] @@ -64,6 +68,7 @@ def test_alert_list(self): self.assert_redirect(url, bad_users) def test_alert_detail(self): + """Test permissions for AdminAlert details""" url = reverse( 'adminalerts:detail', kwargs={'uuid': self.alert.sodar_uuid} ) @@ -73,6 +78,7 @@ def test_alert_detail(self): self.assert_redirect(url, bad_users) def test_alert_activation(self): + """Test permissions for AdminAlert activation API view""" url = reverse('adminalerts:ajax_alert_activation') good_users = [self.superuser] bad_users = [self.anonymous, self.regular_user] diff --git a/adminalerts/tests/test_views.py b/adminalerts/tests/test_views.py index 287ac02d..2bd06e9c 100644 --- a/adminalerts/tests/test_views.py +++ b/adminalerts/tests/test_views.py @@ -232,8 +232,10 @@ def test_delete(self): class TestAdminAlertActivationView(TestViewsBase): + """Tests for the AdminAlert activation API view""" + def test_deactivate_alert(self): - """There is 1 active alert currently. Try to deactivate it.""" + """Test alert deactivation""" with self.login(self.superuser): self.assertTrue(self.alert.active) @@ -246,10 +248,10 @@ def test_deactivate_alert(self): data = json.loads(response.content) self.alert.refresh_from_db() self.assertFalse(self.alert.active) - self.assertFalse(data["is_active"]) + self.assertFalse(data['is_active']) def test_activate_alert(self): - """There is 1 active alert currently. Try to deactivate it.""" + """Test alert activation""" with self.login(self.superuser): self.alert.active = False self.alert.save() @@ -263,4 +265,4 @@ def test_activate_alert(self): data = json.loads(response.content) self.alert.refresh_from_db() self.assertTrue(self.alert.active) - self.assertTrue(data["is_active"]) + self.assertTrue(data['is_active']) diff --git a/adminalerts/urls.py b/adminalerts/urls.py index 5d6e4bdc..5bcf0c65 100644 --- a/adminalerts/urls.py +++ b/adminalerts/urls.py @@ -28,7 +28,7 @@ ), url( regex=r'^ajax/update-state', - view=views.AdminAlertActivationView.as_view(), + view=views.AdminAlertActivationAPIView.as_view(), name='ajax_alert_activation', ), ] diff --git a/adminalerts/views.py b/adminalerts/views.py index ed9d5b3e..6e8152b7 100644 --- a/adminalerts/views.py +++ b/adminalerts/views.py @@ -105,7 +105,29 @@ class AdminAlertUpdateView( slug_field = 'sodar_uuid' -class AdminAlertActivationView(LoggedInPermissionMixin, HTTPRefererMixin, View): +class AdminAlertDeleteView( + LoggedInPermissionMixin, HTTPRefererMixin, DeleteView +): + """AdminAlert deletion view""" + + model = AdminAlert + permission_required = 'adminalerts.update_alert' + slug_url_kwarg = 'uuid' + slug_field = 'sodar_uuid' + + def get_success_url(self): + """Override for redirecting alert list view with message""" + messages.success(self.request, 'Alert deleted.') + return reverse('adminalerts:list') + + +# API views -------------------------------------------------------------------- + + +class AdminAlertActivationAPIView( + LoggedInPermissionMixin, HTTPRefererMixin, View +): + """AdminAlert activation/deactivation API view""" permission_required = 'adminalerts.update_alert' http_method_names = ['post'] @@ -113,28 +135,12 @@ class AdminAlertActivationView(LoggedInPermissionMixin, HTTPRefererMixin, View): def post(self, request: HttpRequest): uuid = request.POST.get('uuid', None) - alert = None try: alert = AdminAlert.objects.get(sodar_uuid__exact=uuid) + except AdminAlert.DoesNotExist: return HttpResponseBadRequest() alert.active = not alert.active alert.save() return JsonResponse({'is_active': alert.active}) - - -class AdminAlertDeleteView( - LoggedInPermissionMixin, HTTPRefererMixin, DeleteView -): - """AdminAlert deletion view""" - - model = AdminAlert - permission_required = 'adminalerts.update_alert' - slug_url_kwarg = 'uuid' - slug_field = 'sodar_uuid' - - def get_success_url(self): - """Override for redirecting alert list view with message""" - messages.success(self.request, 'Alert deleted.') - return reverse('adminalerts:list') diff --git a/docs/source/app_projectroles_usage.rst b/docs/source/app_projectroles_usage.rst index 3051699b..cf325db3 100644 --- a/docs/source/app_projectroles_usage.rst +++ b/docs/source/app_projectroles_usage.rst @@ -155,11 +155,17 @@ before will be presented to the user. App Settings ------------ -Project and site apps may define project or user specific :term:`app settings`. +Project and site apps may define :term:`app settings`, which can be either be +set with the scope of *project*, *user* or *user within a project*. + Widgets for project specific settings will show up in the project creation and -updating form. User specific settings will be displayed in the :ref:`Userpforile -app `. Project settings can only be modified by users with -sufficient project access. +updating form and can only be modified by users with sufficient project access. +User specific settings will be displayed in the +:ref:`Userpforile app `. + +Settings with the scope of user within a project do not currently have a +separate UI of their own. Instead, project apps can produce their own user +specific UIs for this functionality if manual user selection is needed. .. note:: diff --git a/docs/source/dev_general.rst b/docs/source/dev_general.rst index 2cf889cc..8555fff7 100644 --- a/docs/source/dev_general.rst +++ b/docs/source/dev_general.rst @@ -27,6 +27,19 @@ Common Helpers Via the projectroles app, SODAR Core provides optional templates for aiding in maintaining common functionality and layout. Those are defined here. +App Setting API +--------------- + +For accessing and modifying app settings for project or site apps, you should +use the ``AppSettingAPI``. Below is an example of invoking the API. For the full +API docs, see :ref:`app_projectroles_api`. + +.. code-block:: python + + from projectroles.app_settings import AppSettingAPI + app_settings = AppSettingAPI() + app_settings.get_app_setting('app_name', 'setting_name', project_object) # Etc.. + Pagination Template ------------------- diff --git a/example_backend_app/static/example_backend_app/css/greeting.css b/example_backend_app/static/example_backend_app/css/greeting.css index 24929937..bbc93e22 100644 --- a/example_backend_app/static/example_backend_app/css/greeting.css +++ b/example_backend_app/static/example_backend_app/css/greeting.css @@ -1,3 +1,3 @@ #example_backend_greeting { - border: 1px solid grey; + font-weight: bold; } \ No newline at end of file diff --git a/filesfolders/tests/test_models.py b/filesfolders/tests/test_models.py index 0f609a21..7c369122 100644 --- a/filesfolders/tests/test_models.py +++ b/filesfolders/tests/test_models.py @@ -329,7 +329,7 @@ def test__repr__(self): self.assertEqual(repr(self.file), expected) def test_file_access(self): - """Ensure file can be accessed in database after creation""" + """Test file can be accessed in database after creation""" file_data = FileData.objects.get(file_name=self.file.file.name) expected = { @@ -343,7 +343,7 @@ def test_file_access(self): self.assertEqual(model_to_dict(file_data), expected) def test_file_deletion(self): - """Ensure file is removed from database after deletion""" + """Test file is removed from database after deletion""" # Assert precondition self.assertEqual(FileData.objects.all().count(), 1) diff --git a/projectroles/templates/projectroles/project_roles_transfer_owner.html b/projectroles/templates/projectroles/project_roles_transfer_owner.html index c6bbe02c..f16b704b 100644 --- a/projectroles/templates/projectroles/project_roles_transfer_owner.html +++ b/projectroles/templates/projectroles/project_roles_transfer_owner.html @@ -5,50 +5,48 @@ {% load static %} {% block title %} - Transfer ownership of {{ project.title }} + Transfer ownership of {{ project.title }} {% endblock title %} {% block projectroles_extend %} -
-

Transfer Project Ownership from User {{ current_owner.username }}

+
+

Transfer Project Ownership from User {{ current_owner.username }}

+
+ +
+
+ {% csrf_token %} + {{ form | crispy }} + +
+
+ + Cancel + + +
+
+
-
-
- {% csrf_token %} - {{ form | crispy }} - - -
-
- - Cancel - - -
-
-
-
- - + {% endblock projectroles_extend %} {% block javascript %} - {{ block.super }} + {{ block.super }} - - - {{ form.media }} + + + {{ form.media }} {% endblock javascript %} {% block css %} - {{ block.super }} - - + {{ block.super }} + + {% endblock css %} diff --git a/projectroles/tests/test_templatetags.py b/projectroles/tests/test_templatetags.py index 7e37999a..2daba567 100644 --- a/projectroles/tests/test_templatetags.py +++ b/projectroles/tests/test_templatetags.py @@ -258,7 +258,7 @@ def test_highlight_search_term(self): ) def test_get_info_link(self): - """Tet get_info_link()""" + """Test get_info_link()""" self.assertEqual( c_tags.get_info_link('content'), ' {% endfor %} -
diff --git a/userprofile/tests/test_views.py b/userprofile/tests/test_views.py index 7d5f3c58..10b1e176 100644 --- a/userprofile/tests/test_views.py +++ b/userprofile/tests/test_views.py @@ -86,7 +86,8 @@ def setUp(self): user=self.user, ) - def testGet(self): + def test_get(self): + """Test GET request on settings update form""" with self.login(self.user): response = self.client.get(reverse('userprofile:settings_update')) self.assertEqual(response.status_code, 200) @@ -112,7 +113,8 @@ def testGet(self): ) ) - def testPost(self): + def test_post(self): + """Test POST request on settings update form""" self.assertEqual( app_settings.get_app_setting( EXAMPLE_APP_NAME, 'user_str_setting', user=self.user From b2dc7aaad6f31251a4900d52ad8a01f5a4b121a8 Mon Sep 17 00:00:00 2001 From: "mikko.nieminen" Date: Tue, 8 Oct 2019 18:31:14 +0200 Subject: [PATCH 15/20] remove debug prints from remote_projects.py --- projectroles/remote_projects.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/projectroles/remote_projects.py b/projectroles/remote_projects.py index fd27969e..fdccba11 100644 --- a/projectroles/remote_projects.py +++ b/projectroles/remote_projects.py @@ -696,11 +696,6 @@ def _remove_revoked_peers(self, uuid, p_data): project_uuid=uuid, site__mode=SITE_MODE_PEER ) - # DEBUG - if p_data['level'] == REMOTE_LEVEL_REVOKED: - print('DEBUG: local_peers={}'.format(local_peers)) - print('DEBUG: remote_sites={}'.format(p_data['remote_sites'])) - if not local_peers: return From 9e453d4da289210811e0cc8c41c34979a1a6be3f Mon Sep 17 00:00:00 2001 From: "mikko.nieminen" Date: Wed, 9 Oct 2019 11:51:19 +0200 Subject: [PATCH 16/20] hide timeline event extra data in details view, refactor css styling, rename test cases --- .../templates/timeline/_extra_data_modal.html | 14 +++++++++++++- timeline/templates/timeline/_list_item.html | 6 +++--- timeline/tests/test_ui.py | 18 +++++++++--------- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/timeline/templates/timeline/_extra_data_modal.html b/timeline/templates/timeline/_extra_data_modal.html index f6203972..57d9513d 100644 --- a/timeline/templates/timeline/_extra_data_modal.html +++ b/timeline/templates/timeline/_extra_data_modal.html @@ -22,6 +22,18 @@ color: #222222; } + .sodar-tl-link-extra { + cursor: pointer; + } + + .sodar-tl-modal-tabs { + flex: 1; + flex-direction: row; + display: flex; + flex-wrap: wrap; + border-bottom: 1px solid #dee2e6; + } + .sodar-tl-copy-btn { position: absolute; top: 10px; @@ -34,7 +46,7 @@