From b73ffbcef4b43875e91a234f0c562c6837f1c357 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Thu, 7 Nov 2024 17:38:42 +0100 Subject: [PATCH 01/42] Add import function to create set modal on pages --- .../js/interview/actions/interviewActions.js | 4 +- .../assets/js/interview/api/ValueApi.js | 14 ++- .../components/main/page/PageHead.js | 32 ++++-- .../components/main/page/PageHeadFormModal.js | 106 ++++++++++++------ rdmo/projects/managers.py | 4 +- rdmo/projects/viewsets.py | 25 +++-- 6 files changed, 118 insertions(+), 67 deletions(-) diff --git a/rdmo/projects/assets/js/interview/actions/interviewActions.js b/rdmo/projects/assets/js/interview/actions/interviewActions.js index 53ff451506..07b8849553 100644 --- a/rdmo/projects/assets/js/interview/actions/interviewActions.js +++ b/rdmo/projects/assets/js/interview/actions/interviewActions.js @@ -612,7 +612,7 @@ export function deleteSetError(errors) { } export function copySet(currentSet, currentSetValue, attrs) { - const pendingId = `copySet/${currentSet.set_prefix}/${currentSet.set_index}` + const pendingId = isNil(currentSet) ? 'copySet' : `copySet/${currentSet.set_prefix}/${currentSet.set_index}` return (dispatch, getState) => { dispatch(addToPending(pendingId)) @@ -667,7 +667,7 @@ export function copySet(currentSet, currentSetValue, attrs) { }) ) } else { - promise = ValueApi.copySet(projectId, currentSetValue, value) + promise = ValueApi.copySet(projectId, value, currentSetValue) } return promise.then((values) => { diff --git a/rdmo/projects/assets/js/interview/api/ValueApi.js b/rdmo/projects/assets/js/interview/api/ValueApi.js index b925e5acac..e2c4a3bd80 100644 --- a/rdmo/projects/assets/js/interview/api/ValueApi.js +++ b/rdmo/projects/assets/js/interview/api/ValueApi.js @@ -1,11 +1,15 @@ import BaseApi from 'rdmo/core/assets/js/api/BaseApi' import { encodeParams } from 'rdmo/core/assets/js/utils/api' -import isUndefined from 'lodash/isUndefined' +import { isNil, isUndefined } from 'lodash' class ValueApi extends BaseApi { static fetchValues(projectId, params) { - return this.get(`/api/v1/projects/projects/${projectId}/values/?${encodeParams(params)}`) + if (isNil(projectId)) { + return this.get(`/api/v1/projects/values/?${encodeParams(params)}`) + } else { + return this.get(`/api/v1/projects/projects/${projectId}/values/?${encodeParams(params)}`) + } } static storeValue(projectId, value) { @@ -29,8 +33,10 @@ class ValueApi extends BaseApi { } } - static copySet(projectId, currentSetValue, setValue) { - return this.post(`/api/v1/projects/projects/${projectId}/values/${currentSetValue.id}/set/`, setValue) + static copySet(projectId, setValue, copySetValue) { + return this.post(`/api/v1/projects/projects/${projectId}/values/set/`, { + ...setValue, copySetValue: copySetValue.id + }) } static deleteSet(projectId, setValue) { diff --git a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js index 719afecbd3..c2184bea61 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js @@ -35,13 +35,22 @@ const PageHead = ({ templates, page, sets, values, currentSet, openCreateModal() } - const handleCreateSet = (text) => { - createSet({ - attribute: page.attribute, - set_index: last(sets) ? last(sets).set_index + 1 : 0, - set_collection: page.is_collection, - text - }) + const handleCreateSet = (text, copySetValue) => { + if (isNil(copySetValue)) { + createSet({ + attribute: page.attribute, + set_index: last(sets) ? last(sets).set_index + 1 : 0, + set_collection: page.is_collection, + text + }) + } else { + copySet(currentSet, copySetValue, { + attribute: page.attribute, + set_index: last(sets) ? last(sets).set_index + 1 : 0, + set_collection: page.is_collection, + text + }) + } closeCreateModal() } @@ -115,7 +124,8 @@ const PageHead = ({ templates, page, sets, values, currentSet, submitLabel={gettext('Create')} submitColor="success" show={showCreateModal} - initial={isNil(page.attribute) ? null : ''} + attribute={page.attribute} + initial={{ text: '', copySetValue: '' }} onClose={closeCreateModal} onSubmit={handleCreateSet} /> @@ -124,7 +134,8 @@ const PageHead = ({ templates, page, sets, values, currentSet, submitLabel={gettext('Copy')} submitColor="info" show={showCopyModal} - initial={isNil(page.attribute) ? null : ''} + attribute={page.attribute} + initial={{ text: '' }} onClose={closeCopyModal} onSubmit={handleCopySet} /> @@ -135,7 +146,8 @@ const PageHead = ({ templates, page, sets, values, currentSet, submitLabel={gettext('Update')} submitColor="primary" show={showUpdateModal} - initial={currentSetValue.text} + attribute={page.attribute} + initial={{ text: currentSetValue.text }} onClose={closeUpdateModal} onSubmit={handleUpdateSet} /> diff --git a/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js b/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js index fee6feda62..568b4a2e84 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js @@ -1,72 +1,105 @@ import React, { useState, useRef, useEffect } from 'react' import PropTypes from 'prop-types' import classNames from 'classnames' -import { isEmpty, isNil } from 'lodash' +import { isEmpty, isNil, isUndefined } from 'lodash' + +import ValueApi from '../../../api/ValueApi' import Modal from 'rdmo/core/assets/js/components/Modal' +import Select from 'rdmo/core/assets/js/components/Select' + import useFocusEffect from '../../../hooks/useFocusEffect' -const PageHeadFormModal = ({ title, submitLabel, submitColor, show, initial, onClose, onSubmit }) => { +const PageHeadFormModal = ({ title, submitLabel, submitColor, show, attribute, initial, onClose, onSubmit }) => { const ref = useRef(null) - const [inputValue, setInputValue] = useState('') - const [hasError, setHasError] = useState(false) + + const [values, setValues] = useState(initial) + const [errors, setErrors] = useState([]) + const [options, setOptions] = useState([]) const handleSubmit = () => { - if (isEmpty(inputValue) && !isNil(initial)) { - setHasError(true) + if (isEmpty(values.text) && !isNil(attribute)) { + setErrors({ text: true }) } else { - onSubmit(inputValue) + onSubmit(values.text, values.copySetValue) } } - // update the inputValue useEffect(() => { - if (show) { - setInputValue(initial || '') + if (show && !isNil(attribute)) { + // update the form values + setValues(initial) + + // fetch the values for this attribute from all projects and snapshots + if (!isNil(initial)) { + ValueApi.fetchValues({ attribute }).then(response => setOptions(response.map(value => ({ + value: value, + label: value.text + })))) + } } }, [show]) // remove the hasError flag if an inputValue is entered useEffect(() => { - if (!isEmpty(inputValue)) { - setHasError(false) + if (!isEmpty(values.text)) { + setErrors({ text: false }) } - }, [inputValue]) + }, [values, values.text]) // focus when the modal is shown useFocusEffect(ref, show) return ( + onClose={onClose} onSubmit={handleSubmit} disableSubmit={errors}> { - isNil(initial) ? ( + isNil(attribute) ? (
{gettext('You can add a new tab using the create button.')}
) : ( -
- - setInputValue(event.target.value)} - onKeyPress={(event) => { - if (event.code === 'Enter') { - handleSubmit() - } - }} - /> - -

{gettext('Please give the tab a meaningful name.')}

-
+ <> +
+ + setValues({ ...values, text: event.target.value })} + onKeyPress={(event) => { + if (event.code === 'Enter') { + handleSubmit() + } + }} + /> + +

{gettext('Please give the tab a meaningful name.')}

+
+ + { + !isUndefined(values.copySetValue) && ( +
+ + setValues({ ...values, copySetValue: id })} + + gettext('No options found')} + loadingMessage={() => gettext('Loading ...')} + options={[]} value={values.copySetValue} - /> + onChange={(id) => setValues({ ...values, copySetValue: id })} + getOptionValue={(value) => value} + getOptionLabel={(value) => value.value_title} + formatOptionLabel={(value) => ( +
+ {gettext('Project')} {value.project_title} + { + value.snapshot && <> + → + {gettext('Snapshot')} {value.snapshot_title} + + } + → + {value.value_and_unit} +
+ )} + loadOptions={handleLoadOptions} + defaultOptions /> + +
+ +

{gettext('You can fill the tab with answers from a similar tab in this or another project.')}

diff --git a/rdmo/projects/filters.py b/rdmo/projects/filters.py index 1962d71d67..34283597bd 100644 --- a/rdmo/projects/filters.py +++ b/rdmo/projects/filters.py @@ -143,7 +143,9 @@ def filter_queryset(self, request, queryset, view): return queryset snapshot = request.GET.get('snapshot') - if snapshot: + if snapshot == 'all': + pass + elif snapshot: try: snapshot_pk = int(snapshot) except (ValueError, TypeError): diff --git a/rdmo/projects/serializers/v1/__init__.py b/rdmo/projects/serializers/v1/__init__.py index 26ea38402d..475c454c56 100644 --- a/rdmo/projects/serializers/v1/__init__.py +++ b/rdmo/projects/serializers/v1/__init__.py @@ -433,3 +433,21 @@ class Meta: 'unit', 'external_id' ) + + +class ValueSearchSerializer(serializers.ModelSerializer): + + project_title = serializers.CharField(source='project.title', required=False) + snapshot_title = serializers.CharField(source='snapshot.title', required=False) + + class Meta: + model = Value + fields = ( + 'id', + 'project', + 'project_title', + 'snapshot', + 'snapshot_title', + 'value', + 'value_and_unit' + ) diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index 5cb23f1efa..ca34fd6c73 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -9,6 +9,7 @@ from rest_framework import serializers, status from rest_framework.decorators import action from rest_framework.exceptions import NotFound +from rest_framework.filters import SearchFilter from rest_framework.mixins import CreateModelMixin, ListModelMixin, RetrieveModelMixin, UpdateModelMixin from rest_framework.pagination import PageNumberPagination from rest_framework.permissions import IsAuthenticated @@ -72,6 +73,7 @@ ProjectVisibilitySerializer, SnapshotSerializer, UserInviteSerializer, + ValueSearchSerializer, ValueSerializer, ) from .serializers.v1.overview import CatalogSerializer, ProjectOverviewSerializer @@ -777,7 +779,7 @@ class ValueViewSet(ReadOnlyModelViewSet): permission_classes = (HasModelPermission | HasProjectsPermission, ) serializer_class = ValueSerializer - filter_backends = (SnapshotFilterBackend, DjangoFilterBackend) + filter_backends = (SnapshotFilterBackend, DjangoFilterBackend, SearchFilter) filterset_fields = ( 'project', # snapshot is part of SnapshotFilterBackend @@ -789,9 +791,17 @@ class ValueViewSet(ReadOnlyModelViewSet): 'option__uri_path' ) + search_fields = ['text', 'project__title', 'snapshot__title'] + def get_queryset(self): return Value.objects.filter_user(self.request.user).select_related('attribute', 'option') + @action(detail=False, permission_classes=(HasModelPermission | HasProjectsPermission, )) + def search(self, request): + queryset = self.filter_queryset(self.get_queryset()).exclude(text='').select_related('project', 'snapshot')[:5] + serializer = ValueSearchSerializer(queryset, many=True) + return Response(serializer.data) + @action(detail=True, permission_classes=(HasModelPermission | HasProjectsPermission, )) def file(self, request, pk=None): value = self.get_object() From 3cffcaca8a74a16db3fed5b0f77e2aa7be1e1fc8 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 8 Nov 2024 16:48:34 +0100 Subject: [PATCH 03/42] Fix errors after rebase --- rdmo/projects/managers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index 7b4b7396ea..8231088b5c 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -168,6 +168,8 @@ def filter_set(self, set_value): for descendant in element.descendants } + # construct the set_prefix for descendants for this set + descendants_set_prefix = \ f'{set_value.set_prefix}|{set_value.set_index}' if set_value.set_prefix else str(set_value.set_index) # collect all values for this set and all descendants From 1c470dbe55e3bbc5d6d53985fe287394682fc5a9 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 8 Nov 2024 16:51:06 +0100 Subject: [PATCH 04/42] Add error handling to copy-set action and fix/add tests --- .../assets/js/interview/api/ValueApi.js | 2 +- .../tests/test_viewset_project_value.py | 150 +++++++++++++++++- rdmo/projects/viewsets.py | 16 +- 3 files changed, 159 insertions(+), 9 deletions(-) diff --git a/rdmo/projects/assets/js/interview/api/ValueApi.js b/rdmo/projects/assets/js/interview/api/ValueApi.js index 0e27742386..6e59895ab4 100644 --- a/rdmo/projects/assets/js/interview/api/ValueApi.js +++ b/rdmo/projects/assets/js/interview/api/ValueApi.js @@ -35,7 +35,7 @@ class ValueApi extends BaseApi { static copySet(projectId, setValue, copySetValue) { return this.post(`/api/v1/projects/projects/${projectId}/values/set/`, { - ...setValue, copySetValue: copySetValue.id + ...setValue, copy_set_value: copySetValue.id }) } diff --git a/rdmo/projects/tests/test_viewset_project_value.py b/rdmo/projects/tests/test_viewset_project_value.py index 3359527fb9..38d214a7c8 100644 --- a/rdmo/projects/tests/test_viewset_project_value.py +++ b/rdmo/projects/tests/test_viewset_project_value.py @@ -4,11 +4,12 @@ import pytest from django.conf import settings +from django.contrib.auth.models import User from django.urls import reverse from rdmo.core.constants import VALUE_TYPE_FILE, VALUE_TYPE_TEXT -from ..models import Value +from ..models import Membership, Value users = ( ('owner', 'owner'), @@ -42,7 +43,8 @@ urlnames = { 'list': 'v1-projects:project-value-list', 'detail': 'v1-projects:project-value-detail', - 'set': 'v1-projects:project-value-set', + 'copy-set': 'v1-projects:project-value-copy-set', + 'delete-set': 'v1-projects:project-value-delete-set', 'file': 'v1-projects:project-value-file' } @@ -57,6 +59,8 @@ ] values_internal = [456] +other_project_id = 11 + attribute_id = 1 option_id = 1 @@ -257,14 +261,14 @@ def test_copy_set(db, client, username, password, value_id, set_values_count): set_value = Value.objects.get(id=value_id) values_count = Value.objects.count() - url = reverse(urlnames['set'], args=[set_value.project_id, value_id]) + url = reverse(urlnames['copy-set'], args=[set_value.project_id]) data = { 'attribute': set_value.attribute.id, 'set_prefix': set_value.set_prefix, 'set_index': 2, 'text': 'new' } - response = client.post(url, data=json.dumps(data), content_type="application/json") + response = client.post(url, data=json.dumps(dict(**data, copy_set_value=value_id)), content_type="application/json") if set_value.project_id in copy_value_permission_map.get(username, []): assert response.status_code == 201 @@ -290,6 +294,142 @@ def test_copy_set(db, client, username, password, value_id, set_values_count): assert Value.objects.count() == values_count +def test_copy_set_from_other_project(db, client): + ''' + A set can be copied from a project where the user has permissions. + ''' + client.login(username='user', password='user') + + set_value_id, set_values_count = set_values[0] + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user to the project with the set value as well as the other project + Membership.objects.create(project_id=set_value.project.id, user=User.objects.get(username='user'), role='author') + Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(dict(**data, copy_set_value=set_value_id)), + content_type="application/json") + assert response.status_code == 201 + assert Value.objects.get( + project=other_project_id, + snapshot=None, + **data + ) + assert Value.objects.count() == values_count + set_values_count + 1 # one is for set/id + + +def test_copy_set_from_other_project_forbidden(db, client): + ''' + A set cannot be copied from a project where the user has no permissions. + ''' + client.login(username='user', password='user') + + set_value_id, set_values_count = set_values[0] + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user only to the other project + Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(dict(**data, copy_set_value=set_value_id)), + content_type="application/json") + assert response.status_code == 404 + assert Value.objects.count() == values_count + + +def test_copy_set_from_other_project_not_found(db, client): + ''' + A set cannot be copied when the set values does not exist. + ''' + client.login(username='user', password='user') + + set_value_id, set_values_count = set_values[0] + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user only to the other project + Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(dict(**data, copy_set_value=10000)), + content_type="application/json") + assert response.status_code == 404 + assert Value.objects.count() == values_count + + +def test_copy_set_from_other_project_invalid(db, client): + ''' + A set cannot be copied when the set values does not exist. + ''' + client.login(username='user', password='user') + + set_value_id, set_values_count = set_values[0] + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user only to the other project + Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(dict(**data, copy_set_value='wrong')), + content_type="application/json") + assert response.status_code == 404 + assert Value.objects.count() == values_count + + +def test_copy_set_from_other_project_missing(db, client): + ''' + A set cannot be copied when the set values does not exist. + ''' + client.login(username='user', password='user') + + set_value_id, set_values_count = set_values[0] + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user only to the other project + Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(data), + content_type="application/json") + assert response.status_code == 400 + assert Value.objects.count() == values_count + + @pytest.mark.parametrize('username,password', users) @pytest.mark.parametrize('project_id', projects) @pytest.mark.parametrize('value_id, set_values_count', set_values) @@ -298,7 +438,7 @@ def test_delete_set(db, client, username, password, project_id, value_id, set_va value_exists = Value.objects.filter(project_id=project_id, snapshot=None, id=value_id).exists() values_count = Value.objects.count() - url = reverse(urlnames['set'], args=[project_id, value_id]) + url = reverse(urlnames['delete-set'], args=[project_id, value_id]) response = client.delete(url) if value_exists and project_id in delete_value_permission_map.get(username, []): diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index ca34fd6c73..e4e0ee1a7f 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -8,7 +8,7 @@ from rest_framework import serializers, status from rest_framework.decorators import action -from rest_framework.exceptions import NotFound +from rest_framework.exceptions import NotFound, ValidationError from rest_framework.filters import SearchFilter from rest_framework.mixins import CreateModelMixin, ListModelMixin, RetrieveModelMixin, UpdateModelMixin from rest_framework.pagination import PageNumberPagination @@ -538,10 +538,20 @@ def copy_set(self, request, parent_lookup_project, pk=None): # for this value and the same set_prefix and set_index # obtain the id of the value we want to copy - copy_value_id = request.data.pop('copySetValue') + try: + copy_value_id = int(request.data.pop('copy_set_value')) + except KeyError as e: + raise ValidationError({ + 'copy_set_value': [_('This field may not be blank.')] + }) from e + except ValueError as e: + raise NotFound from e # look for this value in the database, using the users permissions - copy_value = Value.objects.filter_user(self.request.user).get(id=copy_value_id) + try: + copy_value = Value.objects.filter_user(self.request.user).get(id=copy_value_id) + except Value.DoesNotExist as e: + raise NotFound from e # collect all values for this set and all descendants copy_values = Value.objects.filter_user(self.request.user).filter_set(copy_value) From 485f9175fc79386c94422c7927eb6b4cf3165807 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 8 Nov 2024 17:17:48 +0100 Subject: [PATCH 05/42] Improve PageHeadFormModal --- .../components/main/page/PageHeadFormModal.js | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js b/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js index 0ddc256410..f2e7586608 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js @@ -19,8 +19,8 @@ const PageHeadFormModal = ({ title, submitLabel, submitColor, show, attribute, i const [values, setValues] = useState(initial) const [errors, setErrors] = useState([]) - const handleLoadOptions = useDebouncedCallback((search, callback) => { - ValueApi.searchValues({ attribute, search, snapshot: values.snapshot ? 'all' : '' }).then(response => { + const handleLoadOptions = useDebouncedCallback((search, snapshot, callback) => { + ValueApi.searchValues({ attribute, search, snapshot }).then(response => { callback(response) }) }, 500) @@ -91,8 +91,10 @@ const PageHeadFormModal = ({ title, submitLabel, submitColor, show, attribute, i gettext('No options found')} + placeholder={gettext('Search for project, snapshot or answer text ...')} + noOptionsMessage={() => gettext( + 'No answers match your search.' + )} loadingMessage={() => gettext('Loading ...')} options={[]} value={values.copySetValue} @@ -112,8 +114,10 @@ const PageHeadFormModal = ({ title, submitLabel, submitColor, show, attribute, i {value.value_and_unit} )} - loadOptions={handleLoadOptions} - defaultOptions /> + loadOptions={(search, callback) => { + handleLoadOptions(search, values.snapshot ? 'all' : '', callback) + }} + defaultOptions={[]} />
-

{gettext('You can fill the tab with answers from a similar tab in this or another project.')}

+

+ {gettext('You can fill this tab with answers from a similar tab in any ' + + 'project you are allowed to access.')} +

) } From 0bf452a87aa952f3fb5cb6147f6f7c2f73e7dbe3 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 8 Nov 2024 20:52:59 +0100 Subject: [PATCH 06/42] Add import function into existing set --- rdmo/core/assets/scss/style.scss | 1 + .../js/interview/actions/interviewActions.js | 19 +++-- .../js/interview/components/main/Search.js | 68 +++++++++++++++ .../components/main/page/PageHead.js | 81 +++++++++++------- .../components/main/page/PageHeadFormModal.js | 68 ++------------- .../main/page/PageHeadImportModal.js | 70 ++++++++++++++++ rdmo/projects/managers.py | 3 + rdmo/projects/viewsets.py | 82 ++++++++++++++----- 8 files changed, 275 insertions(+), 117 deletions(-) create mode 100644 rdmo/projects/assets/js/interview/components/main/Search.js create mode 100644 rdmo/projects/assets/js/interview/components/main/page/PageHeadImportModal.js diff --git a/rdmo/core/assets/scss/style.scss b/rdmo/core/assets/scss/style.scss index 795a6f9228..63c2175578 100644 --- a/rdmo/core/assets/scss/style.scss +++ b/rdmo/core/assets/scss/style.scss @@ -317,6 +317,7 @@ form .yesno label { /* modals */ .modal-body { + > div:last-child, > p:last-child, formgroup:last-child .form-group { margin-bottom: 0; diff --git a/rdmo/projects/assets/js/interview/actions/interviewActions.js b/rdmo/projects/assets/js/interview/actions/interviewActions.js index 07b8849553..31d43a85c2 100644 --- a/rdmo/projects/assets/js/interview/actions/interviewActions.js +++ b/rdmo/projects/assets/js/interview/actions/interviewActions.js @@ -611,18 +611,20 @@ export function deleteSetError(errors) { return {type: DELETE_SET_ERROR, errors} } -export function copySet(currentSet, currentSetValue, attrs) { +export function copySet(currentSet, copySetValue, attrs) { const pendingId = isNil(currentSet) ? 'copySet' : `copySet/${currentSet.set_prefix}/${currentSet.set_index}` return (dispatch, getState) => { dispatch(addToPending(pendingId)) dispatch(copySetInit()) - // create a new set - const set = SetFactory.create(attrs) + // create a new set (create) or use the current one (import) + const set = isNil(attrs.id) ? SetFactory.create(attrs) : currentSet - // create a value for the text if the page has an attribute - const value = isNil(attrs.attribute) ? null : ValueFactory.create(attrs) + // create a value for the text if the page has an attribute (create) or use the current one (import) + const value = isNil(attrs.attribute) ? null : ( + isNil(attrs.id) ? ValueFactory.create(attrs) : attrs + ) // create a callback function to be called immediately or after saving the value const copySetCallback = (setValues) => { @@ -631,7 +633,10 @@ export function copySet(currentSet, currentSetValue, attrs) { const state = getState().interview const page = state.page - const values = [...state.values, ...setValues] + const values = [ + ...state.values.filter(v => !setValues.some(sv => compareValues(v, sv))), // remove updated values + ...setValues + ] const sets = gatherSets(values) initSets(sets, page) @@ -667,7 +672,7 @@ export function copySet(currentSet, currentSetValue, attrs) { }) ) } else { - promise = ValueApi.copySet(projectId, value, currentSetValue) + promise = ValueApi.copySet(projectId, value, copySetValue) } return promise.then((values) => { diff --git a/rdmo/projects/assets/js/interview/components/main/Search.js b/rdmo/projects/assets/js/interview/components/main/Search.js new file mode 100644 index 0000000000..298ab231f7 --- /dev/null +++ b/rdmo/projects/assets/js/interview/components/main/Search.js @@ -0,0 +1,68 @@ +import React from 'react' +import PropTypes from 'prop-types' +import AsyncSelect from 'react-select/async' +import { useDebouncedCallback } from 'use-debounce' + +import ValueApi from '../../api/ValueApi' + +const Search = ({ attribute, values, setValues }) => { + + const handleLoadOptions = useDebouncedCallback((search, snapshot, callback) => { + ValueApi.searchValues({ attribute, search, snapshot }).then(response => { + callback(response) + }) + }, 500) + + return <> + gettext( + 'No answers match your search.' + )} + loadingMessage={() => gettext('Loading ...')} + options={[]} + value={values.copySetValue} + onChange={(id) => setValues({ ...values, copySetValue: id })} + getOptionValue={(value) => value} + getOptionLabel={(value) => value.value_title} + formatOptionLabel={(value) => ( +
+ {gettext('Project')} {value.project_title} + { + value.snapshot && <> + → + {gettext('Snapshot')} {value.snapshot_title} + + } + → + {value.value_and_unit} +
+ )} + loadOptions={(search, callback) => { + handleLoadOptions(search, values.snapshot ? 'all' : '', callback) + }} + defaultOptions={[]} + /> + +
+ +
+ +} + +Search.propTypes = { + attribute: PropTypes.number.isRequired, + values: PropTypes.object.isRequired, + setValues: PropTypes.func.isRequired +} + +export default Search diff --git a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js index 13b4029c88..b4350d987d 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js @@ -8,6 +8,7 @@ import useModal from 'rdmo/core/assets/js/hooks/useModal' import PageHeadDeleteModal from './PageHeadDeleteModal' import PageHeadFormModal from './PageHeadFormModal' +import PageHeadImportModal from './PageHeadImportModal' const PageHead = ({ templates, page, sets, values, currentSet, activateSet, createSet, updateSet, deleteSet, copySet }) => { @@ -18,12 +19,13 @@ const PageHead = ({ templates, page, sets, values, currentSet, )) ) - const {show: showCreateModal, open: openCreateModal, close: closeCreateModal} = useModal() - const {show: showUpdateModal, open: openUpdateModal, close: closeUpdateModal} = useModal() - const {show: showDeleteModal, open: openDeleteModal, close: closeDeleteModal} = useModal() - const {show: showCopyModal, open: openCopyModal, close: closeCopyModal} = useModal() + const createModal = useModal() + const updateModal = useModal() + const copyModal = useModal() + const importModal = useModal() + const deleteModal = useModal() - const handleActivateSet = (event, set) => { + const handleActivate = (event, set) => { event.preventDefault() if (set.set_index != currentSet.set_index) { activateSet(set) @@ -32,10 +34,10 @@ const PageHead = ({ templates, page, sets, values, currentSet, const handleOpenCreateModal = (event) => { event.preventDefault() - openCreateModal() + createModal.open() } - const handleCreateSet = (text, copySetValue) => { + const handleCreate = (text, copySetValue) => { if (isEmpty(copySetValue)) { createSet({ attribute: page.attribute, @@ -51,27 +53,32 @@ const PageHead = ({ templates, page, sets, values, currentSet, text }) } - closeCreateModal() + createModal.close() } - const handleUpdateSet = (text) => { + const handleUpdate = (text) => { updateSet(currentSetValue, { text }) - closeUpdateModal() + updateModal.close() } - const handleDeleteSet = () => { + const handleDelete = () => { deleteSet(currentSet, currentSetValue) - closeDeleteModal() + deleteModal.close() } - const handleCopySet = (text) => { + const handleCopy = (text) => { copySet(currentSet, currentSetValue, { attribute: page.attribute, set_index: last(sets) ? last(sets).set_index + 1 : 0, set_collection: page.is_collection, text }) - closeCopyModal() + copyModal.close() + } + + const handleImport = (copySetValue) => { + copySet(currentSet, copySetValue, currentSetValue) + importModal.close() } return page.is_collection && ( @@ -89,7 +96,7 @@ const PageHead = ({ templates, page, sets, values, currentSet, )) return (
  • - handleActivateSet(event, set)}> + handleActivate(event, set)}> {isNil(setValue) ? `#${set.set_index + 1}` : setValue.text}
    • @@ -105,15 +112,16 @@ const PageHead = ({ templates, page, sets, values, currentSet,
    { page.attribute && ( -
    ) : ( - ) @@ -123,21 +131,21 @@ const PageHead = ({ templates, page, sets, values, currentSet, title={capitalize(page.verbose_name)} submitLabel={gettext('Create')} submitColor="success" - show={showCreateModal} + show={createModal.show} attribute={page.attribute} initial={{ text: '', copySetValue: '', snapshot: false }} - onClose={closeCreateModal} - onSubmit={handleCreateSet} + onClose={createModal.close} + onSubmit={handleCreate} /> { currentSetValue && ( @@ -145,20 +153,31 @@ const PageHead = ({ templates, page, sets, values, currentSet, title={capitalize(page.verbose_name)} submitLabel={gettext('Update')} submitColor="primary" - show={showUpdateModal} + show={updateModal.show} attribute={page.attribute} initial={{ text: currentSetValue.text }} - onClose={closeUpdateModal} - onSubmit={handleUpdateSet} + onClose={updateModal.close} + onSubmit={handleUpdate} + /> + ) + } + { + currentSetValue && ( + ) } ) diff --git a/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js b/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js index f2e7586608..e5226028f1 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHeadFormModal.js @@ -1,16 +1,13 @@ import React, { useState, useRef, useEffect } from 'react' -import AsyncSelect from 'react-select/async' import PropTypes from 'prop-types' import classNames from 'classnames' -import { useDebouncedCallback } from 'use-debounce' import { isEmpty, isNil, isUndefined } from 'lodash' -import ValueApi from '../../../api/ValueApi' - import Modal from 'rdmo/core/assets/js/components/Modal' import useFocusEffect from '../../../hooks/useFocusEffect' +import Search from '../Search' const PageHeadFormModal = ({ title, submitLabel, submitColor, show, attribute, initial, onClose, onSubmit }) => { @@ -19,23 +16,17 @@ const PageHeadFormModal = ({ title, submitLabel, submitColor, show, attribute, i const [values, setValues] = useState(initial) const [errors, setErrors] = useState([]) - const handleLoadOptions = useDebouncedCallback((search, snapshot, callback) => { - ValueApi.searchValues({ attribute, search, snapshot }).then(response => { - callback(response) - }) - }, 500) - const handleSubmit = () => { - if (isEmpty(values.text) && !isNil(attribute)) { + if (isEmpty(values.text)) { setErrors({ text: true }) } else { onSubmit(values.text, values.copySetValue) } } + // init the form values useEffect(() => { if (show && !isNil(attribute)) { - // update the form values setValues(initial) } }, [show]) @@ -78,60 +69,19 @@ const PageHeadFormModal = ({ title, submitLabel, submitColor, show, attribute, i }} /> -

    {gettext('Please give the tab a meaningful name.')}

    +

    {gettext('Please give the tab a meaningful name.')}

    - { !isUndefined(values.copySetValue) && (
    - gettext( - 'No answers match your search.' - )} - loadingMessage={() => gettext('Loading ...')} - options={[]} - value={values.copySetValue} - onChange={(id) => setValues({ ...values, copySetValue: id })} - getOptionValue={(value) => value} - getOptionLabel={(value) => value.value_title} - formatOptionLabel={(value) => ( -
    - {gettext('Project')} {value.project_title} - { - value.snapshot && <> - → - {gettext('Snapshot')} {value.snapshot_title} - - } - → - {value.value_and_unit} -
    - )} - loadOptions={(search, callback) => { - handleLoadOptions(search, values.snapshot ? 'all' : '', callback) - }} - defaultOptions={[]} /> - -
    - -
    - -

    - {gettext('You can fill this tab with answers from a similar tab in any ' + + + +

    + {gettext('You can populate this tab with answers from a similar tab in any ' + 'project you are allowed to access.')}

    diff --git a/rdmo/projects/assets/js/interview/components/main/page/PageHeadImportModal.js b/rdmo/projects/assets/js/interview/components/main/page/PageHeadImportModal.js new file mode 100644 index 0000000000..ea490dcc9b --- /dev/null +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHeadImportModal.js @@ -0,0 +1,70 @@ +import React, { useState, useEffect } from 'react' +import PropTypes from 'prop-types' +import classNames from 'classnames' +import { isEmpty } from 'lodash' + +import Modal from 'rdmo/core/assets/js/components/Modal' + +import Search from '../Search' + +const PageHeadImportModal = ({ title, show, attribute, onClose, onSubmit }) => { + + const initial = { + copySetValue: '', + snapshot: false + } + + const [values, setValues] = useState(initial) + const [errors, setErrors] = useState([]) + + const handleSubmit = () => { + if (isEmpty(values.copySetValue)) { + setErrors({ copySetValue: true }) + } else { + onSubmit(values.copySetValue) + } + } + + // init the form values + useEffect(() => { + if (show) { + setValues(initial) + } + }, [show]) + + // remove the hasError flag if an inputValue is entered + useEffect(() => { + if (!isEmpty(values.copySetValue)) { + setErrors({ copySetValue: false }) + } + }, [values, values.copySetValue]) + + return ( + +
    + + + + +

    + {gettext('You can populate this tab with answers from a similar tab in any ' + + 'project you have access to. This only affects questions that ' + + 'don\'t already have an answer.')} +

    +
    +     + ) +} + +PageHeadImportModal.propTypes = { + title: PropTypes.string.isRequired, + show: PropTypes.bool.isRequired, + attribute: PropTypes.number, + onClose: PropTypes.func.isRequired, + onSubmit: PropTypes.func.isRequired +} + +export default PageHeadImportModal diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index 8231088b5c..3c12923e0a 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -149,6 +149,9 @@ def filter_user(self, user): else: return self.none() + def filter_empty(self): + return self.filter((Q(text='') | Q(text=None)) & Q(option=None) & (Q(file='') | Q(file=None))) + def exclude_empty(self): return self.exclude((Q(text='') | Q(text=None)) & Q(option=None) & (Q(file='') | Q(file=None))) diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index e4e0ee1a7f..fb016e9ca5 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -537,7 +537,7 @@ def copy_set(self, request, parent_lookup_project, pk=None): # copy all values for questions in questionset collections with the attribute # for this value and the same set_prefix and set_index - # obtain the id of the value we want to copy + # obtain the id of the set value for the set we want to copy try: copy_value_id = int(request.data.pop('copy_set_value')) except KeyError as e: @@ -547,42 +547,84 @@ def copy_set(self, request, parent_lookup_project, pk=None): except ValueError as e: raise NotFound from e - # look for this value in the database, using the users permissions + # look for this value in the database, using the users permissions, and + # collect all values for this set and all descendants try: copy_value = Value.objects.filter_user(self.request.user).get(id=copy_value_id) + copy_values = Value.objects.filter_user(self.request.user).filter_set(copy_value) except Value.DoesNotExist as e: raise NotFound from e - # collect all values for this set and all descendants - copy_values = Value.objects.filter_user(self.request.user).filter_set(copy_value) + # init list of values to return + response_values = [] - # de-serialize the posted new set value and save it, use the ValueSerializer - # instead of ProjectValueSerializer, since the latter does not include project - set_value_serializer = ValueSerializer(data={ - 'project': parent_lookup_project, - **request.data - }) - set_value_serializer.is_valid(raise_exception=True) - set_value = set_value_serializer.save() - set_value_data = set_value_serializer.data + set_value_id = request.data.get('id') + if set_value_id: + # if an id is given in the post request, this is an import + try: + # look for the set value for the set we want to import into + set_value = Value.objects.filter_user(self.request.user).get(id=set_value_id) + + # collect all non-empty values for this set and all descendants and convert + # them to a list to compare them later to the new values + set_values = Value.objects.filter_user(self.request.user).filter_set(set_value) + set_values_list = set_values.exclude_empty().values_list('attribute', 'set_prefix', 'set_index') + set_empty_values_list = set_values.filter_empty().values_list( + 'attribute', 'set_prefix', 'set_index', 'collection_index' + ) + except Value.DoesNotExist as e: + raise NotFound from e + else: + # otherwise, we want to create a new set and need to create a new set value + # de-serialize the posted new set value and save it, use the ValueSerializer + # instead of ProjectValueSerializer, since the latter does not include project + set_value_serializer = ValueSerializer(data={ + 'project': parent_lookup_project, + **request.data + }) + set_value_serializer.is_valid(raise_exception=True) + set_value = set_value_serializer.save() + set_values_list = set_empty_values_list = [] + + # add the new set value to response_values + response_values.append(set_value_serializer.data) # create new values for the new set - values = [] - for value in currentValues: + new_values = [] + updated_values = [] + for value in copy_values: value.id = None if value.set_prefix == set_value.set_prefix: value.set_index = set_value.set_index else: value.set_prefix = compute_set_prefix_from_set_value(set_value, value) - values.append(value) + + # check if the value already exists, we do not consider collection_index + # since we do not want to import e.g. into partially filled checkboxes + if (value.attribute_id, value.set_prefix, value.set_index) in set_values_list: + # do not overwrite existing values + pass + elif (value.attribute_id, value.set_prefix, + value.set_index, value.collection_index) in set_empty_values_list: + # update empty values + updated_value = set_values.get(attribute_id=value.attribute_id, set_prefix=value.set_prefix, + set_index=value.set_index, collection_index=value.collection_index) + updated_value.text = value.text + updated_value.option = value.option + updated_value.external_id = value.external_id + updated_value.save() + + updated_values.append(updated_value) + else: + new_values.append(value) # bulk create the new values - values = Value.objects.bulk_create(values) - values_data = [ValueSerializer(instance=value).data for value in values] + created_values = Value.objects.bulk_create(new_values) + response_values += [ValueSerializer(instance=value).data for value in created_values] + response_values += [ValueSerializer(instance=value).data for value in updated_values] # return all new values - headers = self.get_success_headers(set_value_serializer.data) - return Response([set_value_data, *values_data], status=status.HTTP_201_CREATED, headers=headers) + return Response(response_values, status=status.HTTP_201_CREATED) @action(detail=True, methods=['DELETE'], url_path='set', permission_classes=(HasModelPermission | HasProjectPermission, )) From f6b813e0b9715cdde020ba2811ee8bbfb644bd35 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 8 Nov 2024 21:19:13 +0100 Subject: [PATCH 07/42] Update/fix tests --- rdmo/projects/managers.py | 2 +- .../tests/test_viewset_project_value.py | 139 +--------- .../test_viewset_project_value_copy_set.py | 243 ++++++++++++++++++ 3 files changed, 245 insertions(+), 139 deletions(-) create mode 100644 rdmo/projects/tests/test_viewset_project_value_copy_set.py diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index 3c12923e0a..16b4f223e7 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -176,7 +176,7 @@ def filter_set(self, set_value): f'{set_value.set_prefix}|{set_value.set_index}' if set_value.set_prefix else str(set_value.set_index) # collect all values for this set and all descendants - return self.filter(snapshot=set_value.snapshot, attribute__in=attributes).filter( + return self.filter(project=set_value.project, snapshot=set_value.snapshot, attribute__in=attributes).filter( Q(set_prefix=set_value.set_prefix, set_index=set_value.set_index) | Q(set_prefix__startswith=descendants_set_prefix) ) diff --git a/rdmo/projects/tests/test_viewset_project_value.py b/rdmo/projects/tests/test_viewset_project_value.py index 38d214a7c8..e4d9dd089c 100644 --- a/rdmo/projects/tests/test_viewset_project_value.py +++ b/rdmo/projects/tests/test_viewset_project_value.py @@ -4,12 +4,11 @@ import pytest from django.conf import settings -from django.contrib.auth.models import User from django.urls import reverse from rdmo.core.constants import VALUE_TYPE_FILE, VALUE_TYPE_TEXT -from ..models import Membership, Value +from ..models import Value users = ( ('owner', 'owner'), @@ -294,142 +293,6 @@ def test_copy_set(db, client, username, password, value_id, set_values_count): assert Value.objects.count() == values_count -def test_copy_set_from_other_project(db, client): - ''' - A set can be copied from a project where the user has permissions. - ''' - client.login(username='user', password='user') - - set_value_id, set_values_count = set_values[0] - set_value = Value.objects.get(id=set_value_id) - values_count = Value.objects.count() - - # add the user to the project with the set value as well as the other project - Membership.objects.create(project_id=set_value.project.id, user=User.objects.get(username='user'), role='author') - Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') - - url = reverse(urlnames['copy-set'], args=[other_project_id]) - data = { - 'attribute': set_value.attribute.id, - 'set_prefix': set_value.set_prefix, - 'set_index': 0, - 'text': 'new' - } - response = client.post(url, data=json.dumps(dict(**data, copy_set_value=set_value_id)), - content_type="application/json") - assert response.status_code == 201 - assert Value.objects.get( - project=other_project_id, - snapshot=None, - **data - ) - assert Value.objects.count() == values_count + set_values_count + 1 # one is for set/id - - -def test_copy_set_from_other_project_forbidden(db, client): - ''' - A set cannot be copied from a project where the user has no permissions. - ''' - client.login(username='user', password='user') - - set_value_id, set_values_count = set_values[0] - set_value = Value.objects.get(id=set_value_id) - values_count = Value.objects.count() - - # add the user only to the other project - Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') - - url = reverse(urlnames['copy-set'], args=[other_project_id]) - data = { - 'attribute': set_value.attribute.id, - 'set_prefix': set_value.set_prefix, - 'set_index': 0, - 'text': 'new' - } - response = client.post(url, data=json.dumps(dict(**data, copy_set_value=set_value_id)), - content_type="application/json") - assert response.status_code == 404 - assert Value.objects.count() == values_count - - -def test_copy_set_from_other_project_not_found(db, client): - ''' - A set cannot be copied when the set values does not exist. - ''' - client.login(username='user', password='user') - - set_value_id, set_values_count = set_values[0] - set_value = Value.objects.get(id=set_value_id) - values_count = Value.objects.count() - - # add the user only to the other project - Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') - - url = reverse(urlnames['copy-set'], args=[other_project_id]) - data = { - 'attribute': set_value.attribute.id, - 'set_prefix': set_value.set_prefix, - 'set_index': 0, - 'text': 'new' - } - response = client.post(url, data=json.dumps(dict(**data, copy_set_value=10000)), - content_type="application/json") - assert response.status_code == 404 - assert Value.objects.count() == values_count - - -def test_copy_set_from_other_project_invalid(db, client): - ''' - A set cannot be copied when the set values does not exist. - ''' - client.login(username='user', password='user') - - set_value_id, set_values_count = set_values[0] - set_value = Value.objects.get(id=set_value_id) - values_count = Value.objects.count() - - # add the user only to the other project - Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') - - url = reverse(urlnames['copy-set'], args=[other_project_id]) - data = { - 'attribute': set_value.attribute.id, - 'set_prefix': set_value.set_prefix, - 'set_index': 0, - 'text': 'new' - } - response = client.post(url, data=json.dumps(dict(**data, copy_set_value='wrong')), - content_type="application/json") - assert response.status_code == 404 - assert Value.objects.count() == values_count - - -def test_copy_set_from_other_project_missing(db, client): - ''' - A set cannot be copied when the set values does not exist. - ''' - client.login(username='user', password='user') - - set_value_id, set_values_count = set_values[0] - set_value = Value.objects.get(id=set_value_id) - values_count = Value.objects.count() - - # add the user only to the other project - Membership.objects.create(project_id=other_project_id, user=User.objects.get(username='user'), role='author') - - url = reverse(urlnames['copy-set'], args=[other_project_id]) - data = { - 'attribute': set_value.attribute.id, - 'set_prefix': set_value.set_prefix, - 'set_index': 0, - 'text': 'new' - } - response = client.post(url, data=json.dumps(data), - content_type="application/json") - assert response.status_code == 400 - assert Value.objects.count() == values_count - - @pytest.mark.parametrize('username,password', users) @pytest.mark.parametrize('project_id', projects) @pytest.mark.parametrize('value_id, set_values_count', set_values) diff --git a/rdmo/projects/tests/test_viewset_project_value_copy_set.py b/rdmo/projects/tests/test_viewset_project_value_copy_set.py new file mode 100644 index 0000000000..71c03c0b2c --- /dev/null +++ b/rdmo/projects/tests/test_viewset_project_value_copy_set.py @@ -0,0 +1,243 @@ +import json + +from django.contrib.auth.models import User +from django.urls import reverse + +from ..models import Membership, Value + +urlnames = { + 'copy-set': 'v1-projects:project-value-copy-set', +} + +set_value_id = 84 +set_values_count = 31 + +other_project_id = 11 + + +def test_copy(db, client): + ''' + A set can be copied from a project where the user has permissions. + ''' + client.login(username='user', password='user') + + user = User.objects.get(username='user') + + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user to the project with the set value as well as the other project + Membership.objects.create(project_id=set_value.project.id, user=user, role='author') + Membership.objects.create(project_id=other_project_id, user=user, role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(dict(**data, copy_set_value=set_value_id)), + content_type="application/json") + assert response.status_code == 201 + assert Value.objects.get( + project=other_project_id, + snapshot=None, + **data + ) + assert Value.objects.count() == values_count + set_values_count + 1 # one is for set/id + + +def test_copy_forbidden(db, client): + ''' + A set cannot be copied from a project where the user has no permissions. + ''' + client.login(username='user', password='user') + + user = User.objects.get(username='user') + + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user only to the other project + Membership.objects.create(project_id=other_project_id, user=user, role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(dict(**data, copy_set_value=set_value_id)), + content_type="application/json") + assert response.status_code == 404 + assert Value.objects.count() == values_count + + +def test_copy_not_found(db, client): + ''' + A set cannot be copied when the set value does not exist. + ''' + client.login(username='user', password='user') + + user = User.objects.get(username='user') + + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user only to the other project + Membership.objects.create(project_id=other_project_id, user=user, role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(dict(**data, copy_set_value=10000)), + content_type="application/json") + assert response.status_code == 404 + assert Value.objects.count() == values_count + + +def test_copy_invalid(db, client): + ''' + A set cannot be copied when copy_set_value is not an int. + ''' + client.login(username='user', password='user') + + user = User.objects.get(username='user') + + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user only to the other project + Membership.objects.create(project_id=other_project_id, user=user, role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(dict(**data, copy_set_value='wrong')), + content_type="application/json") + assert response.status_code == 404 + assert Value.objects.count() == values_count + + +def test_copy_missing(db, client): + ''' + A set cannot be copied when copy_set_value is not provided. + ''' + client.login(username='user', password='user') + + user = User.objects.get(username='user') + + set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user only to the other project + Membership.objects.create(project_id=other_project_id, user=user, role='author') + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'attribute': set_value.attribute.id, + 'set_prefix': set_value.set_prefix, + 'set_index': 0, + 'text': 'new' + } + response = client.post(url, data=json.dumps(data), + content_type="application/json") + assert response.status_code == 400 + assert Value.objects.count() == values_count + + +def test_copy_import(db, client): + ''' + A set can be copied (imported) into an already existing set. + ''' + client.login(username='user', password='user') + + user = User.objects.get(username='user') + + copy_set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user to the project with the set value as well as the other project + Membership.objects.create(project_id=copy_set_value.project.id, user=user, role='author') + Membership.objects.create(project_id=other_project_id, user=user, role='author') + + # create a new set + set_value = Value.objects.create(project_id=other_project_id, attribute=copy_set_value.attribute) + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'id': set_value.id + } + + response = client.post(url, data=json.dumps(dict(**data, copy_set_value=copy_set_value.id)), + content_type="application/json") + assert response.status_code == 201 + assert Value.objects.count() == values_count + set_values_count + 1 # one is the created set/id value + + +def test_copy_import_not_found(db, client): + ''' + A set cannot be imported when the set value does not exist. + ''' + client.login(username='user', password='user') + + user = User.objects.get(username='user') + + copy_set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user to the project with the set value as well as the other project + Membership.objects.create(project_id=copy_set_value.project.id, user=user, role='author') + Membership.objects.create(project_id=other_project_id, user=user, role='author') + + # create a new set + set_value = Value.objects.create(project_id=other_project_id, attribute=copy_set_value.attribute) + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'id': set_value.id + } + + response = client.post(url, data=json.dumps(dict(**data, copy_set_value=10000)), + content_type="application/json") + assert response.status_code == 404 + assert Value.objects.count() == values_count + 1 # one is the created set/id value + + +def test_copy_import_invalid(db, client): + ''' + A set cannot be imported when copy_set_value is not an int. + ''' + client.login(username='user', password='user') + + user = User.objects.get(username='user') + + copy_set_value = Value.objects.get(id=set_value_id) + values_count = Value.objects.count() + + # add the user to the project with the set value as well as the other project + Membership.objects.create(project_id=copy_set_value.project.id, user=user, role='author') + Membership.objects.create(project_id=other_project_id, user=user, role='author') + + # create a new set + set_value = Value.objects.create(project_id=other_project_id, attribute=copy_set_value.attribute) + + url = reverse(urlnames['copy-set'], args=[other_project_id]) + data = { + 'id': set_value.id + } + + response = client.post(url, data=json.dumps(dict(**data, copy_set_value='wrong')), + content_type="application/json") + assert response.status_code == 404 + assert Value.objects.count() == values_count + 1 # one is the created set/id value From 81f484d21f68e11a9d9a5f6c5a9f15ad2ff53808 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Sat, 9 Nov 2024 14:34:21 +0100 Subject: [PATCH 08/42] Rename import/copy answers to reuse answers --- .../assets/js/interview/components/main/page/PageHead.js | 6 +++--- .../js/interview/components/main/page/PageHeadFormModal.js | 2 +- .../page/{PageHeadImportModal.js => PageHeadReuseModal.js} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename rdmo/projects/assets/js/interview/components/main/page/{PageHeadImportModal.js => PageHeadReuseModal.js} (98%) diff --git a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js index b4350d987d..b5b119ac3b 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js @@ -8,7 +8,7 @@ import useModal from 'rdmo/core/assets/js/hooks/useModal' import PageHeadDeleteModal from './PageHeadDeleteModal' import PageHeadFormModal from './PageHeadFormModal' -import PageHeadImportModal from './PageHeadImportModal' +import PageHeadReuseModal from './PageHeadReuseModal' const PageHead = ({ templates, page, sets, values, currentSet, activateSet, createSet, updateSet, deleteSet, copySet }) => { @@ -116,7 +116,7 @@ const PageHead = ({ templates, page, sets, values, currentSet, ) } + + setShow(false)} onSubmit={handleSubmit}> +
    + + + + +

    + {gettext('You can reuse an answer from a similar question in any ' + + 'project you have access to.')} +

    +
    +     + +} + +QuestionReuseValue.propTypes = { + value: PropTypes.object.isRequired, + updateValue: PropTypes.func.isRequired +} + +export default QuestionReuseValue diff --git a/rdmo/projects/assets/js/interview/components/main/widget/DateWidget.js b/rdmo/projects/assets/js/interview/components/main/widget/DateWidget.js index 71903b3a2e..cc7e82f48b 100644 --- a/rdmo/projects/assets/js/interview/components/main/widget/DateWidget.js +++ b/rdmo/projects/assets/js/interview/components/main/widget/DateWidget.js @@ -5,10 +5,11 @@ import QuestionAddValue from '../question/QuestionAddValue' import QuestionCopyValue from '../question/QuestionCopyValue' import QuestionCopyValues from '../question/QuestionCopyValues' import QuestionDefault from '../question/QuestionDefault' -import QuestionError from '../question/QuestionError' import QuestionEraseValue from '../question/QuestionEraseValue' -import QuestionSuccess from '../question/QuestionSuccess' +import QuestionError from '../question/QuestionError' import QuestionRemoveValue from '../question/QuestionRemoveValue' +import QuestionReuseValue from '../question/QuestionReuseValue' +import QuestionSuccess from '../question/QuestionSuccess' import DateInput from './DateInput' @@ -28,6 +29,7 @@ const DateWidget = ({ question, sets, values, siblings, currentSet, disabled, buttons={
    + + + - + + + + + Date: Sun, 10 Nov 2024 14:43:17 +0100 Subject: [PATCH 11/42] Add reuse value model to checkboxes --- rdmo/core/settings.py | 2 + .../js/interview/components/main/Search.js | 47 ++++++-- .../main/question/QuestionReuseValue.js | 16 +-- .../main/question/QuestionReuseValues.js | 107 ++++++++++++++++++ .../components/main/widget/CheckboxInput.js | 18 ++- .../components/main/widget/CheckboxWidget.js | 44 +++---- .../main/widget/common/AdditionalTextInput.js | 2 +- .../widget/common/AdditionalTextareaInput.js | 2 +- rdmo/projects/models/value.py | 56 +++++---- rdmo/projects/serializers/v1/__init__.py | 16 +-- rdmo/projects/viewsets.py | 22 +++- 11 files changed, 261 insertions(+), 71 deletions(-) create mode 100644 rdmo/projects/assets/js/interview/components/main/question/QuestionReuseValues.js diff --git a/rdmo/core/settings.py b/rdmo/core/settings.py index 63d3b17459..ffaece00ce 100644 --- a/rdmo/core/settings.py +++ b/rdmo/core/settings.py @@ -348,6 +348,8 @@ OPTIONSET_PROVIDERS = [] +PROJECT_VALUES_SEARCH_LIMIT = 10 + PROJECT_VALUES_VALIDATION = False PROJECT_VALUES_VALIDATION_URL = True diff --git a/rdmo/projects/assets/js/interview/components/main/Search.js b/rdmo/projects/assets/js/interview/components/main/Search.js index 298ab231f7..9c307c5775 100644 --- a/rdmo/projects/assets/js/interview/components/main/Search.js +++ b/rdmo/projects/assets/js/interview/components/main/Search.js @@ -2,14 +2,40 @@ import React from 'react' import PropTypes from 'prop-types' import AsyncSelect from 'react-select/async' import { useDebouncedCallback } from 'use-debounce' +import { isNil } from 'lodash' import ValueApi from '../../api/ValueApi' -const Search = ({ attribute, values, setValues }) => { +const Search = ({ attribute, values, setValues, collection = false }) => { const handleLoadOptions = useDebouncedCallback((search, snapshot, callback) => { - ValueApi.searchValues({ attribute, search, snapshot }).then(response => { - callback(response) + ValueApi.searchValues({ attribute, search, snapshot, collection }).then(response => { + if (collection) { + // if the search component is used from QuestionReuseValues/CheckboxWidget + // the list of values from the server needs to be reduced to show only one entry + // for each set_prefix/set_index combination + callback(response.reduce((collections, value) => { + const { project_label, snapshot_label, value_label, set_prefix, set_index } = value + + // look if a value for the same set_prefix/set_index already exists in values_list + const collection = isNil(collections) ? null : collections.find(v => ( + (v.set_prefix == set_prefix) && (v.set_index == set_index) + )) + if (isNil(collection)) { + // append the value + return [...collections, { + project_label, snapshot_label, value_label, set_prefix, set_index, values: [value] + }] + } else { + // update the value_title and the values array of the existing value + collection.value_label += '; ' + value.value_label + collection.values.push(value) + return collections + } + }, [])) + } else { + callback(response) + } }) }, 500) @@ -23,21 +49,21 @@ const Search = ({ attribute, values, setValues }) => { )} loadingMessage={() => gettext('Loading ...')} options={[]} - value={values.copySetValue} - onChange={(id) => setValues({ ...values, copySetValue: id })} + value={values.value} + onChange={(value) => setValues({ ...values, value })} getOptionValue={(value) => value} - getOptionLabel={(value) => value.value_title} + getOptionLabel={(value) => value.value_label} formatOptionLabel={(value) => (
    - {gettext('Project')} {value.project_title} + {gettext('Project')} {value.project_label} { value.snapshot && <> → - {gettext('Snapshot')} {value.snapshot_title} + {gettext('Snapshot')} {value.snapshot_label} } → - {value.value_and_unit} + {value.value_label}
    )} loadOptions={(search, callback) => { @@ -62,7 +88,8 @@ const Search = ({ attribute, values, setValues }) => { Search.propTypes = { attribute: PropTypes.number.isRequired, values: PropTypes.object.isRequired, - setValues: PropTypes.func.isRequired + setValues: PropTypes.func.isRequired, + collection: PropTypes.bool } export default Search diff --git a/rdmo/projects/assets/js/interview/components/main/question/QuestionReuseValue.js b/rdmo/projects/assets/js/interview/components/main/question/QuestionReuseValue.js index 8883331fac..d2836db97d 100644 --- a/rdmo/projects/assets/js/interview/components/main/question/QuestionReuseValue.js +++ b/rdmo/projects/assets/js/interview/components/main/question/QuestionReuseValue.js @@ -10,7 +10,7 @@ import Search from '../Search' const QuestionReuseValue = ({ value, updateValue }) => { const initial = { - copySetValue: '', + value: '', snapshot: false } @@ -19,10 +19,10 @@ const QuestionReuseValue = ({ value, updateValue }) => { const [formErrors, setFormErrors] = useState([]) const handleSubmit = () => { - if (isEmpty(formValues.copySetValue)) { - setFormErrors({ copySetValue: true }) + if (isEmpty(formValues.value)) { + setFormErrors({ value: true }) } else { - const { text, option, external_id } = formValues.copySetValue + const { text, option, external_id } = formValues.value updateValue(value, { text, option, external_id }) setShow(false) } @@ -37,10 +37,10 @@ const QuestionReuseValue = ({ value, updateValue }) => { // remove the hasError flag if an inputValue is entered useEffect(() => { - if (!isEmpty(formValues.copySetValue)) { - setFormErrors({ copySetValue: false }) + if (!isEmpty(formValues.value)) { + setFormErrors({ value: false }) } - }, [formValues, formValues.copySetValue]) + }, [formValues, formValues.value]) return <> + + setShow(false)} onSubmit={handleSubmit}> +
    + + + + +

    + {gettext('You can reuse an answer from a similar question in any ' + + 'project you have access to.')} +

    +
    +     + +} + +QuestionReuseValues.propTypes = { + question: PropTypes.object.isRequired, + values: PropTypes.array.isRequired, + createValues: PropTypes.func.isRequired, + updateValue: PropTypes.func.isRequired, + deleteValue: PropTypes.func.isRequired +} + +export default QuestionReuseValues diff --git a/rdmo/projects/assets/js/interview/components/main/widget/CheckboxInput.js b/rdmo/projects/assets/js/interview/components/main/widget/CheckboxInput.js index cf7b3604a5..23d1311494 100644 --- a/rdmo/projects/assets/js/interview/components/main/widget/CheckboxInput.js +++ b/rdmo/projects/assets/js/interview/components/main/widget/CheckboxInput.js @@ -12,11 +12,25 @@ const CheckboxInput = ({ question, value, option, disabled, onCreate, onUpdate, const checked = !isNil(value) + const handleCreate = (option, additionalInput) => { + if (option.has_provider) { + onCreate([{ + external_id: option.id, + text: option.text + }]) + } else { + onCreate([{ + option: option.id, + text: additionalInput + }]) + } + } + const handleChange = () => { if (checked) { onDelete(value) } else { - onCreate(option) + handleCreate(option) } } @@ -38,7 +52,7 @@ const CheckboxInput = ({ question, value, option, disabled, onCreate, onUpdate, }) } } else { - onCreate(option, additionalInput) + handleCreate(option, additionalInput) } }, 500) diff --git a/rdmo/projects/assets/js/interview/components/main/widget/CheckboxWidget.js b/rdmo/projects/assets/js/interview/components/main/widget/CheckboxWidget.js index e1cf9975f1..2fb12acbf1 100644 --- a/rdmo/projects/assets/js/interview/components/main/widget/CheckboxWidget.js +++ b/rdmo/projects/assets/js/interview/components/main/widget/CheckboxWidget.js @@ -7,6 +7,7 @@ import { gatherOptions } from '../../../utils/options' import QuestionCopyValues from '../question/QuestionCopyValues' import QuestionEraseValues from '../question/QuestionEraseValues' import QuestionError from '../question/QuestionError' +import QuestionReuseValues from '../question/QuestionReuseValues' import QuestionSuccess from '../question/QuestionSuccess' import CheckboxInput from './CheckboxInput' @@ -14,29 +15,23 @@ import CheckboxInput from './CheckboxInput' const CheckboxWidget = ({ question, sets, values, siblings, currentSet, disabled, createValue, updateValue, deleteValue, copyValue }) => { - const handleCreateValue = (option, additionalInput) => { + const handleCreateValue = (attrsList) => { const lastValue = maxBy(values, (v) => v.collection_index) - const collectionIndex = lastValue ? lastValue.collection_index + 1 : 0 - const value = { - attribute: question.attribute, - set_prefix: currentSet.set_prefix, - set_index: currentSet.set_index, - collection_index: collectionIndex, - set_collection: question.set_collection, - unit: question.unit, - value_type: question.value_type - } - - if (option.has_provider) { - value.external_id = option.id - value.text = option.text - } else { - value.option = option.id - value.text = additionalInput - } - - createValue(value, true) + let collectionIndex = lastValue ? lastValue.collection_index + 1 : 0 + attrsList.forEach(attrs => { + createValue({ + attribute: question.attribute, + set_prefix: currentSet.set_prefix, + set_index: currentSet.set_index, + collection_index: collectionIndex, + set_collection: question.set_collection, + unit: question.unit, + value_type: question.value_type, + ...attrs + }, true) + collectionIndex += 1 + }) } const success = values.some((value) => value.success) @@ -77,6 +72,13 @@ const CheckboxWidget = ({ question, sets, values, siblings, currentSet, disabled disabled={disabled} deleteValue={deleteValue} /> + diff --git a/rdmo/projects/assets/js/interview/components/main/widget/common/AdditionalTextareaInput.js b/rdmo/projects/assets/js/interview/components/main/widget/common/AdditionalTextareaInput.js index 83c9df6275..761f2f15a6 100644 --- a/rdmo/projects/assets/js/interview/components/main/widget/common/AdditionalTextareaInput.js +++ b/rdmo/projects/assets/js/interview/components/main/widget/common/AdditionalTextareaInput.js @@ -11,7 +11,7 @@ const AdditionalTextareaInput = ({ value, option, disabled, onChange }) => { } else { setInputValue(value.option == option.id ? value.text : '') } - }, [get(value, 'id'), get(value, 'option'), get(value, 'external_id')]) + }, [get(value, 'id'), get(value, 'text'), get(value, 'option'), get(value, 'external_id')]) return (