diff --git a/.eslintrc.js b/.eslintrc.js index 1419f87dfe..5d67f9ab04 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -44,5 +44,5 @@ module.exports = { 'react': { 'version': 'detect' } - } + }, } diff --git a/.github/dependabot.yml b/.github/dependabot.yml index ee73ab2200..d5e4d5c071 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,32 +1,77 @@ version: 2 updates: - - package-ecosystem: pip - directory: / - schedule: - interval: monthly - open-pull-requests-limit: 10 - target-branch: dependency-updates - labels: - - dependencies - - python - - type:maintenance - - package-ecosystem: github-actions - directory: / - schedule: - interval: monthly - open-pull-requests-limit: 10 - target-branch: dependency-updates - labels: - - dependencies - - github_actions - - type:maintenance - - package-ecosystem: npm - directory: / - schedule: - interval: monthly - open-pull-requests-limit: 10 - target-branch: dependency-updates - labels: - - dependencies - - javascript - - type:maintenance +- package-ecosystem: pip + directory: / + schedule: + interval: monthly + open-pull-requests-limit: 10 + target-branch: dependency-updates + labels: + - dependencies + - python + - type:maintenance + ignore: + - dependency-name: django-mptt # pinned, 0.15 requires Python >= 3.9 + groups: + # create a single pull request containing all updates for the optional dependencies + optional: + patterns: + - coveralls + - django-allauth + - django-auth-ldap + - gunicorn + - mysqlclient + - pre-commit + - psycopg* + - pytest* + # create a single pull request containing all updates for django related dependencies + django: + patterns: + - django* + - drf* +- package-ecosystem: github-actions + directory: / + schedule: + interval: monthly + open-pull-requests-limit: 10 + target-branch: dependency-updates + labels: + - dependencies + - github_actions + - type:maintenance + groups: + # create a single pull request containing all updates for GitHub Actions + github-actions: + patterns: + - '*' +- package-ecosystem: npm + directory: / + schedule: + interval: monthly + open-pull-requests-limit: 10 + target-branch: dependency-updates + labels: + - dependencies + - javascript + - type:maintenance + groups: + react: + patterns: + - react* + redux: + patterns: + - redux* + babel: + patterns: + - '@babel*' + - babel* + webpack: + patterns: + - webpack* + eslint: + patterns: + - eslint* + prod-dependencies: + dependency-type: production + dev-dependencies: + dependency-type: development diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 369086b358..fe1418d7b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,6 +92,30 @@ jobs: GITHUB_DB_BACKEND: ${{ matrix.db-backend }} COVERALLS_FLAG_NAME: '${{ matrix.db-backend }}: ${{ matrix.python-version }}' COVERALLS_PARALLEL: true + # end-to-end tests + - uses: actions/setup-node@v4 + with: + node-version: 18 + cache: npm + if: matrix.python-version == '3.12' && matrix.db-backend == 'postgres' + - name: Cache Playwright browsers + uses: actions/cache@v3 + with: + path: ~/.cache/ms-playwright/ + key: playwright-browsers + if: matrix.python-version == '3.12' && matrix.db-backend == 'postgres' + - name: Install e2e tests dependencies + run: | + npm install + npm run build:prod + playwright install chromium + if: matrix.python-version == '3.12' && matrix.db-backend == 'postgres' + - name: Run end-to-end tests + run: pytest -p randomly -p no:cacheprovider --reuse-db --numprocesses=auto --dist=loadscope -m e2e --nomigrations + if: matrix.python-version == '3.12' && matrix.db-backend == 'postgres' + env: + DJANGO_DEBUG: True + GITHUB_DB_BACKEND: ${{ matrix.db-backend }} coveralls: name: Indicate completion to coveralls @@ -140,20 +164,6 @@ jobs: - run: python -m pip freeze - run: python -m pip list --outdated - webpack-build: - name: Test webpack-build - needs: lint - runs-on: ubuntu-22.04 - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v3 - with: - node-version: 18 - cache: npm - - run: npm install --dev - - run: npm run build - - run: npm run build:prod - required-checks-pass: if: always() needs: @@ -162,7 +172,6 @@ jobs: - coveralls - dev-setup - optional-dependencies - - webpack-build runs-on: ubuntu-22.04 steps: - uses: re-actors/alls-green@release/v1 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f766a30ec6..f1677d86c8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,7 +9,7 @@ repos: hooks: - id: check-hooks-apply - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.4.0 + rev: v4.5.0 hooks: - id: check-ast - id: check-json @@ -21,13 +21,13 @@ repos: - id: trailing-whitespace exclude: \.dot$ - id: debug-statements - - repo: https://github.com/charliermarsh/ruff-pre-commit - rev: v0.0.291 + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.1.6 hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] - repo: https://github.com/pre-commit/mirrors-eslint - rev: v8.50.0 + rev: v8.54.0 hooks: - id: eslint args: [--fix, --color] diff --git a/conftest.py b/conftest.py index b62eb83355..7a2d7ec8d9 100644 --- a/conftest.py +++ b/conftest.py @@ -10,29 +10,33 @@ from rdmo.accounts.utils import set_group_permissions +@pytest.fixture(scope="session") +def fixtures(): + allowed_file_stems = { + 'accounts', + 'conditions', + 'domain', + 'groups', + 'options', + 'overlays', + 'projects', + 'questions', + 'sites', + 'tasks', + 'users', + 'views' + } + fixtures = [] + for fixture_dir in settings.FIXTURE_DIRS: + filenames = [filename for filename in Path(fixture_dir).iterdir() if filename.stem in allowed_file_stems] + fixtures.extend(filenames) + return fixtures + + @pytest.fixture(scope='session') -def django_db_setup(django_db_setup, django_db_blocker): +def django_db_setup(django_db_setup, django_db_blocker, fixtures): """Populate database with test data from fixtures directories.""" with django_db_blocker.unblock(): - fixtures = [] - for fixture_dir in settings.FIXTURE_DIRS: - for file in Path(fixture_dir).iterdir(): - if file.stem in [ - 'accounts', - 'conditions', - 'domain', - 'groups', - 'options', - 'overlays', - 'projects', - 'questions', - 'sites', - 'tasks', - 'users', - 'views' - ]: - fixtures.append(file) - call_command('loaddata', *fixtures) set_group_permissions() @@ -49,7 +53,4 @@ def files(settings, tmp_path): @pytest.fixture def json_data(): json_file = Path(settings.BASE_DIR) / 'import' / 'catalogs.json' - json_data = { - 'elements': json.loads(json_file.read_text()) - } - return json_data + return {'elements': json.loads(json_file.read_text())} diff --git a/pyproject.toml b/pyproject.toml index 86cf1dbcfa..7f396bc22f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,9 @@ dynamic = [ "version", ] dependencies = [ + # dependencies with major version on zero are declared with + # major.minor.patch, because they can potentially introduce breaking changes + # in minor version updates anytime "defusedcsv~=2.0", "defusedxml~=0.7.1", "django~=4.2", @@ -46,7 +49,7 @@ dependencies = [ "django-filter~=23.2", "django-libsass~=0.9", "django-mathfilters~=1.0", - "django-mptt~=0.14.0", + "django-mptt==0.14.0", # pinned, 0.15 requires Python >= 3.9 "django-rest-swagger~=2.2", "django-settings-export~=1.2", "django-split-settings~=1.2", @@ -89,6 +92,7 @@ pytest = [ "pytest-cov~=4.1", "pytest-django~=4.5", "pytest-mock~=3.11", + "pytest-playwright~=0.4.3", "pytest-randomly~=3.15", "pytest-xdist~=3.3", ] @@ -177,7 +181,10 @@ DJANGO_SETTINGS_MODULE = "config.settings" testpaths = ["rdmo"] python_files = "test_*[!.txt].py" pythonpath = [".", "testing"] -addopts = "-p no:randomly" +addopts = '-p no:randomly -m "not e2e"' +markers = [ + "e2e: marks tests as end-to-end tests using playwright (deselect with '-m \"not e2e\"')", +] filterwarnings = [ # fail on RemovedInDjango50Warning exception "error::django.utils.deprecation.RemovedInDjango50Warning", diff --git a/rdmo/accounts/admin.py b/rdmo/accounts/admin.py index ac5a14342a..9c4fea9afe 100644 --- a/rdmo/accounts/admin.py +++ b/rdmo/accounts/admin.py @@ -5,14 +5,17 @@ from .models import AdditionalField, AdditionalFieldValue, ConsentFieldValue, Role +@admin.register(AdditionalField) class AdditionalFieldAdmin(admin.ModelAdmin): pass +@admin.register(AdditionalFieldValue) class AdditionalFieldValueAdmin(admin.ModelAdmin): readonly_fields = ('user', ) +@admin.register(ConsentFieldValue) class ConsentFieldValueAdmin(admin.ModelAdmin): readonly_fields = ('user', 'consent') @@ -20,6 +23,7 @@ def has_add_permission(self, request, obj=None): return False +@admin.register(Role) class RoleAdmin(admin.ModelAdmin): search_fields = ('user__username', 'user__email') list_filter = ('member', 'manager', 'editor', 'reviewer') @@ -50,9 +54,3 @@ def editors(self, obj): def reviewers(self, obj): return self.render_all_sites_or_join(obj, 'reviewer') - - -admin.site.register(AdditionalField, AdditionalFieldAdmin) -admin.site.register(AdditionalFieldValue, AdditionalFieldValueAdmin) -admin.site.register(ConsentFieldValue, ConsentFieldValueAdmin) -admin.site.register(Role, RoleAdmin) diff --git a/rdmo/accounts/templates/socialaccount/snippets/provider_list.html b/rdmo/accounts/templates/socialaccount/snippets/provider_list.html index f8f4e278cd..0d95b4b405 100644 --- a/rdmo/accounts/templates/socialaccount/snippets/provider_list.html +++ b/rdmo/accounts/templates/socialaccount/snippets/provider_list.html @@ -24,13 +24,23 @@ ORCID sign in -{% elif provider.id == 'keycloak' %} -
  • - - Keycloak sign in - -
  • +{% elif provider.id == 'openid_connect' %} + {% if provider.app.provider_id == 'keycloak' %} +
  • + + Keycloak sign in + +
  • + {% else %} +
  • + + {{ provider.name }} + +
  • +{% endif %} + {% else %}
  • { + return fetch(baseUrl + url).catch(error => { throw new ApiError(error.message) }).then(response => { if (response.ok) { @@ -28,7 +30,7 @@ class BaseApi { } static post(url, data) { - return fetch(url, { + return fetch(baseUrl + url, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -51,7 +53,7 @@ class BaseApi { } static put(url, data) { - return fetch(url, { + return fetch(baseUrl + url, { method: 'PUT', headers: { 'Content-Type': 'application/json', @@ -74,7 +76,7 @@ class BaseApi { } static delete(url) { - return fetch(url, { + return fetch(baseUrl + url, { method: 'DELETE', headers: { 'Content-Type': 'application/json', @@ -99,7 +101,7 @@ class BaseApi { var formData = new FormData() formData.append('file', file) - return fetch(url, { + return fetch(baseUrl + url, { method: 'POST', headers: { 'X-CSRFToken': Cookies.get('csrftoken') diff --git a/rdmo/core/assets/js/utils/baseUrl.js b/rdmo/core/assets/js/utils/baseUrl.js new file mode 100644 index 0000000000..22a17df960 --- /dev/null +++ b/rdmo/core/assets/js/utils/baseUrl.js @@ -0,0 +1,2 @@ +// take the baseurl from the of the django template +export default document.querySelector('meta[name="baseurl"]').content.replace(/\/+$/, '') diff --git a/rdmo/core/constants.py b/rdmo/core/constants.py index 38a3ae6a45..1e1cbc3a78 100644 --- a/rdmo/core/constants.py +++ b/rdmo/core/constants.py @@ -58,3 +58,22 @@ 'views.add_view', 'views.change_view', 'views.delete_view' ) } + +HUMAN2BYTES_MAPPER = { + "b": {"base": 1000, "power": 0}, + "kb": {"base": 1000, "power": 1}, + "k": {"base": 1000, "power": 1}, + "mb": {"base": 1000, "power": 2}, + "m": {"base": 1000, "power": 2}, + "gb": {"base": 1000, "power": 3}, + "g": {"base": 1000, "power": 3}, + "tb": {"base": 1000, "power": 4}, + "t": {"base": 1000, "power": 4}, + "p": {"base": 1000, "power": 5}, + "pb": {"base": 1000, "power": 5}, + "kib": {"base": 1024, "power": 1}, + "mib": {"base": 1024, "power": 2}, + "gib": {"base": 1024, "power": 3}, + "tib": {"base": 1024, "power": 4}, + "pib": {"base": 1024, "power": 5}, +} diff --git a/rdmo/core/static/core/css/base.scss b/rdmo/core/static/core/css/base.scss index 5d372c3206..02bda4951e 100644 --- a/rdmo/core/static/core/css/base.scss +++ b/rdmo/core/static/core/css/base.scss @@ -129,9 +129,14 @@ table { } } +details { + margin-bottom: 10px; +} + summary { - color: $link-color; + display: list-item; cursor: pointer; + margin-bottom: 5px; } metadata { @@ -477,3 +482,13 @@ li.has-warning > a.control-label > i { text-decoration: underline; text-decoration-style: dotted; } + +.more, +.show-less { + display: none; +} +.show-more, +.show-less { + color: $link-color; + cursor: pointer; +} diff --git a/rdmo/core/static/core/js/core.js b/rdmo/core/static/core/js/core.js index 39f154fc2c..ee790f74fa 100644 --- a/rdmo/core/static/core/js/core.js +++ b/rdmo/core/static/core/js/core.js @@ -16,6 +16,13 @@ angular.module('core', ['ngResource']) method: 'PUT', params: {} }; + $resourceProvider.defaults.actions.postAction = { + method: 'POST', + params: { + id: '@id', + detail_action: '@detail_action' + } + }; }]) .filter('capitalize', function() { diff --git a/rdmo/core/static/core/js/utils.js b/rdmo/core/static/core/js/utils.js new file mode 100644 index 0000000000..8c34a73526 --- /dev/null +++ b/rdmo/core/static/core/js/utils.js @@ -0,0 +1,11 @@ +function showMore(element) { + $(element).siblings('.more').show(); + $(element).siblings('.show-less').show(); + $(element).hide(); +} + +function showLess(element) { + $(element).siblings('.more').hide(); + $(element).siblings('.show-more').show(); + $(element).hide(); +} diff --git a/rdmo/core/templates/core/base.html b/rdmo/core/templates/core/base.html index fbe8dcec9a..d9508e1be9 100644 --- a/rdmo/core/templates/core/base.html +++ b/rdmo/core/templates/core/base.html @@ -20,6 +20,9 @@ {% endblock %} {% block js %} + {% compress js %} + + {% endcompress %} {% endblock %} {% block head %}{% endblock %} diff --git a/rdmo/core/tests/test_utils.py b/rdmo/core/tests/test_utils.py index f0d9bfb119..5964a56d50 100644 --- a/rdmo/core/tests/test_utils.py +++ b/rdmo/core/tests/test_utils.py @@ -1,6 +1,8 @@ +from typing import Optional + import pytest -from rdmo.core.utils import join_url, sanitize_url +from rdmo.core.utils import human2bytes, join_url, sanitize_url urls = ( ('', ''), @@ -15,6 +17,12 @@ (1, ''), ) +human2bytes_test_values = ( + ("1Gb", 1e+9), + (None, 0), + ("0", 0), +) + @pytest.mark.parametrize("url,sanitized_url", urls) def test_sanitize_url(url, sanitized_url): @@ -23,3 +31,8 @@ def test_sanitize_url(url, sanitized_url): def test_join_url(): assert join_url('https://example.com//', '/terms', 'foo') == 'https://example.com/terms/foo' + + +@pytest.mark.parametrize("human,bytes", human2bytes_test_values) +def test_human2bytes(human: Optional[str], bytes: float): + assert human2bytes(human) == bytes diff --git a/rdmo/core/utils.py b/rdmo/core/utils.py index 22d9d42b31..c2c944028c 100644 --- a/rdmo/core/utils.py +++ b/rdmo/core/utils.py @@ -18,6 +18,8 @@ from defusedcsv import csv from markdown import markdown +from .constants import HUMAN2BYTES_MAPPER + log = logging.getLogger(__name__) @@ -351,32 +353,15 @@ def copy_model(instance, **kwargs): def human2bytes(string): - if not string: + if not string or string == '0': return 0 m = re.match(r'([0-9.]+)\s*([A-Za-z]+)', string) number, unit = float(m.group(1)), m.group(2).strip().lower() - if unit == 'kb' or unit == 'k': - return number * 1000 - elif unit == 'mb' or unit == 'm': - return number * 1000**2 - elif unit == 'gb' or unit == 'g': - return number * 1000**3 - elif unit == 'tb' or unit == 't': - return number * 1000**4 - elif unit == 'pb' or unit == 'p': - return number * 1000**5 - elif unit == 'kib': - return number * 1024 - elif unit == 'mib': - return number * 1024**2 - elif unit == 'gib': - return number * 1024**3 - elif unit == 'tib': - return number * 1024**4 - elif unit == 'pib': - return number * 1024**5 + conversion = HUMAN2BYTES_MAPPER[unit] + number = number*conversion['base']**(conversion['power']) + return number def is_truthy(value): @@ -384,15 +369,26 @@ def is_truthy(value): def markdown2html(markdown_string): - # adoption of the normal markdown function which also converts - # `[]{}` to <span title="<title>"><string></span> to - # allow for underlined tooltips - html = markdown(force_str(markdown_string)) + # adoption of the normal markdown function + html = markdown(force_str(markdown_string)).strip() + + # convert `[<string>]{<title>}` to <span title="<title>"><string></span> to allow for underlined tooltips html = re.sub( r'\[(.*?)\]\{(.*?)\}', r'<span data-toggle="tooltip" data-placement="bottom" data-html="true" title="\2">\1</span>', html ) + + # convert everything after `{more}` to <span class="more"><string></span> to be shown/hidden on user input + show_string = _('show more') + hide_string = _('show less') + html = re.sub( + r'(\{more\})(.*?)</p>$', + f'<span class="show-more" onclick="showMore(this)">... ({show_string})</span>' + r'<span class="more">\2</span>' + f'<span class="show-less" onclick="showLess(this)"> ({hide_string})</span></p>', + html + ) return html diff --git a/rdmo/domain/admin.py b/rdmo/domain/admin.py index 756e41f4d3..44377ac95d 100644 --- a/rdmo/domain/admin.py +++ b/rdmo/domain/admin.py @@ -19,6 +19,7 @@ def clean(self): AttributeLockedValidator(self.instance)(self.cleaned_data) +@admin.register(Attribute) class AttributeAdmin(admin.ModelAdmin): form = AttributeAdminForm @@ -37,6 +38,3 @@ def values_count(self, obj): def projects_count(self, obj): return obj.projects_count - - -admin.site.register(Attribute, AttributeAdmin) diff --git a/rdmo/management/assets/js/reducers/configReducer.js b/rdmo/management/assets/js/reducers/configReducer.js index 12f8196864..4eb2500380 100644 --- a/rdmo/management/assets/js/reducers/configReducer.js +++ b/rdmo/management/assets/js/reducers/configReducer.js @@ -1,7 +1,9 @@ import set from 'lodash/set' +import baseUrl from 'rdmo/core/assets/js/utils/baseUrl' + const initialState = { - baseUrl: '/management/', + baseUrl: baseUrl + '/management/', settings: {}, filter: {}, display: {} diff --git a/rdmo/management/tests/test_frontend.py b/rdmo/management/tests/test_frontend.py new file mode 100644 index 0000000000..e4d30cd18a --- /dev/null +++ b/rdmo/management/tests/test_frontend.py @@ -0,0 +1,192 @@ +import os +import re +from dataclasses import dataclass +from urllib.parse import urlparse + +import pytest + +from django.core.management import call_command + +from playwright.sync_api import Page, expect +from pytest_django.live_server_helper import LiveServer + +from rdmo.accounts.utils import set_group_permissions +from rdmo.conditions.models import Condition +from rdmo.core.models import Model +from rdmo.domain.models import Attribute +from rdmo.options.models import Option, OptionSet +from rdmo.questions.models import Catalog, Question, Section +from rdmo.questions.models import Page as PageModel +from rdmo.questions.models.questionset import QuestionSet +from rdmo.tasks.models import Task +from rdmo.views.models import View + +pytestmark = pytest.mark.e2e + +# needed for playwright to run +os.environ.setdefault("DJANGO_ALLOW_ASYNC_UNSAFE", "true") + + +@dataclass +class ModelHelper: + """Helper class to bundle information about models for test cases.""" + + model: Model + form_field: str = "URI Path" + db_field: str = "uri_path" + has_nested: bool = False + + @property + def url(self) -> str: + return f"{self.model._meta.model_name}s" + + @property + def verbose_name(self) -> str: + """Return the verbose_name for the model.""" + return self.model._meta.verbose_name + + @property + def verbose_name_plural(self) -> str: + """Return the verbose_name_plural for the model.""" + return self.model._meta.verbose_name_plural + + +@pytest.fixture(scope="function") +def e2e_tests_django_db_setup(django_db_setup, django_db_blocker, fixtures): + """Set up database and populate with fixtures, that get restored for every test case.""" + with django_db_blocker.unblock(): + call_command("loaddata", *fixtures) + set_group_permissions() + + +@pytest.fixture(scope="session") +def base_url(live_server: LiveServer) -> str: + """Enable playwright to address URLs with base URL automatically prefixed.""" + return live_server.url + + +@pytest.fixture +def logged_in_admin_user(e2e_tests_django_db_setup, page: Page) -> Page: + """Log in as admin user through django login UI, returns logged in page for e2e tests.""" + page.goto("/account/login") + page.get_by_label("Username").fill("admin", timeout=5000) + page.get_by_label("Password").fill("admin") + page.get_by_role("button", name="Login").click() + page.goto("/management") + yield page + + +model_helpers = ( + ModelHelper(Catalog, has_nested=True), + ModelHelper(Section, has_nested=True), + ModelHelper(PageModel, has_nested=True), + ModelHelper(QuestionSet, has_nested=True), + ModelHelper(Question), + ModelHelper( + Attribute, has_nested=True, form_field="Key", db_field="key" + ), + ModelHelper(OptionSet, has_nested=True), + ModelHelper(Option), + ModelHelper(Condition), + ModelHelper(Task), + ModelHelper(View), +) + +@pytest.mark.parametrize("helper", model_helpers) +def test_management_navigation(logged_in_admin_user: Page, helper: ModelHelper) -> None: + """Test that each content type is available through the navigation.""" + page = logged_in_admin_user + expect(page.get_by_role("heading", name="Management")).to_be_visible() + + # click a link in the navigation + name = helper.verbose_name_plural + page.get_by_role("link", name=name, exact=True).click() + + # make sure the browser opened a new page + url_name = name.lower() + url_name = url_name.replace(" ", "") + expect(page).to_have_url(re.compile(rf".*/{url_name}/")) + + +@pytest.mark.parametrize("helper", model_helpers) +def test_management_has_items(logged_in_admin_user: Page, helper: ModelHelper) -> None: + """Test all items in database are visible in management UI.""" + page = logged_in_admin_user + num_items_in_database = helper.model.objects.count() + page.goto(f"/management/{helper.url}") + items_in_ui = page.locator(".list-group > .list-group-item") + expect(items_in_ui).to_have_count(num_items_in_database) + + +@pytest.mark.parametrize("helper", model_helpers) +def test_management_nested_view( + logged_in_admin_user: Page, helper: ModelHelper +) -> None: + """For each element type, that has a nested view, click the first example.""" + page = logged_in_admin_user + page.goto(f"/management/{helper.url}") + # Open nested view for element type + if helper.has_nested: + page.get_by_title(f"View {helper.verbose_name} nested").first.click() + expect(page.locator(".panel-default").first).to_be_visible() + expect(page.locator(".panel-default > .panel-body").first).to_be_visible() + + +@pytest.mark.parametrize("helper", model_helpers) +def test_management_create_model( + logged_in_admin_user: Page, helper: ModelHelper +) -> None: + """Test management UI can create objects in the database.""" + page = logged_in_admin_user + num_objects_at_start = helper.model.objects.count() + page.goto(f"/management/{helper.url}") + # click "New" button + page.get_by_role("button", name="New").click() + # fill mandatory fields + value = "some-value" + page.get_by_label(helper.form_field).fill(value) + if helper.model == Condition: + # conditions need to have a source attribute + source_form = ( + page.locator(".form-group") + .filter(has_text="Source") + .locator(".select-item > .react-select") + ) + source_form.click() + page.keyboard.type(Attribute.objects.first().uri) + page.keyboard.press("Enter") + + # save + page.get_by_role("button", name="Create").nth(1).click() + # check if new item is in list + items_in_ui = page.locator(".list-group > .list-group-item") + expect(items_in_ui).to_have_count(num_objects_at_start + 1) + + num_objects_after_save = helper.model.objects.count() + assert num_objects_after_save - num_objects_at_start == 1 + query = {helper.db_field: value} + assert helper.model.objects.get(**query) + + +@pytest.mark.parametrize("helper", model_helpers) +def test_management_edit_model(logged_in_admin_user: Page, helper: ModelHelper) -> None: + page = logged_in_admin_user + page.goto(f"/management/{helper.url}") + # click edit + edit_button_title = f"Edit {helper.verbose_name}" + page.get_by_title(f"{edit_button_title}").first.click() + # edit + page.get_by_label("Comment").click() + comment = "this is a comment." + page.get_by_label("Comment").fill(comment) + # save + page.get_by_role("button", name="Save").nth(1).click() + # click on edit element again + page.get_by_title(f"{edit_button_title}").first.click() + # check the updated comment + comment_locator = page.get_by_label("Comment") + expect(comment_locator).to_have_text(comment) + # compare the updated comment with element object from db + url_id = int(urlparse(page.url).path.rstrip("/").split("/")[-1]) + model_obj = helper.model.objects.get(id=url_id) + assert model_obj.comment == comment diff --git a/rdmo/options/admin.py b/rdmo/options/admin.py index bf5e21c65c..f95d8787bd 100644 --- a/rdmo/options/admin.py +++ b/rdmo/options/admin.py @@ -39,6 +39,7 @@ class OptionSetOptionInline(admin.TabularInline): extra = 0 +@admin.register(OptionSet) class OptionSetAdmin(admin.ModelAdmin): form = OptionSetAdminForm inlines = (OptionSetOptionInline, ) @@ -49,6 +50,7 @@ class OptionSetAdmin(admin.ModelAdmin): filter_horizontal = ('editors', 'conditions') +@admin.register(Option) class OptionAdmin(admin.ModelAdmin): form = OptionAdminForm @@ -57,7 +59,3 @@ class OptionAdmin(admin.ModelAdmin): readonly_fields = ('uri', ) list_filter = ('editors', 'optionsets', 'additional_input') filter_horizontal = ('editors', ) - - -admin.site.register(OptionSet, OptionSetAdmin) -admin.site.register(Option, OptionAdmin) diff --git a/rdmo/overlays/admin.py b/rdmo/overlays/admin.py index d1267b834e..ee663ac45c 100644 --- a/rdmo/overlays/admin.py +++ b/rdmo/overlays/admin.py @@ -3,10 +3,8 @@ from .models import Overlay +@admin.register(Overlay) class OverlayAdmin(admin.ModelAdmin): search_fields = ('user__username', 'url_name', 'current') list_display = ('user', 'site', 'url_name', 'current') list_filter = ('url_name', 'current') - - -admin.site.register(Overlay, OverlayAdmin) diff --git a/rdmo/projects/admin.py b/rdmo/projects/admin.py index e8890cf19d..fcf6749b5c 100644 --- a/rdmo/projects/admin.py +++ b/rdmo/projects/admin.py @@ -15,6 +15,7 @@ ) +@admin.register(Project) class ProjectAdmin(admin.ModelAdmin): search_fields = ('title', 'user__username') list_display = ('title', 'owners', 'updated', 'created') @@ -32,43 +33,51 @@ def owners(self, obj): return ', '.join([membership.user.username for membership in obj.owner_memberships]) +@admin.register(Membership) class MembershipAdmin(admin.ModelAdmin): search_fields = ('project__title', 'user__username', 'role') list_display = ('project', 'user', 'role') +@admin.register(Continuation) class ContinuationAdmin(admin.ModelAdmin): search_fields = ('project__title', 'user__username') list_display = ('project', 'user', 'page') +@admin.register(Integration) class IntegrationAdmin(admin.ModelAdmin): search_fields = ('project__title', 'provider_key') list_display = ('project', 'provider_key') +@admin.register(IntegrationOption) class IntegrationOptionAdmin(admin.ModelAdmin): search_fields = ('integration__project__title', 'key', 'value') list_display = ('integration', 'key', 'value') +@admin.register(Invite) class InviteAdmin(admin.ModelAdmin): search_fields = ('project__title', 'user__username', 'email', 'role') list_display = ('project', 'user', 'email', 'token', 'timestamp') readonly_fields = ('token', 'timestamp') +@admin.register(Issue) class IssueAdmin(admin.ModelAdmin): search_fields = ('project__title', 'task', 'status') list_display = ('project', 'task', 'status') list_filter = ('status', ) +@admin.register(IssueResource) class IssueResourceAdmin(admin.ModelAdmin): search_fields = ('issue__project__title', 'url') list_display = ('issue', 'url') +@admin.register(Snapshot) class SnapshotAdmin(admin.ModelAdmin): search_fields = ('title', 'project__title', 'project__user__username') list_display = ('title', 'project', 'owners', 'updated', 'created') @@ -86,6 +95,7 @@ def owners(self, obj): return ', '.join([membership.user.username for membership in obj.project.owner_memberships]) +@admin.register(Value) class ValueAdmin(admin.ModelAdmin): search_fields = ('attribute__uri', 'project__title', 'snapshot__title', 'project__user__username') list_display = ('attribute', 'set_prefix', 'set_index', 'collection_index', 'project', 'snapshot_title') @@ -94,15 +104,3 @@ class ValueAdmin(admin.ModelAdmin): def snapshot_title(self, obj): if obj.snapshot: return obj.snapshot.title - - -admin.site.register(Project, ProjectAdmin) -admin.site.register(Membership, MembershipAdmin) -admin.site.register(Continuation, ContinuationAdmin) -admin.site.register(Integration, IntegrationAdmin) -admin.site.register(IntegrationOption, IntegrationOptionAdmin) -admin.site.register(Invite, InviteAdmin) -admin.site.register(Issue, IssueAdmin) -admin.site.register(IssueResource, IssueResourceAdmin) -admin.site.register(Snapshot, SnapshotAdmin) -admin.site.register(Value, ValueAdmin) diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index 8ee22b0da2..72e7c21a0b 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -1,5 +1,6 @@ from django.conf import settings from django.db import models +from django.db.models import Q from mptt.models import TreeManager from mptt.querysets import TreeQuerySet @@ -141,6 +142,12 @@ def filter_user(self, user): else: return self.none() + def exclude_empty(self): + return self.exclude((Q(text='') | Q(text=None)) & Q(option=None) & (Q(file='') | Q(file=None))) + + def distinct_list(self): + return self.order_by('attribute').values_list('attribute', 'set_prefix', 'set_index').distinct() + class ProjectManager(CurrentSiteManagerMixin, TreeManager): diff --git a/rdmo/projects/migrations/0059_project_progress.py b/rdmo/projects/migrations/0059_project_progress.py new file mode 100644 index 0000000000..abce63ea50 --- /dev/null +++ b/rdmo/projects/migrations/0059_project_progress.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.19 on 2023-08-24 09:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('projects', '0058_meta'), + ] + + operations = [ + migrations.AddField( + model_name='project', + name='progress_count', + field=models.IntegerField(help_text='The number of values for the progress bar.', null=True, verbose_name='Progress count'), + ), + migrations.AddField( + model_name='project', + name='progress_total', + field=models.IntegerField(help_text='The total number of expected values for the progress bar.', null=True, verbose_name='Progress total'), + ), + ] diff --git a/rdmo/projects/models/project.py b/rdmo/projects/models/project.py index eae2128fe2..d13b33a60a 100644 --- a/rdmo/projects/models/project.py +++ b/rdmo/projects/models/project.py @@ -2,7 +2,6 @@ from django.contrib.sites.models import Site from django.core.exceptions import ValidationError from django.db import models -from django.db.models import Exists, OuterRef from django.db.models.signals import pre_delete from django.dispatch import receiver from django.urls import reverse @@ -12,8 +11,7 @@ from mptt.models import MPTTModel, TreeForeignKey from rdmo.core.models import Model -from rdmo.domain.models import Attribute -from rdmo.questions.models import Catalog, Question +from rdmo.questions.models import Catalog from rdmo.tasks.models import Task from rdmo.views.models import View @@ -65,6 +63,16 @@ class Project(MPTTModel, Model): verbose_name=_('Views'), help_text=_('The views that will be used for this project.') ) + progress_total = models.IntegerField( + null=True, + verbose_name=_('Progress total'), + help_text=_('The total number of expected values for the progress bar.') + ) + progress_count = models.IntegerField( + null=True, + verbose_name=_('Progress count'), + help_text=_('The number of values for the progress bar.') + ) class Meta: ordering = ('tree_id', 'level', 'title') @@ -86,36 +94,6 @@ def clean(self): 'parent': [_('A project may not be moved to be a child of itself or one of its descendants.')] }) - @property - def progress(self): - # create a queryset for the attributes of the catalog for this project - # the subquery is used to query only attributes which have a question in the catalog, which is not optional - questions = Question.objects.filter_by_catalog(self.catalog) \ - .filter(attribute_id=OuterRef('pk')).exclude(is_optional=True) - attributes = Attribute.objects.annotate(active=Exists(questions)).filter(active=True).distinct() - - # query the total number of attributes from the qs above - total = attributes.count() - - # query all current values with attributes from the qs above, but where the text, option, or file field is set, - # and count only one value per attribute - values = self.values.filter(snapshot=None) \ - .filter(attribute__in=attributes) \ - .exclude((models.Q(text='') | models.Q(text=None)) & models.Q(option=None) & - (models.Q(file='') | models.Q(file=None))) \ - .distinct().values('attribute').count() - - try: - ratio = values / total - except ZeroDivisionError: - ratio = 0 - - return { - 'total': total, - 'values': values, - 'ratio': ratio - } - @property def catalog_uri(self): if self.catalog is not None: diff --git a/rdmo/projects/permissions.py b/rdmo/projects/permissions.py index c549b02e49..cb924a9c57 100644 --- a/rdmo/projects/permissions.py +++ b/rdmo/projects/permissions.py @@ -62,3 +62,14 @@ class HasProjectPagePermission(HasProjectPermission): def get_required_object_permissions(self, method, model_cls): return ('projects.view_page_object', ) + + +class HasProjectProgressPermission(HasProjectPermission): + + def get_required_object_permissions(self, method, model_cls): + if method == 'GET': + return ('projects.view_project_object', ) + elif method == 'POST': + return ('projects.change_project_progress_object', ) + else: + raise RuntimeError('Unsupported method for HasProjectProgressPermission') diff --git a/rdmo/projects/progress.py b/rdmo/projects/progress.py new file mode 100644 index 0000000000..ef082fa507 --- /dev/null +++ b/rdmo/projects/progress.py @@ -0,0 +1,148 @@ +from collections import defaultdict + +from django.db.models import Exists, OuterRef, Q + +from rdmo.conditions.models import Condition +from rdmo.questions.models import Page, Question, QuestionSet + + +def resolve_conditions(project, values): + # get all conditions for this catalog + pages_conditions_subquery = Page.objects.filter_by_catalog(project.catalog) \ + .filter(conditions=OuterRef('pk')) + questionsets_conditions_subquery = QuestionSet.objects.filter_by_catalog(project.catalog) \ + .filter(conditions=OuterRef('pk')) + questions_conditions_subquery = Question.objects.filter_by_catalog(project.catalog) \ + .filter(conditions=OuterRef('pk')) + + catalog_conditions = Condition.objects.annotate(has_page=Exists(pages_conditions_subquery)) \ + .annotate(has_questionset=Exists(questionsets_conditions_subquery)) \ + .annotate(has_question=Exists(questions_conditions_subquery)) \ + .filter(Q(has_page=True) | Q(has_questionset=True) | Q(has_question=True)) \ + .distinct().select_related('source', 'target_option') + + # evaluate conditions + conditions = set() + for condition in catalog_conditions: + if condition.resolve(values): + conditions.add(condition.id) + + # return all true conditions for this project + return conditions + + +def compute_navigation(section, project, snapshot=None): + # get all values for this project and snapshot + values = project.values.filter(snapshot=snapshot).select_related('attribute', 'option') + + # get true conditions + conditions = resolve_conditions(project, values) + + # compute sets from values (including empty values) + sets = defaultdict(lambda: defaultdict(list)) + for attribute, set_prefix, set_index in values.distinct_list(): + sets[attribute][set_prefix].append(set_index) + + # query distinct, non empty set values + values_list = values.exclude_empty().distinct_list() + + navigation = [] + for catalog_section in project.catalog.elements: + navigation_section = { + 'id': catalog_section.id, + 'uri': catalog_section.uri, + 'title': catalog_section.title, + 'first': catalog_section.elements[0].id if section.elements else None + } + if catalog_section.id == section.id: + navigation_section['pages'] = [] + for page in catalog_section.elements: + pages_conditions = {page.id for page in page.conditions.all()} + show = bool(not pages_conditions or pages_conditions.intersection(conditions)) + + # count the total number of questions, taking sets and conditions into account + counts = count_questions(page, sets, conditions) + + # filter the values_list for the attributes, and compute the total sum of counts + count = len(tuple(filter(lambda value: value[0] in counts.keys(), values_list))) + total = sum(counts.values()) + + navigation_section['pages'].append({ + 'id': page.id, + 'uri': page.uri, + 'title': page.title, + 'show': show, + 'count': count, + 'total': total + }) + + navigation.append(navigation_section) + + return navigation + + +def compute_progress(project, snapshot=None): + # get all values for this project and snapshot + values = project.values.filter(snapshot=snapshot).select_related('attribute', 'option') + + # get true conditions + conditions = resolve_conditions(project, values) + + # compute sets from values (including empty values) + sets = defaultdict(lambda: defaultdict(list)) + for attribute, set_prefix, set_index in values.distinct_list(): + sets[attribute][set_prefix].append(set_index) + + # query distinct, non empty set values + values_list = values.exclude_empty().distinct_list() + + + # count the total number of questions, taking sets and conditions into account + counts = count_questions(project.catalog, sets, conditions) + + # filter the values_list for the attributes, and compute the total sum of counts + count = len(tuple(filter(lambda value: value[0] in counts.keys(), values_list))) + total = sum(counts.values()) + + return count, total + + +def count_questions(element, sets, conditions): + counts = defaultdict(int) + + # obtain the maximum number of set-distinct values the questions in this element + # this number is how often each question is displayed and we will use this number + # to determine how often a question needs to be counted + if isinstance(element, (Page, QuestionSet)) and element.is_collection: + set_count = 0 + + if element.attribute is not None: + child_count = sum(len(set_indexes) for set_indexes in sets[element.attribute.id].values()) + set_count = max(set_count, child_count) + + for child in element.elements: + if isinstance(child, Question): + child_count = sum(len(set_indexes) for set_indexes in sets[child.attribute.id].values()) + set_count = max(set_count, child_count) + else: + set_count = 1 + + # loop over all children of this element + for child in element.elements: + # look for the elements conditions + if isinstance(child, (Page, QuestionSet, Question)): + child_conditions = {condition.id for condition in child.conditions.all()} + else: + child_conditions = [] + + if not child_conditions or child_conditions.intersection(conditions): + if isinstance(child, Question): + # for questions add the set_count to the counts dict + # use the max function, since the same attribute could apear twice in the tree + if child.attribute is not None: + counts[child.attribute.id] = max(counts[child.attribute.id], set_count) + else: + # for everthing else, call this function recursively + counts.update(count_questions(child, sets, conditions)) + + return counts diff --git a/rdmo/projects/rules.py b/rdmo/projects/rules.py index 93a2fec87e..f11061e064 100644 --- a/rdmo/projects/rules.py +++ b/rdmo/projects/rules.py @@ -57,6 +57,7 @@ def is_site_manager_for_current_site(user, request): rules.add_perm('projects.view_project_object', is_project_member | is_site_manager) rules.add_perm('projects.change_project_object', is_project_manager | is_project_owner | is_site_manager) +rules.add_perm('projects.change_project_progress_object', is_project_author | is_project_manager | is_project_owner | is_site_manager) # noqa: E501 rules.add_perm('projects.delete_project_object', is_project_owner | is_site_manager) rules.add_perm('projects.leave_project_object', is_current_project_member) rules.add_perm('projects.export_project_object', is_project_owner | is_project_manager | is_site_manager) diff --git a/rdmo/projects/serializers/v1/__init__.py b/rdmo/projects/serializers/v1/__init__.py index 7e65e7cb9f..a5ec382b30 100644 --- a/rdmo/projects/serializers/v1/__init__.py +++ b/rdmo/projects/serializers/v1/__init__.py @@ -8,7 +8,7 @@ from rdmo.services.validators import ProviderValidator from ...models import Integration, IntegrationOption, Invite, Issue, IssueResource, Membership, Project, Snapshot, Value -from ...validators import ValueValidator +from ...validators import ValueConflictValidator, ValueQuotaValidator class UserSerializer(serializers.ModelSerializer): @@ -259,7 +259,10 @@ class Meta: 'unit', 'external_id' ) - validators = (ValueValidator(), ) + validators = ( + ValueConflictValidator(), + ValueQuotaValidator() + ) class MembershipSerializer(serializers.ModelSerializer): diff --git a/rdmo/projects/serializers/v1/overview.py b/rdmo/projects/serializers/v1/overview.py index a0428b9836..cc5cfdf3e2 100644 --- a/rdmo/projects/serializers/v1/overview.py +++ b/rdmo/projects/serializers/v1/overview.py @@ -1,51 +1,18 @@ from rest_framework import serializers from rdmo.projects.models import Project -from rdmo.questions.models import Catalog, Page, Section - - -class PageSerializer(serializers.ModelSerializer): - - class Meta: - model = Page - fields = ( - 'id', - 'title', - 'has_conditions' - ) - - -class SectionSerializer(serializers.ModelSerializer): - - pages = serializers.SerializerMethodField() - - class Meta: - model = Section - fields = ( - 'id', - 'title', - 'pages' - ) - - def get_pages(self, obj): - return PageSerializer(obj.elements, many=True, read_only=True).data +from rdmo.questions.models import Catalog class CatalogSerializer(serializers.ModelSerializer): - sections = serializers.SerializerMethodField() - class Meta: model = Catalog fields = ( 'id', - 'title', - 'sections' + 'title' ) - def get_sections(self, obj): - return SectionSerializer(obj.elements, many=True, read_only=True).data - class ProjectOverviewSerializer(serializers.ModelSerializer): diff --git a/rdmo/projects/serializers/v1/page.py b/rdmo/projects/serializers/v1/page.py index 49fe49fd03..de51adeaa9 100644 --- a/rdmo/projects/serializers/v1/page.py +++ b/rdmo/projects/serializers/v1/page.py @@ -181,6 +181,7 @@ def get_section(self, obj): return { 'id': section.id, 'title': section.title, + 'first': section.elements[0].id if section.elements else None } if section else {} def get_prev_page(self, obj): diff --git a/rdmo/projects/static/projects/css/project_questions.scss b/rdmo/projects/static/projects/css/project_questions.scss index 9f1a92f117..f4141d99b9 100644 --- a/rdmo/projects/static/projects/css/project_questions.scss +++ b/rdmo/projects/static/projects/css/project_questions.scss @@ -302,3 +302,9 @@ .project-progress { margin-bottom: 10px; } +.project-progress-count { + position: absolute; + left: 0; + right: 0; + text-align: center; +} diff --git a/rdmo/projects/static/projects/js/project_questions/services.js b/rdmo/projects/static/projects/js/project_questions/services.js index efa8e4ea9d..d8df624071 100644 --- a/rdmo/projects/static/projects/js/project_questions/services.js +++ b/rdmo/projects/static/projects/js/project_questions/services.js @@ -9,7 +9,7 @@ angular.module('project_questions') /* configure resources */ var resources = { - projects: $resource(baseurl + 'api/v1/projects/projects/:id/:detail_action/'), + projects: $resource(baseurl + 'api/v1/projects/projects/:id/:detail_action/:detail_id/'), values: $resource(baseurl + 'api/v1/projects/projects/:project/values/:id/:detail_action/'), pages: $resource(baseurl + 'api/v1/projects/projects/:project/pages/:list_action/:id/'), settings: $resource(baseurl + 'api/v1/core/settings/') @@ -147,13 +147,14 @@ angular.module('project_questions') } return service.fetchPage(page_id) + .then(service.fetchNavigation) .then(service.fetchOptions) .then(service.fetchValues) .then(service.fetchConditions) .then(function () { // copy future objects angular.forEach([ - 'page', 'progress', 'attributes', 'questionsets', 'questions', 'valuesets', 'values' + 'page', 'progress', 'attributes', 'questionsets', 'questions', 'valuesets', 'values', 'navigation' ], function (key) { service[key] = angular.copy(future[key]); }); @@ -276,6 +277,16 @@ angular.module('project_questions') }); }; + service.fetchNavigation = function() { + future.navigation = resources.projects.query({ + id: service.project.id, + detail_id: future.page.section.id, + detail_action: 'navigation' + }); + + return future.navigation.$promise + }; + service.initPage = function(page) { // store attributes in a separate array if (page.attribute !== null) future.attributes.push(page.attribute); @@ -706,6 +717,9 @@ angular.module('project_questions') }; service.storeValue = function(value, question, set_prefix, set_index, collection_index) { + // reset value errors + value.errors = [] + if (angular.isDefined(value.removed) && value.removed) { // remove additional_input from unselected checkboxes value.additional_input = {}; @@ -826,10 +840,16 @@ angular.module('project_questions') }, function (response) { if (response.status == 500) { service.error = response; + } else if (response.status == 400) { + service.error = true; + value.errors = Object.keys(response.data); + } else if (response.status == 404) { + service.error = true; + value.errors = ['not_found'] } }) } -}; + }; service.storeValues = function() { var promises = []; @@ -882,16 +902,7 @@ angular.module('project_questions') } else if (angular.isDefined(page)) { service.initView(page.id); } else if (angular.isDefined(section)) { - if (angular.isDefined(section.pages)) { - service.initView(section.pages[0].id); - } else { - // jump to first page of the section in breadcrumb - // let section_from_service = service.project.catalog.sections.find(x => x.id === section.id) - var section_from_service = $filter('filter')(service.project.catalog.sections, { - id: section.id - })[0] - service.initView(section_from_service.pages[0].id); - } + service.initView(section.first); } else { service.initView(null); } @@ -900,16 +911,7 @@ angular.module('project_questions') if (angular.isDefined(page)) { service.initView(page.id); } else if (angular.isDefined(section)) { - if (angular.isDefined(section.pages)) { - service.initView(section.pages[0].id); - } else { - // jump to first page of the section in breadcrumb - // let section_from_service = service.project.catalog.sections.find(x => x.id === section.id) - var section_from_service = $filter('filter')(service.project.catalog.sections, { - id: section.id - })[0] - service.initView(section_from_service.pages[0].id); - } + service.initView(section.first); } else { service.initView(null); } @@ -946,14 +948,26 @@ angular.module('project_questions') } } else { // update progress - resources.projects.get({ - id: service.project.id, - detail_action: 'progress' - }, function(response) { - if (service.progress.values != response.values) { + if (service.project.read_only !== true) { + resources.projects.postAction({ + id: service.project.id, + detail_action: 'progress' + }, function(response) { service.progress = response - } - }); + }); + } + + // update navigation + if (service.project.read_only !== true) { + resources.projects.query({ + id: service.project.id, + detail_id: future.page.section.id, + detail_action: 'navigation' + }, function(response) { + console.log(response); + service.navigation = response + }); + } // check if we need to refresh the site angular.forEach([service.page].concat(service.questionsets), function(questionset) { @@ -1058,11 +1072,19 @@ angular.module('project_questions') service.values[attribute][set_prefix][set_index][collection_index].autocomplete_text = ''; service.values[attribute][set_prefix][set_index][collection_index].autocomplete_locked = false; service.values[attribute][set_prefix][set_index][collection_index].changed = true; + + if (service.settings.project_questions_autosave) { + service.save(false); + } }; service.removeValue = function(attribute, set_prefix, set_index, collection_index) { service.values[attribute][set_prefix][set_index][collection_index].removed = true; service.values[attribute][set_prefix][set_index][collection_index].changed = true; + + if (service.settings.project_questions_autosave) { + service.save(false); + } }; service.openValueSetFormModal = function(questionset, set_prefix, set_index) { diff --git a/rdmo/projects/templates/projects/project_detail_header_hierarchy.html b/rdmo/projects/templates/projects/project_detail_header_hierarchy.html index 33d6f62a3f..c7004fc525 100644 --- a/rdmo/projects/templates/projects/project_detail_header_hierarchy.html +++ b/rdmo/projects/templates/projects/project_detail_header_hierarchy.html @@ -19,13 +19,13 @@ {% if can_view_parent_project %} <a href="{% url 'project' node.id %}"> {% if node.id == project.id %} - <strong>{{ node.title }}</strong> + <strong>{{ node.title }}</strong> {% project_progress_text node %} {% else %} - {{ node.title }} + {{ node.title }} {% project_progress_text node %} {% endif %} </a> {% else %} - {{ node.title }} + {{ node.title }} {% project_progress_text node %} {% endif %} </li> {% endfor %} diff --git a/rdmo/projects/templates/projects/project_questions_form_group_autocomplete.html b/rdmo/projects/templates/projects/project_questions_form_group_autocomplete.html index e35e795114..2996f3a321 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_autocomplete.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_autocomplete.html @@ -44,6 +44,8 @@ </div> </div> </div> + + {% include 'projects/project_questions_value_errors.html' %} </div> <div ng-if="question.is_collection"> diff --git a/rdmo/projects/templates/projects/project_questions_form_group_checkbox.html b/rdmo/projects/templates/projects/project_questions_form_group_checkbox.html index 48ed47be19..bb344cd818 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_checkbox.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_checkbox.html @@ -24,6 +24,9 @@ ng-disabled="service.project.read_only" ng-change="service.changed(service.values[question.attribute][valueset.set_prefix][valueset.set_index][$index])" /> </label> + <div ng-init="value = service.values[question.attribute][valueset.set_prefix][valueset.set_index][$index]"> + {% include 'projects/project_questions_value_errors.html' %} + </div> </div> </div> </div> diff --git a/rdmo/projects/templates/projects/project_questions_form_group_date.html b/rdmo/projects/templates/projects/project_questions_form_group_date.html index 8994983bd5..36e60cf423 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_date.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_date.html @@ -24,6 +24,8 @@ ng-change="service.changed(value, true)" ng-class="{'default-value': service.isDefaultValue(question, value)}"/> </div> + + {% include 'projects/project_questions_value_errors.html' %} </div> <div ng-if="question.is_collection"> diff --git a/rdmo/projects/templates/projects/project_questions_form_group_file.html b/rdmo/projects/templates/projects/project_questions_form_group_file.html index 4ddf98dc31..ee0db7fad5 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_file.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_file.html @@ -33,6 +33,8 @@ </ul> </label> </div> + + {% include 'projects/project_questions_value_errors.html' %} </div> <div ng-if="question.is_collection"> diff --git a/rdmo/projects/templates/projects/project_questions_form_group_radio.html b/rdmo/projects/templates/projects/project_questions_form_group_radio.html index d1e8f462f6..29e498e8c7 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_radio.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_radio.html @@ -43,6 +43,8 @@ </div> </div> </div> + + {% include 'projects/project_questions_value_errors.html' %} </div> <div ng-if="question.is_collection"> diff --git a/rdmo/projects/templates/projects/project_questions_form_group_range.html b/rdmo/projects/templates/projects/project_questions_form_group_range.html index 384f413d59..39fb0552d5 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_range.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_range.html @@ -31,6 +31,8 @@ ng-class="{'default-value': service.isDefaultValue(question, value)}"/> </div> </div> + + {% include 'projects/project_questions_value_errors.html' %} </div> <div ng-if="question.is_collection"> diff --git a/rdmo/projects/templates/projects/project_questions_form_group_select.html b/rdmo/projects/templates/projects/project_questions_form_group_select.html index d672b98981..4589371cb0 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_select.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_select.html @@ -28,6 +28,8 @@ </option> </select> </div> + + {% include 'projects/project_questions_value_errors.html' %} </div> <div ng-if="question.is_collection"> diff --git a/rdmo/projects/templates/projects/project_questions_form_group_text.html b/rdmo/projects/templates/projects/project_questions_form_group_text.html index d9284bdf56..e3fad422ec 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_text.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_text.html @@ -21,6 +21,8 @@ ng-disabled="service.project.read_only" ng-change="service.changed(value)" ng-class="{'default-value': service.isDefaultValue(question, value)}"> + + {% include 'projects/project_questions_value_errors.html' %} </div> </div> diff --git a/rdmo/projects/templates/projects/project_questions_form_group_textarea.html b/rdmo/projects/templates/projects/project_questions_form_group_textarea.html index 756e21c96e..26baee92a1 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_textarea.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_textarea.html @@ -23,6 +23,8 @@ ng-class="{'default-value': service.isDefaultValue(question, value)}"> </textarea> </div> + + {% include 'projects/project_questions_value_errors.html' %} </div> <div ng-if="question.is_collection"> diff --git a/rdmo/projects/templates/projects/project_questions_form_group_yesno.html b/rdmo/projects/templates/projects/project_questions_form_group_yesno.html index c06ce86618..31dda7fb79 100644 --- a/rdmo/projects/templates/projects/project_questions_form_group_yesno.html +++ b/rdmo/projects/templates/projects/project_questions_form_group_yesno.html @@ -41,6 +41,8 @@ </div> </div> </div> + + {% include 'projects/project_questions_value_errors.html' %} </div> <div ng-if="question.is_collection"> diff --git a/rdmo/projects/templates/projects/project_questions_navigation.html b/rdmo/projects/templates/projects/project_questions_navigation.html index f5945d8db2..ba5f062f12 100644 --- a/rdmo/projects/templates/projects/project_questions_navigation.html +++ b/rdmo/projects/templates/projects/project_questions_navigation.html @@ -3,22 +3,27 @@ {% include 'projects/project_questions_navigation_help.html' %} <ul class="list-unstyled project-questions-overview"> - <li ng-repeat="section in service.project.catalog.sections"> + <li ng-repeat="section in service.navigation"> <a href="" ng-click="service.jump(section)"> {$ section.title $} </a> <ul class="list-unstyled" - ng-show="section.id == service.page.section.id"> + ng-show="section.pages"> <li ng-repeat="page in section.pages" ng-class="{'active': page.id == service.page.id}"> - <a href="" ng-click="service.jump(section, page)"> + + <a href="" ng-click="service.jump(section, page)" ng-show="page.show"> <span>{$ page.title $}<span> - <span ng-show="page.has_conditions" class="small"> - <i class="fa fa-question-circle-o small" aria-hidden="true"></i> + <span ng-show="page.count > 0 && page.count == page.total"> + <i class="fa fa-check" aria-hidden="true"></i> </span> + <span ng-show="page.count > 0 && page.count != page.total"> + ({$ page.count $} of {$ page.total $}) + <span> </a> + <span class="text-muted" ng-hide="page.show">{$ page.title $}<span> </li> </ul> </li> diff --git a/rdmo/projects/templates/projects/project_questions_navigation_help.html b/rdmo/projects/templates/projects/project_questions_navigation_help.html index 73e58ea1a8..60abef29bf 100644 --- a/rdmo/projects/templates/projects/project_questions_navigation_help.html +++ b/rdmo/projects/templates/projects/project_questions_navigation_help.html @@ -12,6 +12,6 @@ <p class="help-block"> {% blocktrans trimmed %} -Entries with <i class="fa fa-question-circle-o small" aria-hidden="true"></i> might be skipped based on your input. +Grey entries will be conditionally skipped based on your input. {% endblocktrans %} </p> diff --git a/rdmo/projects/templates/projects/project_questions_overview.html b/rdmo/projects/templates/projects/project_questions_overview.html index 1ee6ea347b..6058e465a5 100644 --- a/rdmo/projects/templates/projects/project_questions_overview.html +++ b/rdmo/projects/templates/projects/project_questions_overview.html @@ -10,6 +10,9 @@ </ul> <ul class="list-unstyled"> + <li> + <a href="#" onclick="window.location.reload();">{% trans 'Reload page' %}</a> + </li> <li> <a href="{% url 'projects' %}">{% trans 'Back to my projects' %}</a> </li> diff --git a/rdmo/projects/templates/projects/project_questions_progress.html b/rdmo/projects/templates/projects/project_questions_progress.html index 9b66e61bb9..b66e5d4a62 100644 --- a/rdmo/projects/templates/projects/project_questions_progress.html +++ b/rdmo/projects/templates/projects/project_questions_progress.html @@ -1,10 +1,15 @@ {% load i18n %} +<div class="project-progress-count" ng-show="service.progress.ratio <= 0.25"> +{% blocktrans trimmed with count='{$ service.progress.count $}' total='{$ service.progress.total $}' %} + {{ count }} of {{ total }} +{% endblocktrans %} +</div> <div class="progress project-progress"> <div class="progress-bar" role="progressbar" style="width: {$ service.progress.ratio * 100 $}%;"> <span ng-show="service.progress.ratio > 0.25"> - {% blocktrans trimmed with values='{$ service.progress.values $}' total='{$ service.progress.total $}' %} - {{ values }} of {{ total }} + {% blocktrans trimmed with count='{$ service.progress.count $}' total='{$ service.progress.total $}' %} + {{ count }} of {{ total }} {% endblocktrans %} </span> </div> diff --git a/rdmo/projects/templates/projects/project_questions_save_error.html b/rdmo/projects/templates/projects/project_questions_save_error.html index a3d6d90cba..e3a5aba785 100644 --- a/rdmo/projects/templates/projects/project_questions_save_error.html +++ b/rdmo/projects/templates/projects/project_questions_save_error.html @@ -1,7 +1,7 @@ {% load i18n %} {% load core_tags %} - <div class="text-danger" ng-show="service.error"> + <div class="text-danger" ng-show="service.error.statusText && service.error.status"> <p> {% trans 'An error occurred while saving the answer. Please contact support if this problem persists.' %} </p> diff --git a/rdmo/projects/templates/projects/project_questions_value_errors.html b/rdmo/projects/templates/projects/project_questions_value_errors.html new file mode 100644 index 0000000000..d8245287ad --- /dev/null +++ b/rdmo/projects/templates/projects/project_questions_value_errors.html @@ -0,0 +1,21 @@ +{% load i18n %} + + <ul class="help-block list-unstyled" ng-if="value.errors.length > 0"> + <li class="text-danger" ng-repeat="error in value.errors"> + <span ng-show="error == 'conflict'"> + {% blocktrans trimmed %} + This field could not be saved, since somebody else did so while you were editing. + You will need to reload the page to make changes, but your input will be overwritten. + {% endblocktrans %} + </span> + <span ng-show="error == 'not_found'"> + {% blocktrans trimmed %} + This field could not be saved, since somebody else removed it while you were editing. + You will need to reload the page to proceed, but your input will be lost. + {% endblocktrans %} + </span> + <span ng-show="error == 'quota'"> + {% trans 'You reached the file quota for this project.' %} + </span> + </li> + </ul> diff --git a/rdmo/projects/templates/projects/projects.html b/rdmo/projects/templates/projects/projects.html index 0edfaace36..b2aed864a1 100644 --- a/rdmo/projects/templates/projects/projects.html +++ b/rdmo/projects/templates/projects/projects.html @@ -124,6 +124,7 @@ <h1>{% trans 'My Projects' %}</h1> <thead> <tr> <th style="width: 60%;">{% trans 'Name' %}</th> + <th style="width: 20%;">{% trans 'Progress' %}</th> <th style="width: 10%;">{% trans 'Role' %}</th> <th style="width: 23%;">{% trans 'Last changed' %}</th> <th style="width: 7%;"></th> @@ -138,6 +139,9 @@ <h1>{% trans 'My Projects' %}</h1> <strong>{{ project.title }}</strong> </a> </td> + <td> + {% project_progress project %} + </td> <td> {{ project.role|projects_role }} </td> diff --git a/rdmo/projects/templates/projects/site_projects.html b/rdmo/projects/templates/projects/site_projects.html index f2a8f49c23..5caba675ed 100644 --- a/rdmo/projects/templates/projects/site_projects.html +++ b/rdmo/projects/templates/projects/site_projects.html @@ -61,7 +61,8 @@ <h1>{% blocktrans trimmed with site=request.site %}All projects on {{ site }}{% <table class="table projects-table"> <thead> <tr> - <th style="width: 50%;">{% trans 'Name' %}</th> + <th style="width: 40%;">{% trans 'Name' %}</th> + <th style="width: 10%;">{% trans 'Progress' %}</th> <th style="width: 20%;">{% trans 'Created' %}</th> <th style="width: 20%;">{% trans 'Last changed' %}</th> <th style="width: 10%;"></th> @@ -76,6 +77,9 @@ <h1>{% blocktrans trimmed with site=request.site %}All projects on {{ site }}{% <strong>{{ project.title }}</strong> </a> </td> + <td> + {% project_progress project %} + </td> <td> {{ project.created }} </td> diff --git a/rdmo/projects/templatetags/projects_tags.py b/rdmo/projects/templatetags/projects_tags.py index 390a79e4f8..7b64ab9b44 100644 --- a/rdmo/projects/templatetags/projects_tags.py +++ b/rdmo/projects/templatetags/projects_tags.py @@ -1,6 +1,7 @@ from django import template from django.template.defaultfilters import stringfilter from django.utils.safestring import mark_safe +from django.utils.translation import gettext as _ from ..models import Membership @@ -11,13 +12,45 @@ def projects_indent(level): string = '' if level > 0: - for _ in range(level - 1): + for i in range(level - 1): string += '  ' string += '• ' return mark_safe('<span class="projects-indent">' + string + '</span>') +@register.simple_tag() +def project_progress(project): + if project.progress_count is None or project.progress_total is None: + return '' + + return _('%(count)s of %(total)s') % { + 'count': project.progress_count, + 'total': project.progress_total + } + +@register.simple_tag() +def project_progress_ratio(project): + if project.progress_count is None or project.progress_total is None: + return '' + + try: + ratio = project.progress_count / project.progress_total + except ZeroDivisionError: + ratio = 0 + + return f'{ratio:.0%}' + + +@register.simple_tag() +def project_progress_text(project): + progress = project_progress(project) + if progress: + return _('(%(progress)s progress)') % {'progress': progress} + else: + return '' + + @register.filter() @stringfilter def projects_role(role): diff --git a/rdmo/projects/tests/test_navigation.py b/rdmo/projects/tests/test_navigation.py new file mode 100644 index 0000000000..7f7a9c90b3 --- /dev/null +++ b/rdmo/projects/tests/test_navigation.py @@ -0,0 +1,67 @@ +import pytest + +from rdmo.projects.models import Project +from rdmo.projects.progress import compute_navigation + +sections = ( + 'http://example.com/terms/questions/catalog/individual', + 'http://example.com/terms/questions/catalog/collections', + 'http://example.com/terms/questions/catalog/set', + 'http://example.com/terms/questions/catalog/conditions', + 'http://example.com/terms/questions/catalog/blocks' +) + +# (count, total, show) for each page or section (as default fallback) +result_map = { + 'http://example.com/terms/questions/catalog/individual': (1, 1, True), + 'http://example.com/terms/questions/catalog/individual/autocomplete': (0, 1, True), + 'http://example.com/terms/questions/catalog/collections': (1, 1, True), + 'http://example.com/terms/questions/catalog/collections/autocomplete': (0, 1, True), + 'http://example.com/terms/questions/catalog/set/individual-single': (8, 9, True), + 'http://example.com/terms/questions/catalog/set/individual-collection': (9, 10, True), + 'http://example.com/terms/questions/catalog/set/collection-single': (14, 18, True), + 'http://example.com/terms/questions/catalog/set/collection-collection': (16, 20, True), + 'http://example.com/terms/questions/catalog/conditions/input': (2, 2, True), + 'http://example.com/terms/questions/catalog/conditions/text_contains': (1, 1, True), + 'http://example.com/terms/questions/catalog/conditions/text_empty': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/text_equal': (1, 1, True), + 'http://example.com/terms/questions/catalog/conditions/text_greater_than': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/text_greater_than_equal': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/text_lesser_than': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/text_lesser_than_equal': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/text_not_empty': (1, 1, True), + 'http://example.com/terms/questions/catalog/conditions/text_not_equal': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/option_empty': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/option_equal': (1, 1, True), + 'http://example.com/terms/questions/catalog/conditions/option_not_empty': (1, 1, True), + 'http://example.com/terms/questions/catalog/conditions/option_not_equal': (1, 1, False), + 'http://example.com/terms/questions/catalog/conditions/set': (0, 2, True), + 'http://example.com/terms/questions/catalog/conditions/set_set': (0, 2, True), + 'http://example.com/terms/questions/catalog/conditions/optionset': (0, 2, True), + 'http://example.com/terms/questions/catalog/conditions/text_set': (0, 2, True), + 'http://example.com/terms/questions/catalog/blocks/set': (9, 18, True), +} + + +@pytest.mark.parametrize('section_uri', sections) +def test_compute_navigation(db, section_uri): + project = Project.objects.get(id=1) + project.catalog.prefetch_elements() + + section = project.catalog.sections.get(uri=section_uri) + + navigation = compute_navigation(section, project) + assert [item['id'] for item in navigation] == [element.id for element in project.catalog.elements] + + for section in navigation: + if 'pages' in section: + for page in section['pages']: + if page['uri'] in result_map: + count, total, show = result_map[page['uri']] + elif section['uri'] in result_map: + count, total, show = result_map[section['uri']] + else: + raise AssertionError('{uri} not in result_map'.format(**page)) + assert page['count'] == count, page['uri'] + assert page['total'] == total, page['uri'] + assert page['show'] == show, page['uri'] diff --git a/rdmo/projects/tests/test_progress.py b/rdmo/projects/tests/test_progress.py new file mode 100644 index 0000000000..58782ca807 --- /dev/null +++ b/rdmo/projects/tests/test_progress.py @@ -0,0 +1,21 @@ +import pytest + +from rdmo.projects.models import Project +from rdmo.projects.progress import compute_progress + +projects = [1, 11] + +results_map = { + 1: (58, 87), + 11: (0, 29) +} + + +@pytest.mark.parametrize('project_id', projects) +def test_compute_progress(db, project_id): + project = Project.objects.get(id=project_id) + project.catalog.prefetch_elements() + + progress = compute_progress(project) + + assert progress == results_map[project_id] diff --git a/rdmo/projects/tests/test_validator_conflict.py b/rdmo/projects/tests/test_validator_conflict.py new file mode 100644 index 0000000000..e48191c6ce --- /dev/null +++ b/rdmo/projects/tests/test_validator_conflict.py @@ -0,0 +1,123 @@ +from datetime import timedelta + +import pytest + +from rest_framework.exceptions import ValidationError as RestFameworkValidationError + +from ..models import Project, Value +from ..serializers.v1 import ValueSerializer +from ..validators import ValueConflictValidator + +project_id = 1 +attribute_path = attribute__path='individual/single/text' + + +def test_serializer_create(db): + class MockedView: + project = Project.objects.get(id=project_id) + + value = Value.objects.get(project_id=project_id, snapshot=None, attribute__path=attribute_path) + + validator = ValueConflictValidator() + serializer = ValueSerializer() + serializer.context['view'] = MockedView() + + validator({ + 'attribute': value.attribute, + 'set_prefix': value.set_prefix, + 'set_index': value.set_index, + 'collection_index': value.collection_index + 1, + }, serializer) + + +def test_serializer_create_error(db): + class MockedView: + project = Project.objects.get(id=project_id) + + value = Value.objects.get(project_id=project_id, snapshot=None, attribute__path=attribute_path) + + validator = ValueConflictValidator() + serializer = ValueSerializer() + serializer.context['view'] = MockedView() + + with pytest.raises(RestFameworkValidationError): + validator({ + 'attribute': value.attribute, + 'set_prefix': value.set_prefix, + 'set_index': value.set_index, + 'collection_index': value.collection_index, + }, serializer) + + +def test_serializer_update(db): + value = Value.objects.get(project_id=project_id, snapshot=None, attribute__path=attribute_path) + + class MockedRequest: + data = { + 'updated': value.updated .isoformat() + } + + class MockedView: + request = MockedRequest() + project = Project.objects.get(id=project_id) + + validator = ValueConflictValidator() + serializer = ValueSerializer() + serializer.instance = value + serializer.context['view'] = MockedView() + + validator({ + 'attribute': value.attribute, + 'set_prefix': value.set_prefix, + 'set_index': value.set_index, + 'collection_index': value.collection_index, + }, serializer) + + +def test_serializer_update_error(db): + value = Value.objects.get(project_id=project_id, snapshot=None, attribute__path=attribute_path) + + class MockedRequest: + data = { + 'updated': (value.updated - timedelta(seconds=1)).isoformat() + } + + class MockedView: + request = MockedRequest() + project = Project.objects.get(id=project_id) + + validator = ValueConflictValidator() + serializer = ValueSerializer() + serializer.instance = value + serializer.context['view'] = MockedView() + + with pytest.raises(RestFameworkValidationError): + validator({ + 'attribute': value.attribute, + 'set_prefix': value.set_prefix, + 'set_index': value.set_index, + 'collection_index': value.collection_index, + }, serializer) + + +def test_serializer_update_missing_updated(db): + value = Value.objects.get(project_id=project_id, snapshot=None, attribute__path=attribute_path) + + class MockedRequest: + data = {} + + class MockedView: + request = MockedRequest() + project = Project.objects.get(id=project_id) + + validator = ValueConflictValidator() + serializer = ValueSerializer() + serializer.instance = value + serializer.context['view'] = MockedView() + + validator({ + 'attribute': value.attribute, + 'set_prefix': value.set_prefix, + 'set_index': value.set_index, + 'collection_index': value.collection_index, + }, serializer) diff --git a/rdmo/projects/tests/test_validator_quota.py b/rdmo/projects/tests/test_validator_quota.py new file mode 100644 index 0000000000..4eee080e26 --- /dev/null +++ b/rdmo/projects/tests/test_validator_quota.py @@ -0,0 +1,86 @@ +import pytest + +from rest_framework.exceptions import ValidationError as RestFameworkValidationError + +from ..serializers.v1 import ValueSerializer +from ..validators import ValueQuotaValidator + +project_id = 1 +attribute_path = attribute__path='individual/single/text' + + +def test_serializer_create_file(db, settings): + class MockedProject: + file_size = 1 + + class MockedView: + action = 'create' + project = MockedProject() + + settings.PROJECT_FILE_QUOTA = '1b' + + validator = ValueQuotaValidator() + serializer = ValueSerializer() + serializer.context['view'] = MockedView() + + validator({ + 'value_type': 'file' + }, serializer) + + +def test_serializer_create_file_error(db, settings): + class MockedProject: + file_size = 1 + + class MockedView: + action = 'create' + project = MockedProject() + + settings.PROJECT_FILE_QUOTA = '0' + + validator = ValueQuotaValidator() + serializer = ValueSerializer() + serializer.context['view'] = MockedView() + + with pytest.raises(RestFameworkValidationError): + validator({ + 'value_type': 'file' + }, serializer) + + +def test_serializer_create_text(db, settings): + class MockedProject: + file_size = 1 + + class MockedView: + action = 'create' + project = MockedProject() + + settings.PROJECT_FILE_QUOTA = '0' + + validator = ValueQuotaValidator() + serializer = ValueSerializer() + serializer.context['view'] = MockedView() + + validator({ + 'value_type': 'text' + }, serializer) + + +def test_serializer_update(db, settings): + class MockedProject: + file_size = 1 + + class MockedView: + action = 'update' + project = MockedProject() + + settings.PROJECT_FILE_QUOTA = '0' + + validator = ValueQuotaValidator() + serializer = ValueSerializer() + serializer.context['view'] = MockedView() + + validator({ + 'value_type': 'file' + }, serializer) diff --git a/rdmo/projects/tests/test_viewset_project.py b/rdmo/projects/tests/test_viewset_project.py index c5d08d0c40..3e523882f4 100644 --- a/rdmo/projects/tests/test_viewset_project.py +++ b/rdmo/projects/tests/test_viewset_project.py @@ -41,18 +41,17 @@ 'list': 'v1-projects:project-list', 'detail': 'v1-projects:project-detail', 'overview': 'v1-projects:project-overview', + 'navigation': 'v1-projects:project-navigation', 'resolve': 'v1-projects:project-resolve', - 'progress': 'v1-projects:project-progress' } projects = [1, 2, 3, 4, 5] conditions = [1] -project_values = 37 -project_total = 54 catalog_id = 1 catalog_id_not_available = 2 +section_id = 1 @pytest.mark.parametrize('username,password', users) def test_list(db, client, username, password): @@ -241,15 +240,15 @@ def test_overview(db, client, username, password, project_id, condition_id): @pytest.mark.parametrize('username,password', users) @pytest.mark.parametrize('project_id', projects) @pytest.mark.parametrize('condition_id', conditions) -def test_resolve(db, client, username, password, project_id, condition_id): +def test_navigation(db, client, username, password, project_id, condition_id): client.login(username=username, password=password) - url = reverse(urlnames['resolve'], args=[project_id]) + f'?condition={condition_id}' + url = reverse(urlnames['navigation'], args=[project_id, section_id]) response = client.get(url) if project_id in view_project_permission_map.get(username, []): assert response.status_code == 200 - assert isinstance(response.json(), dict) + assert isinstance(response.json(), list) else: if password: assert response.status_code == 404 @@ -259,22 +258,16 @@ def test_resolve(db, client, username, password, project_id, condition_id): @pytest.mark.parametrize('username,password', users) @pytest.mark.parametrize('project_id', projects) -def test_progress(db, client, username, password, project_id): +@pytest.mark.parametrize('condition_id', conditions) +def test_resolve(db, client, username, password, project_id, condition_id): client.login(username=username, password=password) - url = reverse(urlnames['progress'], args=[project_id]) + url = reverse(urlnames['resolve'], args=[project_id]) + f'?condition={condition_id}' response = client.get(url) if project_id in view_project_permission_map.get(username, []): assert response.status_code == 200 assert isinstance(response.json(), dict) - - if project_id == 1: - assert response.json().get('values') == project_values - else: - assert response.json().get('values') == 1 - - assert response.json().get('total') == project_total else: if password: assert response.status_code == 404 diff --git a/rdmo/projects/tests/test_viewset_project_progress.py b/rdmo/projects/tests/test_viewset_project_progress.py new file mode 100644 index 0000000000..6c39a52798 --- /dev/null +++ b/rdmo/projects/tests/test_viewset_project_progress.py @@ -0,0 +1,93 @@ +import pytest + +from django.urls import reverse + +from ..models import Project + +users = ( + ('owner', 'owner'), + ('manager', 'manager'), + ('author', 'author'), + ('guest', 'guest'), + ('api', 'api'), + ('user', 'user'), + ('site', 'site'), + ('anonymous', None), +) + +view_progress_permission_map = { + 'owner': [1, 2, 3, 4, 5, 10], + 'manager': [1, 3, 5, 7], + 'author': [1, 3, 5, 8], + 'guest': [1, 3, 5, 9], + 'api': [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], + 'site': [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] +} + +change_progress_permission_map = { + 'owner': [1, 2, 3, 4, 5, 10], + 'manager': [1, 3, 5, 7], + 'author': [1, 3, 5, 8], + 'api': [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], + 'site': [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] +} + +urlnames = { + 'progress': 'v1-projects:project-progress' +} + +projects = [1, 2, 3, 4, 5] + +@pytest.mark.parametrize('username,password', users) +@pytest.mark.parametrize('project_id', projects) +def test_progress_get(db, client, username, password, project_id): + client.login(username=username, password=password) + + url = reverse(urlnames['progress'], args=[project_id]) + response = client.get(url) + + if project_id in view_progress_permission_map.get(username, []): + assert response.status_code == 200 + assert isinstance(response.json(), dict) + + project = Project.objects.get(id=project_id) + assert response.json()['count'] == project.progress_count + assert response.json()['total'] == project.progress_total + + else: + if password: + assert response.status_code == 404 + else: + assert response.status_code == 401 + + +@pytest.mark.parametrize('username,password', users) +@pytest.mark.parametrize('project_id', projects) +def test_progress_post(db, client, username, password, project_id): + client.login(username=username, password=password) + + if project_id in change_progress_permission_map.get(username, []): + # set project count and value to a different value + project = Project.objects.get(id=project_id) + project.progress_count = 0 + project.progress_total = 0 + project.save() + + url = reverse(urlnames['progress'], args=[project_id]) + response = client.post(url) + + if project_id in change_progress_permission_map.get(username, []): + assert response.status_code == 200 + assert isinstance(response.json(), dict) + + project.refresh_from_db() + assert response.json()['count'] > 0 + assert response.json()['total'] > 0 + + else: + if project_id in view_progress_permission_map.get(username, []): + assert response.status_code == 403 + elif password: + assert response.status_code == 404 + else: + assert response.status_code == 401 diff --git a/rdmo/projects/utils.py b/rdmo/projects/utils.py index 94b57e0793..d20e641b99 100644 --- a/rdmo/projects/utils.py +++ b/rdmo/projects/utils.py @@ -101,10 +101,7 @@ def save_import_snapshot_values(project, snapshots, checked): for value in snapshot.snapshot_values: if value.attribute: - value_key = '{value.attribute.uri}[{snapshot_index}][{value.set_prefix}][{value.set_index}][{value.collection_index}]'.format( # noqa: E501 - value=value, - snapshot_index=snapshot.snapshot_index - ) + value_key = f"{value.attribute.uri}[{snapshot.snapshot_index}][{value.set_prefix}][{value.set_index}][{value.collection_index}]" # noqa: E501 if value_key in checked: # assert that this is a new value diff --git a/rdmo/projects/validators.py b/rdmo/projects/validators.py index 3c60a66a32..65f2deebd5 100644 --- a/rdmo/projects/validators.py +++ b/rdmo/projects/validators.py @@ -1,4 +1,6 @@ from django.conf import settings +from django.core.exceptions import ObjectDoesNotExist +from django.utils.dateparse import parse_datetime from django.utils.translation import gettext_lazy as _ from rest_framework import serializers @@ -7,18 +9,42 @@ from rdmo.core.utils import human2bytes -class ValueValidator: +class ValueConflictValidator: requires_context = True def __call__(self, data, serializer): - if data.get('value_type') == VALUE_TYPE_FILE: + if serializer.instance: + # for an update, check if the value was updated in the meantime + updated = serializer.context['view'].request.data.get('updated') + if updated is not None and parse_datetime(updated) < serializer.instance.updated: + raise serializers.ValidationError({ + 'conflict': [_('A newer version of this value was found.')] + }) + else: + # for a new value, check if there is already a value with the same attribute and indexes try: - serializer.context['view'].get_object() - except AssertionError as e: - project = serializer.context['view'].project - - if project.file_size > human2bytes(settings.PROJECT_FILE_QUOTA): - raise serializers.ValidationError({ - 'value': [_('You reached the file quota for this project.')] - }) from e + serializer.context['view'].project.values.filter(snapshot=None).get( + attribute=data.get('attribute'), + set_prefix=data.get('set_prefix'), + set_index=data.get('set_index'), + collection_index=data.get('collection_index') + ) + raise serializers.ValidationError({ + 'conflict': [_('An existing value for this attribute/set_prefix/set_index/collection_index' + ' was found.')] + }) + except ObjectDoesNotExist: + pass + +class ValueQuotaValidator: + + requires_context = True + + def __call__(self, data, serializer): + if serializer.context['view'].action == 'create' and data.get('value_type') == VALUE_TYPE_FILE: + project = serializer.context['view'].project + if project.file_size > human2bytes(settings.PROJECT_FILE_QUOTA): + raise serializers.ValidationError({ + 'quota': [_('The file quota for this project has been reached.')] + }) diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index 257c510dc1..0d3ab9e29e 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -1,6 +1,6 @@ from django.conf import settings from django.contrib.sites.shortcuts import get_current_site -from django.db.models import prefetch_related_objects +from django.core.exceptions import ObjectDoesNotExist from django.http import Http404, HttpResponseRedirect from django.utils.translation import gettext_lazy as _ @@ -8,7 +8,6 @@ from rest_framework.decorators import action from rest_framework.exceptions import NotFound from rest_framework.mixins import CreateModelMixin, ListModelMixin, RetrieveModelMixin, UpdateModelMixin -from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.reverse import reverse from rest_framework.viewsets import GenericViewSet, ModelViewSet, ReadOnlyModelViewSet @@ -26,7 +25,13 @@ from .filters import SnapshotFilterBackend, ValueFilterBackend from .models import Continuation, Integration, Invite, Issue, Membership, Project, Snapshot, Value -from .permissions import HasProjectPagePermission, HasProjectPermission, HasProjectsPermission +from .permissions import ( + HasProjectPagePermission, + HasProjectPermission, + HasProjectProgressPermission, + HasProjectsPermission, +) +from .progress import compute_navigation, compute_progress from .serializers.v1 import ( IntegrationSerializer, InviteSerializer, @@ -65,17 +70,27 @@ class ProjectViewSet(ModelViewSet): def get_queryset(self): return Project.objects.filter_user(self.request.user).select_related('catalog') - @action(detail=True, permission_classes=(IsAuthenticated, )) + @action(detail=True, permission_classes=(HasModelPermission | HasProjectPermission, )) def overview(self, request, pk=None): project = self.get_object() - - # prefetch only the pages (and their conditions) - prefetch_related_objects([project.catalog], - 'catalog_sections__section__section_pages__page__conditions') - serializer = ProjectOverviewSerializer(project, context={'request': request}) return Response(serializer.data) + @action(detail=True, url_path=r'navigation/(?P<section_id>\d+)', + permission_classes=(HasModelPermission | HasProjectPermission, )) + def navigation(self, request, pk=None, section_id=None): + project = self.get_object() + + try: + section = project.catalog.sections.get(pk=section_id) + except ObjectDoesNotExist as e: + raise NotFound() from e + + project.catalog.prefetch_elements() + + navigation = compute_navigation(section, project) + return Response(navigation) + @action(detail=True, permission_classes=(HasModelPermission | HasProjectPermission, )) def resolve(self, request, pk=None): snapshot_id = request.GET.get('snapshot') @@ -158,11 +173,27 @@ def options(self, request, pk=None): # if it didn't work return 404 raise NotFound() - @action(detail=True, permission_classes=(IsAuthenticated, )) + @action(detail=True, methods=['get', 'post'], + permission_classes=(HasModelPermission | HasProjectProgressPermission, )) def progress(self, request, pk=None): project = self.get_object() - project.catalog.prefetch_elements() - return Response(project.progress) + + if request.method == 'POST' or project.progress_count is None or project.progress_total is None: + # compute the progress and store + project.catalog.prefetch_elements() + project.progress_count, project.progress_total = compute_progress(project) + project.save() + + try: + ratio = project.progress_count / project.progress_total + except ZeroDivisionError: + ratio = 0 + + return Response({ + 'count': project.progress_count, + 'total': project.progress_total, + 'ratio': ratio + }) def perform_create(self, serializer): project = serializer.save(site=get_current_site(self.request)) diff --git a/rdmo/questions/admin.py b/rdmo/questions/admin.py index 9b52d07f74..1ff106b968 100644 --- a/rdmo/questions/admin.py +++ b/rdmo/questions/admin.py @@ -96,6 +96,7 @@ class CatalogSectionInline(admin.TabularInline): extra = 0 +@admin.register(Catalog) class CatalogAdmin(admin.ModelAdmin): form = CatalogAdminForm inlines = (CatalogSectionInline, ) @@ -119,6 +120,7 @@ class SectionPageInline(admin.TabularInline): extra = 0 +@admin.register(Section) class SectionAdmin(admin.ModelAdmin): form = SectionAdminForm inlines = (SectionPageInline, ) @@ -140,6 +142,7 @@ class PageQuestionInline(admin.TabularInline): extra = 0 +@admin.register(Page) class PageAdmin(admin.ModelAdmin): form = PageAdminForm inlines = (PageQuestionSetInline, PageQuestionInline) @@ -162,6 +165,7 @@ class QuestionSetQuestionInline(admin.TabularInline): extra = 0 +@admin.register(QuestionSet) class QuestionSetAdmin(admin.ModelAdmin): form = QuestionSetAdminForm inlines = (QuestionSetQuestionSetInline, QuestionSetQuestionInline) @@ -173,6 +177,7 @@ class QuestionSetAdmin(admin.ModelAdmin): filter_horizontal = ('editors', 'conditions') +@admin.register(Question) class QuestionAdmin(admin.ModelAdmin): form = QuestionAdminForm @@ -182,10 +187,3 @@ class QuestionAdmin(admin.ModelAdmin): list_filter = ('pages__sections__catalogs', 'pages__sections', 'pages', 'is_collection', 'widget_type', 'value_type') filter_horizontal = ('editors', 'optionsets', 'conditions') - - -admin.site.register(Catalog, CatalogAdmin) -admin.site.register(Section, SectionAdmin) -admin.site.register(Page, PageAdmin) -admin.site.register(QuestionSet, QuestionSetAdmin) -admin.site.register(Question, QuestionAdmin) diff --git a/rdmo/questions/migrations/0091_alter_questionset_options.py b/rdmo/questions/migrations/0091_alter_questionset_options.py new file mode 100644 index 0000000000..037cce001f --- /dev/null +++ b/rdmo/questions/migrations/0091_alter_questionset_options.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.7 on 2023-11-08 05:40 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("questions", "0090_add_editors"), + ] + + operations = [ + migrations.AlterModelOptions( + name="questionset", + options={ + "ordering": ("uri",), + "verbose_name": "Question set", + "verbose_name_plural": "Question sets", + }, + ), + ] diff --git a/rdmo/questions/models/questionset.py b/rdmo/questions/models/questionset.py index 8e3f0026cb..64aaaebddc 100644 --- a/rdmo/questions/models/questionset.py +++ b/rdmo/questions/models/questionset.py @@ -185,7 +185,7 @@ class QuestionSet(Model, TranslationMixin): class Meta: ordering = ('uri', ) verbose_name = _('Question set') - verbose_name_plural = _('Question set') + verbose_name_plural = _('Question sets') def __str__(self): return self.uri diff --git a/rdmo/services/providers.py b/rdmo/services/providers.py index 41b6530224..ea06629425 100644 --- a/rdmo/services/providers.py +++ b/rdmo/services/providers.py @@ -25,14 +25,14 @@ def get(self, request, url): response = requests.get(url, headers=self.get_authorization_headers(access_token)) if response.status_code == 401: - logger.warn('get forbidden: %s (%s)', response.content, response.status_code) + logger.warning('get forbidden: %s (%s)', response.content, response.status_code) else: try: response.raise_for_status() return self.get_success(request, response) except requests.HTTPError: - logger.warn('get error: %s (%s)', response.content, response.status_code) + logger.warning('get error: %s (%s)', response.content, response.status_code) return render(request, 'core/error.html', { 'title': _('OAuth error'), @@ -53,14 +53,14 @@ def post(self, request, url, data): response = requests.post(url, json=data, headers=self.get_authorization_headers(access_token)) if response.status_code == 401: - logger.warn('post forbidden: %s (%s)', response.content, response.status_code) + logger.warning('post forbidden: %s (%s)', response.content, response.status_code) else: try: response.raise_for_status() return self.post_success(request, response) except requests.HTTPError: - logger.warn('post error: %s (%s)', response.content, response.status_code) + logger.warning('post error: %s (%s)', response.content, response.status_code) return render(request, 'core/error.html', { 'title': _('OAuth error'), diff --git a/rdmo/tasks/admin.py b/rdmo/tasks/admin.py index bb2c85c429..6d1acf36cc 100644 --- a/rdmo/tasks/admin.py +++ b/rdmo/tasks/admin.py @@ -19,6 +19,7 @@ def clean(self): TaskLockedValidator(self.instance)(self.cleaned_data) +@admin.register(Task) class TaskAdmin(admin.ModelAdmin): form = TaskAdminForm @@ -27,6 +28,3 @@ class TaskAdmin(admin.ModelAdmin): readonly_fields = ('uri', ) list_filter = ('available', ) filter_horizontal = ('catalogs', 'sites', 'editors', 'groups', 'conditions') - - -admin.site.register(Task, TaskAdmin) diff --git a/rdmo/views/admin.py b/rdmo/views/admin.py index 1cfebec9af..9fedfe6d53 100644 --- a/rdmo/views/admin.py +++ b/rdmo/views/admin.py @@ -19,6 +19,7 @@ def clean(self): ViewLockedValidator(self.instance)(self.cleaned_data) +@admin.register(View) class ViewAdmin(admin.ModelAdmin): form = ViewAdminForm @@ -27,6 +28,3 @@ class ViewAdmin(admin.ModelAdmin): readonly_fields = ('uri', ) list_filter = ('available', ) filter_horizontal = ('catalogs', 'sites', 'editors', 'groups') - - -admin.site.register(View, ViewAdmin) diff --git a/testing/config/settings/base.py b/testing/config/settings/base.py index 270ec4ea20..e39edf6c80 100644 --- a/testing/config/settings/base.py +++ b/testing/config/settings/base.py @@ -2,7 +2,7 @@ from django.utils.translation import gettext_lazy as _ -DEBUG = False +DEBUG = os.getenv("DJANGO_DEBUG", False) == "True" TEMPLATE_DEBUG = False DEBUG_LOGGING = False