From e554c4c3e0f177d547b9e063448dc2c64ba3379f Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Mon, 19 Aug 2024 18:30:21 +0200 Subject: [PATCH 01/18] Add copySet action to interview --- .../js/interview/actions/actionTypes.js | 4 + .../js/interview/actions/interviewActions.js | 88 +++++++++++++++++-- .../js/interview/components/main/page/Page.js | 17 ++-- .../components/main/page/PageHead.js | 43 +++++++-- .../components/main/page/PageHeadFormModal.js | 13 +-- .../assets/js/interview/containers/Main.js | 1 + .../js/interview/reducers/interviewReducer.js | 10 ++- .../projects/assets/js/interview/utils/set.js | 6 +- 8 files changed, 152 insertions(+), 30 deletions(-) diff --git a/rdmo/projects/assets/js/interview/actions/actionTypes.js b/rdmo/projects/assets/js/interview/actions/actionTypes.js index d8025bf3ad..b42995cdbf 100644 --- a/rdmo/projects/assets/js/interview/actions/actionTypes.js +++ b/rdmo/projects/assets/js/interview/actions/actionTypes.js @@ -50,3 +50,7 @@ export const CREATE_SET = 'CREATE_SET' export const DELETE_SET_INIT = 'DELETE_SET_INIT' export const DELETE_SET_SUCCESS = 'DELETE_SET_SUCCESS' export const DELETE_SET_ERROR = 'DELETE_SET_ERROR' + +export const COPY_SET_INIT = 'COPY_SET_INIT' +export const COPY_SET_SUCCESS = 'COPY_SET_SUCCESS' +export const COPY_SET_ERROR = 'COPY_SET_ERROR' diff --git a/rdmo/projects/assets/js/interview/actions/interviewActions.js b/rdmo/projects/assets/js/interview/actions/interviewActions.js index bf8bce4898..1be911c312 100644 --- a/rdmo/projects/assets/js/interview/actions/interviewActions.js +++ b/rdmo/projects/assets/js/interview/actions/interviewActions.js @@ -47,7 +47,10 @@ import { CREATE_SET, DELETE_SET_INIT, DELETE_SET_SUCCESS, - DELETE_SET_ERROR + DELETE_SET_ERROR, + COPY_SET_INIT, + COPY_SET_SUCCESS, + COPY_SET_ERROR } from './actionTypes' import { updateConfig } from 'rdmo/core/assets/js/actions/configActions' @@ -459,8 +462,8 @@ export function createSet(attrs) { // create a value for the text if the page has an attribute const value = isNil(attrs.attribute) ? null : ValueFactory.create(attrs) - // create an action to be called immediately or after saving the value - const createSetSuccess = (value) => { + // create a callback function to be called immediately or after saving the value + const createSetCallback = (value) => { dispatch(activateSet(set)) const state = getState().interview @@ -476,12 +479,12 @@ export function createSet(attrs) { } if (isNil(value)) { - return createSetSuccess() + return createSetCallback() } else { return dispatch(storeValue(value)).then(() => { const storedValue = getState().interview.values.find((v) => compareValues(v, value)) if (!isNil(storedValue)) { - createSetSuccess(storedValue) + createSetCallback(storedValue) } }) } @@ -559,3 +562,78 @@ export function deleteSetSuccess(set) { export function deleteSetError(errors) { return {type: DELETE_SET_ERROR, errors} } + +export function copySet(currentSet, attrs) { + const pendingId = `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 value for the text if the page has an attribute + const value = isNil(attrs.attribute) ? null : ValueFactory.create(attrs) + + // create a callback function to be called immediately or after saving the value + const copySetCallback = (setValues) => { + dispatch(activateSet(set)) + + const state = getState().interview + + const page = state.page + const values = [...state.values, ...setValues] + const sets = gatherSets(values) + + initSets(sets, page) + initValues(sets, values, page) + + return dispatch({type: COPY_SET_SUCCESS, values, sets}) + } + + if (isNil(value)) { + // gather all values for the currentSet and it's descendants + const currentValues = getDescendants(getState().interview.values, currentSet) + + // store each value in currentSet with the new set_index + return Promise.all( + currentValues.filter((currentValue) => !isEmptyValue(currentValue)).map((currentValue) => { + const value = {...currentValue} + const setPrefixLength = set.set_prefix.split('|').length + + if (value.set_prefix == set.set_prefix) { + value.set_index == set.set_index + } else { + value.set_prefix = value.set_prefix.split('|').reduce((acc, cur, idx) => { + return [...acc, (idx == setPrefixLength - 1) ? set.set_index : cur] + }, []).join('|') + } + + delete value.id + return ValueApi.storeValue(projectId, value) + }) + ).then((values) => { + dispatch(removeFromPending(pendingId)) + dispatch(copySetCallback(values)) + }).catch((errors) => { + dispatch(removeFromPending(pendingId)) + dispatch(copySetError(errors)) + }) + } else { + console.log(value) + } + } +} + +export function copySetInit() { + return {type: COPY_SET_INIT} +} + +export function copySetSuccess(values, sets) { + return {type: COPY_SET_SUCCESS, values, sets} +} + +export function copySetError(errors) { + return {type: COPY_SET_ERROR, errors} +} diff --git a/rdmo/projects/assets/js/interview/components/main/page/Page.js b/rdmo/projects/assets/js/interview/components/main/page/Page.js index 56bc9eba84..7b9d443025 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/Page.js +++ b/rdmo/projects/assets/js/interview/components/main/page/Page.js @@ -13,12 +13,17 @@ import PageHead from './PageHead' const Page = ({ config, templates, overview, page, sets, values, fetchPage, createValue, updateValue, deleteValue, copyValue, - activateSet, createSet, updateSet, deleteSet }) => { + activateSet, createSet, updateSet, deleteSet, copySet }) => { const currentSetPrefix = '' - const currentSetIndex = page.is_collection ? get(config, 'page.currentSetIndex', 0) : 0 - const currentSet = sets.find((set) => (set.set_prefix == currentSetPrefix && set.set_index == currentSetIndex)) || - sets.find((set) => (set.set_prefix == currentSetPrefix && set.set_index == 0)) // sanity check + let currentSetIndex = page.is_collection ? get(config, 'page.currentSetIndex', 0) : 0 + let currentSet = sets.find((set) => (set.set_prefix == currentSetPrefix && set.set_index == currentSetIndex)) + + // sanity check + if (isNil(currentSet)) { + currentSetIndex = 0 + currentSet = sets.find((set) => (set.set_prefix == currentSetPrefix && set.set_index == 0)) + } const isManager = (overview.is_superuser || overview.is_editor || overview.is_reviewer) @@ -36,6 +41,7 @@ const Page = ({ config, templates, overview, page, sets, values, fetchPage, createSet={createSet} updateSet={updateSet} deleteSet={deleteSet} + copySet={copySet} />
{ @@ -111,11 +117,12 @@ Page.propTypes = { createValue: PropTypes.func.isRequired, updateValue: PropTypes.func.isRequired, deleteValue: PropTypes.func.isRequired, + copyValue: PropTypes.func.isRequired, activateSet: PropTypes.func.isRequired, createSet: PropTypes.func.isRequired, updateSet: PropTypes.func.isRequired, deleteSet: PropTypes.func.isRequired, - copyValue: PropTypes.func.isRequired + copySet: PropTypes.func.isRequired } export default Page 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 2b3be16650..b1ff8afab0 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js @@ -9,7 +9,8 @@ import useModal from 'rdmo/core/assets/js/hooks/useModal' import PageHeadDeleteModal from './PageHeadDeleteModal' import PageHeadFormModal from './PageHeadFormModal' -const PageHead = ({ templates, page, sets, values, currentSet, activateSet, createSet, updateSet, deleteSet }) => { +const PageHead = ({ templates, page, sets, values, currentSet, + activateSet, createSet, updateSet, deleteSet, copySet }) => { const currentSetValue = isNil(currentSet) ? null : ( values.find((value) => ( @@ -20,6 +21,7 @@ const PageHead = ({ templates, page, sets, values, currentSet, activateSet, crea 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 handleActivateSet = (event, set) => { event.preventDefault() @@ -53,6 +55,16 @@ const PageHead = ({ templates, page, sets, values, currentSet, activateSet, crea closeDeleteModal() } + const handleCopySet = (text) => { + copySet(currentSet, { + attribute: page.attribute, + set_index: last(sets) ? last(sets).set_index + 1 : 0, + set_collection: page.is_collection, + text + }) + closeCopyModal() + } + return page.is_collection && (
@@ -82,12 +94,13 @@ const PageHead = ({ templates, page, sets, values, currentSet, activateSet, crea
- { - page.attribute && ( -
) : ( @@ -99,15 +112,28 @@ const PageHead = ({ templates, page, sets, values, currentSet, activateSet, crea + { currentSetValue && ( { +const PageHeadFormModal = ({ title, submitLabel, submitColor, show, initial, onClose, onSubmit }) => { const ref = useRef(null) const [inputValue, setInputValue] = useState('') const [hasError, setHasError] = useState(false) - const submitLabel = isEmpty(initial) ? gettext('Create') : gettext('Update') - const submitProps = { - className: classNames('btn', { - 'btn-success': isEmpty(initial), - 'btn-primary': !isEmpty(initial), - }) - } const handleSubmit = () => { if (isEmpty(inputValue) && !isNil(initial)) { @@ -46,7 +39,7 @@ const PageHeadFormModal = ({ title, show, initial, onClose, onSubmit }) => { useFocusEffect(ref, show) return ( - { isNil(initial) ? ( @@ -82,6 +75,8 @@ const PageHeadFormModal = ({ title, show, initial, onClose, onSubmit }) => { PageHeadFormModal.propTypes = { title: PropTypes.string.isRequired, + submitLabel: PropTypes.string.isRequired, + submitColor: PropTypes.string.isRequired, show: PropTypes.bool.isRequired, initial: PropTypes.string, onClose: PropTypes.func.isRequired, diff --git a/rdmo/projects/assets/js/interview/containers/Main.js b/rdmo/projects/assets/js/interview/containers/Main.js index a8f6189fe2..be0b98f316 100644 --- a/rdmo/projects/assets/js/interview/containers/Main.js +++ b/rdmo/projects/assets/js/interview/containers/Main.js @@ -54,6 +54,7 @@ const Main = ({ config, settings, templates, user, project, interview, configAct updateSet={interviewActions.updateSet} deleteSet={interviewActions.deleteSet} copyValue={interviewActions.copyValue} + copySet={interviewActions.copySet} /> ) } diff --git a/rdmo/projects/assets/js/interview/reducers/interviewReducer.js b/rdmo/projects/assets/js/interview/reducers/interviewReducer.js index 39ac156bc2..d62bcd60cf 100644 --- a/rdmo/projects/assets/js/interview/reducers/interviewReducer.js +++ b/rdmo/projects/assets/js/interview/reducers/interviewReducer.js @@ -25,7 +25,10 @@ import { CREATE_SET, DELETE_SET_INIT, DELETE_SET_SUCCESS, - DELETE_SET_ERROR + DELETE_SET_ERROR, + COPY_SET_INIT, + COPY_SET_SUCCESS, + COPY_SET_ERROR } from '../actions/actionTypes' const initialState = { @@ -76,6 +79,8 @@ export default function interviewReducer(state = initialState, action) { values: state.values.filter((value) => !action.values.includes(value)), sets: state.sets.filter((set) => !action.sets.includes(set)) } + case COPY_SET_SUCCESS: + return { ...state, values: action.values, sets: action.sets } case FETCH_PAGE_INIT: case FETCH_NAVIGATION_INIT: case FETCH_OPTIONS_INIT: @@ -97,6 +102,7 @@ export default function interviewReducer(state = initialState, action) { )) } case DELETE_SET_INIT: + case COPY_SET_INIT: return { ...state, errors: [] } case FETCH_PAGE_ERROR: case FETCH_NAVIGATION_ERROR: @@ -117,6 +123,8 @@ export default function interviewReducer(state = initialState, action) { case DELETE_VALUE_ERROR: case DELETE_SET_ERROR: return { ...state, errors: [...state.errors, { actionType: action.type, ...action.error }] } + case COPY_SET_ERROR: + return { ...state, errors: [...state.errors, { actionType: action.type, ...action.error }] } default: return state } diff --git a/rdmo/projects/assets/js/interview/utils/set.js b/rdmo/projects/assets/js/interview/utils/set.js index 282fa90b21..9a576875f9 100644 --- a/rdmo/projects/assets/js/interview/utils/set.js +++ b/rdmo/projects/assets/js/interview/utils/set.js @@ -1,4 +1,4 @@ -import { isEmpty, isNil, toNumber, toString, last } from 'lodash' +import { isEmpty, isNil, toNumber, toString, last, sortBy } from 'lodash' import SetFactory from '../factories/SetFactory' @@ -27,7 +27,7 @@ const getDescendants = (items, set) => { } const gatherSets = (values) => { - return values.reduce((sets, value) => { + const sets = values.reduce((sets, value) => { if (sets.find((set) => ( (set.set_prefix === value.set_prefix) && (set.set_index === value.set_index) @@ -40,6 +40,8 @@ const gatherSets = (values) => { })] } }, []) + + return sortBy(sets, ['set_prefix', 'set_index']) } const initSets = (sets, element, setPrefix) => { From f5ce70f4505bb1386ad49076fe51ea2343ec414d Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Tue, 20 Aug 2024 11:50:11 +0200 Subject: [PATCH 02/18] Add copy_set function to ProjectValueViewSet --- .../js/interview/actions/interviewActions.js | 25 +++--- .../assets/js/interview/api/ValueApi.js | 8 +- .../components/main/page/PageHead.js | 2 +- rdmo/projects/managers.py | 23 ++++++ rdmo/projects/viewsets.py | 77 +++++++++++++------ 5 files changed, 96 insertions(+), 39 deletions(-) diff --git a/rdmo/projects/assets/js/interview/actions/interviewActions.js b/rdmo/projects/assets/js/interview/actions/interviewActions.js index 1be911c312..472ce5822f 100644 --- a/rdmo/projects/assets/js/interview/actions/interviewActions.js +++ b/rdmo/projects/assets/js/interview/actions/interviewActions.js @@ -563,7 +563,7 @@ export function deleteSetError(errors) { return {type: DELETE_SET_ERROR, errors} } -export function copySet(currentSet, attrs) { +export function copySet(currentSet, currentSetValue, attrs) { const pendingId = `copySet/${currentSet.set_prefix}/${currentSet.set_index}` return (dispatch, getState) => { @@ -592,18 +592,19 @@ export function copySet(currentSet, attrs) { return dispatch({type: COPY_SET_SUCCESS, values, sets}) } + let promise if (isNil(value)) { // gather all values for the currentSet and it's descendants const currentValues = getDescendants(getState().interview.values, currentSet) // store each value in currentSet with the new set_index - return Promise.all( + promise = Promise.all( currentValues.filter((currentValue) => !isEmptyValue(currentValue)).map((currentValue) => { const value = {...currentValue} const setPrefixLength = set.set_prefix.split('|').length if (value.set_prefix == set.set_prefix) { - value.set_index == set.set_index + value.set_index = set.set_index } else { value.set_prefix = value.set_prefix.split('|').reduce((acc, cur, idx) => { return [...acc, (idx == setPrefixLength - 1) ? set.set_index : cur] @@ -613,16 +614,18 @@ export function copySet(currentSet, attrs) { delete value.id return ValueApi.storeValue(projectId, value) }) - ).then((values) => { - dispatch(removeFromPending(pendingId)) - dispatch(copySetCallback(values)) - }).catch((errors) => { - dispatch(removeFromPending(pendingId)) - dispatch(copySetError(errors)) - }) + ) } else { - console.log(value) + promise = ValueApi.copySet(projectId, currentSetValue, value) } + + return promise.then((values) => { + dispatch(removeFromPending(pendingId)) + dispatch(copySetCallback(values)) + }).catch((errors) => { + dispatch(removeFromPending(pendingId)) + dispatch(copySetError(errors)) + }) } } diff --git a/rdmo/projects/assets/js/interview/api/ValueApi.js b/rdmo/projects/assets/js/interview/api/ValueApi.js index 208bbca611..b925e5acac 100644 --- a/rdmo/projects/assets/js/interview/api/ValueApi.js +++ b/rdmo/projects/assets/js/interview/api/ValueApi.js @@ -29,8 +29,12 @@ class ValueApi extends BaseApi { } } - static deleteSet(projectId, value) { - return this.delete(`/api/v1/projects/projects/${projectId}/values/${value.id}/set/`) + static copySet(projectId, currentSetValue, setValue) { + return this.post(`/api/v1/projects/projects/${projectId}/values/${currentSetValue.id}/set/`, setValue) + } + + static deleteSet(projectId, setValue) { + return this.delete(`/api/v1/projects/projects/${projectId}/values/${setValue.id}/set/`) } } 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 b1ff8afab0..ce822f9380 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js @@ -56,7 +56,7 @@ const PageHead = ({ templates, page, sets, values, currentSet, } const handleCopySet = (text) => { - copySet(currentSet, { + copySet(currentSet, currentSetValue, { attribute: page.attribute, set_index: last(sets) ? last(sets).set_index + 1 : 0, set_collection: page.is_collection, diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index a6d49e4ff6..e2dd1b1bde 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -155,6 +155,29 @@ def exclude_empty(self): def distinct_list(self): return self.order_by('attribute').values_list('attribute', 'set_prefix', 'set_index').distinct() + def filter_set(self, set_value): + # get the catalog and prefetch most elements of the catalog + catalog = set_value.project.catalog + catalog.prefetch_elements() + + # collect the attributes of all questions of all pages or questionsets + # of this catalog, which have the attribute of this value + attributes = set() + elements = catalog.pages + catalog.questions + for element in elements: + if element.attribute == set_value.attribute: + attributes.update([descendant.attribute for descendant in element.descendants]) + + # construct the set_prefix for decendants for this set + decendants_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 decendants + return self.filter(attribute__in=attributes).filter( + Q(set_prefix=set_value.set_prefix, set_index=set_value.set_index) | + Q(set_prefix__startswith=decendants_set_prefix) + ) + class ProjectManager(CurrentSiteManagerMixin, TreeManager): diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index a0221ee894..a7c1d109ef 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 MethodNotAllowed, NotFound from rest_framework.mixins import CreateModelMixin, ListModelMixin, RetrieveModelMixin, UpdateModelMixin from rest_framework.pagination import PageNumberPagination from rest_framework.permissions import IsAuthenticated @@ -497,34 +497,61 @@ def get_queryset(self): # this is needed for the swagger ui return Value.objects.none() - @action(detail=True, methods=['DELETE'], + @action(detail=True, methods=['POST', 'DELETE'], url_path='set', permission_classes=(HasModelPermission | HasProjectPermission, )) def set(self, request, parent_lookup_project, pk=None): + if request.method == 'POST': + return self.copy_set(request, parent_lookup_project, pk) + elif request.method == 'DELETE': + return self.delete_set(request, parent_lookup_project, pk) + else: + raise MethodNotAllowed + + 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 + currentValue = self.get_object() + + # collect all values for this set and all decendants + currentValues = self.get_queryset().filter_set(currentValue) + + # 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=request.data) + set_value_serializer.is_valid(raise_exception=True) + set_value = set_value_serializer.save() + set_value_data = set_value_serializer.data + + # create new values for the new set + values = [] + set_prefix_length = len(set_value.set_prefix.split('|')) + for value in currentValues: + value.id = None + if value.set_prefix == set_value.set_prefix: + value.set_index = set_value.set_index + else: + value.set_prefix = '|'.join([ + str(set_value.set_index) if (index == set_prefix_length - 1) else value + for index, value in enumerate(value.set_prefix.split('|')) + ]) + values.append(value) + + # bulk create the new values + values = Value.objects.bulk_create(values) + values_data = [ValueSerializer(instance=value).data for value in 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) + + def delete_set(self, request, parent_lookup_project, pk=None): # delete all values for questions in questionset collections with the attribute # for this value and the same set_prefix and set_index - value = self.get_object() - value.delete() - - # prefetch most elements of the catalog - self.project.catalog.prefetch_elements() - - # collect the attributes of all questions of all pages or questionsets - # of this catalog, which have the attribute of this value - attributes = set() - elements = self.project.catalog.pages + self.project.catalog.questions - for element in elements: - if element.attribute == value.attribute: - attributes.update([descendant.attribute for descendant in element.descendants]) - - # construct the set_prefix for descendants for this set - descendants_set_prefix = f'{value.set_prefix}|{value.set_index}' if value.set_prefix else str(value.set_index) - - # delete all values for this set and all descendants - values = self.get_queryset().filter(attribute__in=attributes) \ - .filter( - Q(set_prefix=value.set_prefix, set_index=value.set_index) | - Q(set_prefix__startswith=descendants_set_prefix) - ) + set_value = self.get_object() + set_value.delete() + + # collect all values for this set and all descendants and delete them + values = self.get_queryset().filter_set(set_value) values.delete() return Response(status=204) From 02916e9a5e1f05d94fac69b9b3d09cdd6b3e3f5e Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Tue, 20 Aug 2024 12:51:07 +0200 Subject: [PATCH 03/18] Update tests --- rdmo/projects/serializers/v1/__init__.py | 3 ++ .../tests/test_viewset_project_value.py | 48 +++++++++++++++++-- rdmo/projects/viewsets.py | 5 +- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/rdmo/projects/serializers/v1/__init__.py b/rdmo/projects/serializers/v1/__init__.py index 8e61748193..1ea857fd9d 100644 --- a/rdmo/projects/serializers/v1/__init__.py +++ b/rdmo/projects/serializers/v1/__init__.py @@ -5,6 +5,7 @@ from rest_framework import serializers +from rdmo.domain.models import Attribute from rdmo.questions.models import Catalog from rdmo.services.validators import ProviderValidator @@ -404,6 +405,8 @@ class Meta: class ValueSerializer(serializers.ModelSerializer): + attribute = serializers.PrimaryKeyRelatedField(queryset=Attribute.objects.all(), required=True) + class Meta: model = Value fields = ( diff --git a/rdmo/projects/tests/test_viewset_project_value.py b/rdmo/projects/tests/test_viewset_project_value.py index aaa1fb2693..3359527fb9 100644 --- a/rdmo/projects/tests/test_viewset_project_value.py +++ b/rdmo/projects/tests/test_viewset_project_value.py @@ -1,3 +1,4 @@ +import json from pathlib import Path import pytest @@ -30,7 +31,7 @@ 'site': [1, 2, 3, 4, 5, 12] } -add_value_permission_map = change_value_permission_map = delete_value_permission_map = { +add_value_permission_map = change_value_permission_map = delete_value_permission_map = copy_value_permission_map = { 'owner': [1, 2, 3, 4, 5, 12], 'manager': [1, 3, 5], 'author': [1, 3, 5], @@ -76,6 +77,7 @@ ('phone', '+49 (0) 1337 12345678') ) + @pytest.mark.parametrize('username,password', users) @pytest.mark.parametrize('project_id', projects) def test_list(db, client, username, password, project_id): @@ -248,10 +250,50 @@ def test_delete(db, client, username, password, value_id): assert Value.objects.filter(pk=value_id).exists() +@pytest.mark.parametrize('username,password', users) +@pytest.mark.parametrize('value_id, set_values_count', set_values) +def test_copy_set(db, client, username, password, value_id, set_values_count): + client.login(username=username, password=password) + set_value = Value.objects.get(id=value_id) + values_count = Value.objects.count() + + url = reverse(urlnames['set'], args=[set_value.project_id, value_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") + + if set_value.project_id in copy_value_permission_map.get(username, []): + assert response.status_code == 201 + assert len(response.json()) == set_values_count + 1 + assert Value.objects.get( + project=set_value.project_id, + snapshot=None, + **data + ) + assert Value.objects.count() == values_count + set_values_count + 1 # one is for set/id + for value_data in response.json(): + if value_data['set_prefix'] == data['set_prefix']: + assert value_data['set_index'] == data['set_index'] + else: + set_prefix_split = value_data['set_prefix'].split('|') + assert set_prefix_split[0] == str(data['set_index']) + + elif set_value.project_id in view_value_permission_map.get(username, []): + assert response.status_code == 403 + assert Value.objects.count() == values_count + else: + assert response.status_code == 404 + 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) -def test_set(db, client, username, password, project_id, value_id, set_values_count): +@pytest.mark.parametrize('value_id, set_values_count', set_values) +def test_delete_set(db, client, username, password, project_id, value_id, set_values_count): client.login(username=username, password=password) value_exists = Value.objects.filter(project_id=project_id, snapshot=None, id=value_id).exists() values_count = Value.objects.count() diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index a7c1d109ef..b416347b08 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -517,7 +517,10 @@ def copy_set(self, request, parent_lookup_project, pk=None): # 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=request.data) + 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 From 8b316e483eadde21c61f021d5118d7c7c8d11aae Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Tue, 20 Aug 2024 17:35:57 +0200 Subject: [PATCH 04/18] Add copy_set function to question sets --- rdmo/core/assets/js/components/Modal.js | 10 +++-- .../js/interview/components/main/page/Page.js | 1 + .../main/questionset/QuestionSet.js | 8 +++- .../main/questionset/QuestionSetCopyModal.js | 21 +++++++++ .../main/questionset/QuestionSetCopySet.js | 44 +++++++++++++++++++ .../main/questionset/QuestionSetRemoveSet.js | 10 ++--- rdmo/projects/assets/scss/interview.scss | 16 +++---- 7 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetCopyModal.js create mode 100644 rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetCopySet.js diff --git a/rdmo/core/assets/js/components/Modal.js b/rdmo/core/assets/js/components/Modal.js index 22c07abeab..2ef42a543b 100644 --- a/rdmo/core/assets/js/components/Modal.js +++ b/rdmo/core/assets/js/components/Modal.js @@ -8,9 +8,13 @@ const Modal = ({ title, show, modalProps, submitLabel, submitProps, onClose, onS

{title}

 - - { children } - + { + children && ( + + { children } + + ) + } + + + + ) +} + +QuestionCopySet.propTypes = { + questionset: PropTypes.object.isRequired, + sets: PropTypes.array.isRequired, + currentSet: PropTypes.object.isRequired, + copySet: PropTypes.func.isRequired +} + +export default QuestionCopySet diff --git a/rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetRemoveSet.js b/rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetRemoveSet.js index a7364a8909..06a0dcae61 100644 --- a/rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetRemoveSet.js +++ b/rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetRemoveSet.js @@ -6,12 +6,12 @@ import useModal from 'rdmo/core/assets/js/hooks/useModal' import QuestionSetDeleteModal from './QuestionSetDeleteModal' -const QuestionAddSet = ({ questionset, set, deleteSet }) => { +const QuestionRemoveSet = ({ questionset, currentSet, deleteSet }) => { const {show: showDeleteModal, open: openDeleteModal, close: closeDeleteModal} = useModal() const handleDeleteSet = () => { - deleteSet(set) + deleteSet(currentSet) closeDeleteModal() } @@ -31,10 +31,10 @@ const QuestionAddSet = ({ questionset, set, deleteSet }) => { ) } -QuestionAddSet.propTypes = { +QuestionRemoveSet.propTypes = { questionset: PropTypes.object.isRequired, - set: PropTypes.object.isRequired, + currentSet: PropTypes.object.isRequired, deleteSet: PropTypes.func.isRequired } -export default QuestionAddSet +export default QuestionRemoveSet diff --git a/rdmo/projects/assets/scss/interview.scss b/rdmo/projects/assets/scss/interview.scss index 20eb128e67..b26bd10373 100644 --- a/rdmo/projects/assets/scss/interview.scss +++ b/rdmo/projects/assets/scss/interview.scss @@ -100,22 +100,20 @@ .interview-block-options { position: absolute; - top: 0; - right: 0; + top: 6px; + right: 8px; z-index: 5; + display: flex; + gap: 4px; + + .btn-copy-set, .btn-remove-set { opacity: 0.8; line-height: 20px; font-size: 14px; - position: absolute; - z-index: 5; - top: 0; - right: 0; - - padding-left: 8px; - padding-right: 8px; + padding: 0; &:hover { opacity: 1; From b453485104543997cc8a986b3245991a4f43ac55 Mon Sep 17 00:00:00 2001 From: David Wallace Date: Thu, 7 Nov 2024 17:18:27 +0100 Subject: [PATCH 05/18] style(projects): autofix typos Signed-off-by: David Wallace --- rdmo/projects/managers.py | 8 ++++---- rdmo/projects/viewsets.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index e2dd1b1bde..defe807031 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -168,14 +168,14 @@ def filter_set(self, set_value): if element.attribute == set_value.attribute: attributes.update([descendant.attribute for descendant in element.descendants]) - # construct the set_prefix for decendants for this set - decendants_set_prefix = \ + # 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 decendants + # collect all values for this set and all descendants return self.filter(attribute__in=attributes).filter( Q(set_prefix=set_value.set_prefix, set_index=set_value.set_index) | - Q(set_prefix__startswith=decendants_set_prefix) + Q(set_prefix__startswith=descendants_set_prefix) ) diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index b416347b08..cee88a84d4 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -1,7 +1,7 @@ from django.conf import settings from django.contrib.sites.shortcuts import get_current_site from django.core.exceptions import ObjectDoesNotExist -from django.db.models import OuterRef, Prefetch, Q, Subquery +from django.db.models import OuterRef, Prefetch, Subquery from django.db.models.functions import Coalesce, Greatest from django.http import Http404, HttpResponseRedirect from django.utils.translation import gettext_lazy as _ @@ -512,7 +512,7 @@ def copy_set(self, request, parent_lookup_project, pk=None): # for this value and the same set_prefix and set_index currentValue = self.get_object() - # collect all values for this set and all decendants + # collect all values for this set and all descendants currentValues = self.get_queryset().filter_set(currentValue) # de-serialize the posted new set value and save it, use the ValueSerializer From 1e957cb9b640b7ef2683d1c71522249c0387085d Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 10 Jan 2025 15:19:41 +0100 Subject: [PATCH 06/18] Fix modal in QuestionSetCopySet component --- .../components/main/questionset/QuestionSetCopySet.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetCopySet.js b/rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetCopySet.js index f472b099c8..cc2e5492d5 100644 --- a/rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetCopySet.js +++ b/rdmo/projects/assets/js/interview/components/main/questionset/QuestionSetCopySet.js @@ -7,27 +7,26 @@ import useModal from 'rdmo/core/assets/js/hooks/useModal' import QuestionSetCopyModal from './QuestionSetCopyModal' const QuestionCopySet = ({ questionset, sets, currentSet, copySet }) => { - - const [showCopyModal, openCopyModal, closeCopyModal] = useModal() + const modal = useModal() const handleCopySet = () => { copySet(currentSet, null, { set_prefix: currentSet.set_prefix, set_index: last(sets) ? last(sets).set_index + 1 : 0, }) - closeCopyModal() + modal.close() } return questionset.is_collection && ( <> - From 7d9e64998b55fbae45af1a26368144a6f2320dee Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 10 Jan 2025 15:20:41 +0100 Subject: [PATCH 07/18] Fix set_prefix computation in copySet --- .../assets/js/interview/actions/interviewActions.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/rdmo/projects/assets/js/interview/actions/interviewActions.js b/rdmo/projects/assets/js/interview/actions/interviewActions.js index 472ce5822f..2df51f2bf1 100644 --- a/rdmo/projects/assets/js/interview/actions/interviewActions.js +++ b/rdmo/projects/assets/js/interview/actions/interviewActions.js @@ -601,14 +601,17 @@ export function copySet(currentSet, currentSetValue, attrs) { promise = Promise.all( currentValues.filter((currentValue) => !isEmptyValue(currentValue)).map((currentValue) => { const value = {...currentValue} - const setPrefixLength = set.set_prefix.split('|').length + const setPrefixLength = isEmpty(set.set_prefix) ? 0 : set.set_prefix.split('|').length if (value.set_prefix == set.set_prefix) { value.set_index = set.set_index } else { - value.set_prefix = value.set_prefix.split('|').reduce((acc, cur, idx) => { - return [...acc, (idx == setPrefixLength - 1) ? set.set_index : cur] - }, []).join('|') + value.set_prefix = value.set_prefix.split('|').map((sp, idx) => { + // for the set_prefix of the new value, set the number at the position, which is one more + // than the length of the set_prefix of the new (and old) set, to the set_index of the new set. + // since idx counts from 0, this equals setPrefixLength + return (idx == setPrefixLength) ? set.set_index : sp + }).join('|') } delete value.id From 3dfd383ebc8837517ae8f492cdbeb3bd51d60a01 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 10 Jan 2025 15:33:43 +0100 Subject: [PATCH 08/18] Fix copy_set action --- rdmo/projects/viewsets.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index cee88a84d4..e378be478c 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -527,14 +527,14 @@ def copy_set(self, request, parent_lookup_project, pk=None): # create new values for the new set values = [] - set_prefix_length = len(set_value.set_prefix.split('|')) + set_prefix_length = len(set_value.set_prefix.split('|')) if set_value.set_prefix else 0 for value in currentValues: value.id = None if value.set_prefix == set_value.set_prefix: value.set_index = set_value.set_index else: value.set_prefix = '|'.join([ - str(set_value.set_index) if (index == set_prefix_length - 1) else value + str(set_value.set_index) if (index == set_prefix_length) else value for index, value in enumerate(value.set_prefix.split('|')) ]) values.append(value) From 4074a841525a89b5d2569a95945abd0f79810ba3 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Thu, 16 Jan 2025 15:53:03 +0100 Subject: [PATCH 09/18] Activate right tab when deleting tab --- .../assets/js/interview/actions/interviewActions.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rdmo/projects/assets/js/interview/actions/interviewActions.js b/rdmo/projects/assets/js/interview/actions/interviewActions.js index 2df51f2bf1..83e02c2b82 100644 --- a/rdmo/projects/assets/js/interview/actions/interviewActions.js +++ b/rdmo/projects/assets/js/interview/actions/interviewActions.js @@ -527,10 +527,11 @@ export function deleteSet(set, setValue) { if (sets.length > 1) { const index = sets.indexOf(set) - if (index > 0) { + if (index < sets.length - 1) { + dispatch(activateSet(sets[index + 1])) + } else { + // If it's the last set, activate the new last set dispatch(activateSet(sets[index - 1])) - } else if (index == 0) { - dispatch(activateSet(sets[1])) } } From 0821cfc1f8f3720b33b50312e1a97d8f7df6d616 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Thu, 16 Jan 2025 16:00:38 +0100 Subject: [PATCH 10/18] Add name to PageHeadDeleteModal --- .../js/interview/components/main/page/PageHead.js | 1 + .../components/main/page/PageHeadDeleteModal.js | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) 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 ce822f9380..5404c4fb82 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js @@ -143,6 +143,7 @@ const PageHead = ({ templates, page, sets, values, currentSet, } { +const PageHeadDeleteModal = ({ title, name, show, onClose, onSubmit }) => { return ( -

{gettext('You are about to permanently delete this tab.')}

+ { + name ? ( +

%s'), [name]) + }}>

+ ) : ( +

{gettext('You are about to permanently delete this tab.')}

+ ) + }

{gettext('This includes all given answers for this tab on all pages, not just this one.')}

{gettext('This action cannot be undone!')}

 @@ -16,6 +24,7 @@ const PageHeadDeleteModal = ({ title, show, onClose, onSubmit }) => { PageHeadDeleteModal.propTypes = { title: PropTypes.string.isRequired, + name: PropTypes.string, show: PropTypes.bool.isRequired, onClose: PropTypes.func.isRequired, onSubmit: PropTypes.func.isRequired, From 0dcd7b7684b82d18ead9b097b8f0f2e498ee9454 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Thu, 16 Jan 2025 17:40:04 +0100 Subject: [PATCH 11/18] Fix PageHeadDeleteModal --- .../assets/js/interview/components/main/page/PageHead.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5404c4fb82..719afecbd3 100644 --- a/rdmo/projects/assets/js/interview/components/main/page/PageHead.js +++ b/rdmo/projects/assets/js/interview/components/main/page/PageHead.js @@ -143,7 +143,7 @@ const PageHead = ({ templates, page, sets, values, currentSet, } Date: Thu, 16 Jan 2025 19:07:14 +0100 Subject: [PATCH 12/18] Refactor copyValue so that fetchNavigation etc is only called once --- .../js/interview/actions/interviewActions.js | 87 ++++++++++++++----- .../main/question/QuestionCopyValue.js | 2 +- .../main/question/QuestionCopyValues.js | 8 +- 3 files changed, 70 insertions(+), 27 deletions(-) diff --git a/rdmo/projects/assets/js/interview/actions/interviewActions.js b/rdmo/projects/assets/js/interview/actions/interviewActions.js index 83e02c2b82..e55aa275a6 100644 --- a/rdmo/projects/assets/js/interview/actions/interviewActions.js +++ b/rdmo/projects/assets/js/interview/actions/interviewActions.js @@ -1,4 +1,4 @@ -import { isEmpty, isNil } from 'lodash' +import { first, isEmpty, isNil } from 'lodash' import PageApi from '../api/PageApi' import ProjectApi from '../api/ProjectApi' @@ -366,26 +366,73 @@ export function updateValue(value, attrs, store = true) { } } -export function copyValue(value) { +export function copyValue(...originalValues) { + const firstValue = first(originalValues) + const pendingId = `copyValue/${firstValue.attribute}/${firstValue.set_prefix}/${firstValue.set_index}` + return (dispatch, getState) => { - const sets = getState().interview.sets - const values = getState().interview.values - - sets.filter((set) => ( - (set.set_prefix == value.set_prefix) && - (set.set_index != value.set_index) - )).forEach((set) => { - const sibling = values.find((v) => ( - (v.attribute == value.attribute) && - (v.set_prefix == set.set_prefix) && - (v.set_index == set.set_index) && - (v.collection_index == value.collection_index) - )) - - if (isNil(sibling)) { - dispatch(storeValue(ValueFactory.create({ ...value, set_index: set.set_index }))) - } else if (isEmptyValue(sibling)) { - dispatch(storeValue(ValueFactory.update(sibling, value))) + dispatch(addToPending(pendingId)) + + const { sets, values } = getState().interview + + // create copies for each value for all it's empty siblings + const copies = originalValues.reduce((copies, value) => { + return [ + ...copies, + ...sets.filter((set) => ( + (set.set_prefix == value.set_prefix) && + (set.set_index != value.set_index) + )).map((set) => { + const siblingIndex = values.findIndex((v) => ( + (v.attribute == value.attribute) && + (v.set_prefix == set.set_prefix) && + (v.set_index == set.set_index) && + (v.collection_index == value.collection_index) + )) + + const sibling = siblingIndex > 0 ? values[siblingIndex] : null + + if (isNil(sibling)) { + return [ValueFactory.create({ ...value, set_index: set.set_index }), siblingIndex] + } else if (isEmptyValue(sibling)) { + // the spread operator { ...sibling } does prevent an update in place + return [ValueFactory.update({ ...sibling }, value), siblingIndex] + } else { + return null + } + }).filter((value) => !isNil(value)) + ] + }, []) + + // dispatch storeValueInit for each of the updated values, + // created values have valueIndex -1 and will be skipped + // eslint-disable-next-line no-unused-vars + copies.forEach(([value, valueIndex]) => dispatch(storeValueInit(valueIndex))) + + // loop over all copies and store the values on the server + // afterwards fetchNavigation, updateProgress and check refresh once + return Promise.all( + copies.map(([value, valueIndex]) => { + return ValueApi.storeValue(projectId, value) + .then((value) => dispatch(storeValueSuccess(value, valueIndex))) + }) + ).then(() => { + dispatch(removeFromPending(pendingId)) + + const page = getState().interview.page + const sets = getState().interview.sets + const question = page.questions.find((question) => question.attribute === firstValue.attribute) + const refresh = question && question.optionsets.some((optionset) => optionset.has_refresh) + + dispatch(fetchNavigation(page)) + dispatch(updateProgress()) + + if (refresh) { + // if the refresh flag is set, reload all values for the page, + // resolveConditions will be called in fetchValues + dispatch(fetchValues(page)) + } else { + dispatch(resolveConditions(page, sets)) } }) } diff --git a/rdmo/projects/assets/js/interview/components/main/question/QuestionCopyValue.js b/rdmo/projects/assets/js/interview/components/main/question/QuestionCopyValue.js index 57c9d40879..23892a72bf 100644 --- a/rdmo/projects/assets/js/interview/components/main/question/QuestionCopyValue.js +++ b/rdmo/projects/assets/js/interview/components/main/question/QuestionCopyValue.js @@ -9,7 +9,7 @@ const QuestionCopyValue = ({ question, value, siblings, copyValue }) => { !question.is_collection && !isEmptyValue(value) && siblings.some((value) => isEmptyValue(value)) && ( - diff --git a/rdmo/projects/assets/js/interview/components/main/question/QuestionCopyValues.js b/rdmo/projects/assets/js/interview/components/main/question/QuestionCopyValues.js index 699c22a767..181098590b 100644 --- a/rdmo/projects/assets/js/interview/components/main/question/QuestionCopyValues.js +++ b/rdmo/projects/assets/js/interview/components/main/question/QuestionCopyValues.js @@ -5,17 +5,13 @@ import { isEmpty } from 'lodash' import { isEmptyValue } from '../../../utils/value' const QuestionCopyValues = ({ question, sets, values, siblings, currentSet, copyValue }) => { - const handleCopyValues = () => { - values.forEach((value) => copyValue(value)) - } - const button = question.widget_type == 'checkbox' ? ( - ) : ( - ) From 5f623b4e5b03cc18cdb023430922fd5917b8d052 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 17 Jan 2025 10:19:50 +0100 Subject: [PATCH 13/18] Use set comprehension in ValueQuerySet --- rdmo/projects/managers.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index defe807031..20764ae3b6 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -160,13 +160,13 @@ def filter_set(self, set_value): catalog = set_value.project.catalog catalog.prefetch_elements() - # collect the attributes of all questions of all pages or questionsets - # of this catalog, which have the attribute of this value - attributes = set() - elements = catalog.pages + catalog.questions - for element in elements: - if element.attribute == set_value.attribute: - attributes.update([descendant.attribute for descendant in element.descendants]) + # Get all attributes from matching elements and their descendants + attributes = { + descendant.attribute + for element in (catalog.pages + catalog.questions) + if element.attribute == set_value.attribute + for descendant in element.descendants + } # construct the set_prefix for descendants for this set descendants_set_prefix = \ From 6f7e57fbe954963424b3f3331cd08422010d294f Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 17 Jan 2025 11:13:58 +0100 Subject: [PATCH 14/18] Add get_set_prefix_for_value_copy function with tests --- rdmo/projects/tests/test_utils.py | 21 +++++++++++++++++++-- rdmo/projects/utils.py | 8 ++++++++ rdmo/projects/viewsets.py | 14 ++++++++------ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/rdmo/projects/tests/test_utils.py b/rdmo/projects/tests/test_utils.py index db2bfe2c51..a3a182c33f 100644 --- a/rdmo/projects/tests/test_utils.py +++ b/rdmo/projects/tests/test_utils.py @@ -7,8 +7,8 @@ from rdmo.core.tests.utils import compute_checksum from ..filters import ProjectFilter -from ..models import Project -from ..utils import copy_project, set_context_querystring_with_filter_and_page +from ..models import Project, Value +from ..utils import compute_set_prefix_from_set_value, copy_project, set_context_querystring_with_filter_and_page GET_queries = [ 'page=2&title=project', @@ -17,6 +17,18 @@ '' ] +SET_VALUES = [ + ({'set_prefix': '' , 'set_index': 1}, {'set_prefix': '0'}, '1'), + ({'set_prefix': '' , 'set_index': 1}, {'set_prefix': '0|0'}, '1|0'), + ({'set_prefix': '' , 'set_index': 1}, {'set_prefix': '0|0|0'}, '1|0|0'), + ({'set_prefix': '' , 'set_index': 2}, {'set_prefix': '0'}, '2'), + ({'set_prefix': '' , 'set_index': 2}, {'set_prefix': '0|0'}, '2|0'), + ({'set_prefix': '' , 'set_index': 2}, {'set_prefix': '0|0|0'}, '2|0|0'), + ({'set_prefix': '0' , 'set_index': 1}, {'set_prefix': '0|0'}, '0|1'), + ({'set_prefix': '0' , 'set_index': 1}, {'set_prefix': '0|0|0'}, '0|1|0'), + ({'set_prefix': '0|0', 'set_index': 1}, {'set_prefix': '0|0|0'}, '0|0|1'), +] + @pytest.mark.parametrize('GET_query', GET_queries) def test_set_context_querystring_with_filter_and_page(GET_query): querydict = QueryDict(GET_query) @@ -128,3 +140,8 @@ def test_copy_project(db, files): compute_checksum(value.file.open('rb').read()) else: assert not value.file + + +@pytest.mark.parametrize('set_value, value, result', SET_VALUES) +def test_compute_set_prefix_from_set_value(set_value, value, result): + assert compute_set_prefix_from_set_value(Value(**set_value), Value(**value)) == result diff --git a/rdmo/projects/utils.py b/rdmo/projects/utils.py index ab619b54b4..06153b855e 100644 --- a/rdmo/projects/utils.py +++ b/rdmo/projects/utils.py @@ -261,3 +261,11 @@ def get_upload_accept(): else: return None return ','.join(accept) + + +def compute_set_prefix_from_set_value(set_value, value): + set_prefix_length = len(set_value.set_prefix.split('|')) if set_value.set_prefix else 0 + return '|'.join([ + str(set_value.set_index) if (index == set_prefix_length) else value + for index, value in enumerate(value.set_prefix.split('|')) + ]) diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index e378be478c..c126171595 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -75,7 +75,13 @@ ) from .serializers.v1.overview import CatalogSerializer, ProjectOverviewSerializer from .serializers.v1.page import PageSerializer -from .utils import check_conditions, copy_project, get_upload_accept, send_invite_email +from .utils import ( + check_conditions, + compute_set_prefix_from_set_value, + copy_project, + get_upload_accept, + send_invite_email, +) class ProjectPagination(PageNumberPagination): @@ -527,16 +533,12 @@ def copy_set(self, request, parent_lookup_project, pk=None): # create new values for the new set values = [] - set_prefix_length = len(set_value.set_prefix.split('|')) if set_value.set_prefix else 0 for value in currentValues: value.id = None if value.set_prefix == set_value.set_prefix: value.set_index = set_value.set_index else: - value.set_prefix = '|'.join([ - str(set_value.set_index) if (index == set_prefix_length) else value - for index, value in enumerate(value.set_prefix.split('|')) - ]) + value.set_prefix = compute_set_prefix_from_set_value(set_value, value) values.append(value) # bulk create the new values From 23d4623436e6fef0f8ee6252addee342fc110f01 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Thu, 16 Jan 2025 15:25:44 +0100 Subject: [PATCH 15/18] Fix select input component --- .../js/interview/components/main/widget/SelectInput.js | 5 +---- rdmo/projects/assets/scss/interview.scss | 4 ++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/rdmo/projects/assets/js/interview/components/main/widget/SelectInput.js b/rdmo/projects/assets/js/interview/components/main/widget/SelectInput.js index a0d694957e..b53f1f6baf 100644 --- a/rdmo/projects/assets/js/interview/components/main/widget/SelectInput.js +++ b/rdmo/projects/assets/js/interview/components/main/widget/SelectInput.js @@ -22,14 +22,10 @@ import OptionText from './common/OptionText' const SelectInput = ({ question, value, options, disabled, creatable, updateValue, buttons }) => { const [inputValue, setInputValue] = useState('') - // const [isOpen, setIsOpen] = useState(false) const handleChange = (option) => { if (isNil(option)) { - // close the select input when the value is reset - // setIsOpen(false) setInputValue('') - updateValue(value, {}) } else if (option.__isNew__ === true) { updateValue(value, { @@ -85,6 +81,7 @@ const SelectInput = ({ question, value, options, disabled, creatable, updateValu const isAsync = question.optionsets.some((optionset) => optionset.has_search) const selectProps = { + key: value.id, classNamePrefix: 'react-select', className: classnames, backspaceRemovesValue: false, diff --git a/rdmo/projects/assets/scss/interview.scss b/rdmo/projects/assets/scss/interview.scss index b26bd10373..4fa5b2a1c4 100644 --- a/rdmo/projects/assets/scss/interview.scss +++ b/rdmo/projects/assets/scss/interview.scss @@ -370,6 +370,10 @@ display: inline; } } + + .react-select { + max-width: 100%; + } } .badge-optional { From c619ff53a4113bb80d62872082a8b8daf32c38a7 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Thu, 16 Jan 2025 15:36:45 +0100 Subject: [PATCH 16/18] Fix visibility related issues --- .../js/projects/components/main/Projects.js | 24 +++++++++++++++---- rdmo/projects/serializers/v1/__init__.py | 5 +++- .../projects/project_form_visibility.html | 8 +++---- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/rdmo/projects/assets/js/projects/components/main/Projects.js b/rdmo/projects/assets/js/projects/components/main/Projects.js index 6f15f8e7b8..fb9b672e5a 100644 --- a/rdmo/projects/assets/js/projects/components/main/Projects.js +++ b/rdmo/projects/assets/js/projects/components/main/Projects.js @@ -116,20 +116,36 @@ const Projects = ({ config, configActions, currentUserObject, projectsActions, p if (myProjects) { visibleColumns.splice(2, 0, 'role') - columnWidths = ['35%', '20%', '20%', '20%', '5%'] + columnWidths = ['40%', '18%', '18%', '18%', '6%'] } else { visibleColumns.splice(2, 0, 'created') visibleColumns.splice(2, 0, 'owner') - columnWidths = ['35%', '10%', '20%', '20%', '20%', '5%'] + columnWidths = ['30%', '10%', '18%', '18%', '18%', '6%'] } const cellFormatters = { title: (content, row) => renderTitle(content, row), role: (_content, row) => { const { rolesString } = getUserRoles(row, currentUserId) - return rolesString + return <> + { + rolesString &&

{rolesString}

+ } + { + row.visibility &&

{row.visibility}

+ } + }, - owner: (_content, row) => row.owners.map(owner => `${owner.first_name} ${owner.last_name}`).join('; '), + owner: (_content, row) => ( + <> +

+ {row.owners.map(owner => `${owner.first_name} ${owner.last_name}`).join('; ')} +

+ { + row.visibility &&

{row.visibility}

+ } + + ), progress: (_content, row) => getProgressString(row), created: content => useFormattedDateTime(content, language), last_changed: content => useFormattedDateTime(content, language), diff --git a/rdmo/projects/serializers/v1/__init__.py b/rdmo/projects/serializers/v1/__init__.py index 1ea857fd9d..192f20aed8 100644 --- a/rdmo/projects/serializers/v1/__init__.py +++ b/rdmo/projects/serializers/v1/__init__.py @@ -65,6 +65,8 @@ def get_queryset(self): last_changed = serializers.DateTimeField(read_only=True) + visibility = serializers.CharField(source='visibility.get_help_display') + class Meta: model = Project fields = ( @@ -85,7 +87,8 @@ class Meta: 'site', 'views', 'progress_total', - 'progress_count' + 'progress_count', + 'visibility' ) read_only_fields = ( 'snapshots', diff --git a/rdmo/projects/templates/projects/project_form_visibility.html b/rdmo/projects/templates/projects/project_form_visibility.html index 2356b821ee..4c5a736c5c 100644 --- a/rdmo/projects/templates/projects/project_form_visibility.html +++ b/rdmo/projects/templates/projects/project_form_visibility.html @@ -19,12 +19,12 @@

{% endblocktrans %}

- {% if object.visibility and 'sites' in form.fields or 'groups' in form.fields %} + {% if not object.visibility %} + {% bootstrap_form submit=_('Make visible') %} + {% elif object.visibility and 'sites' in form.fields or 'groups' in form.fields %} {% bootstrap_form submit=_('Update visibility') delete=_('Remove visibility') %} - {% elif object.visibility %} - {% bootstrap_form delete=_('Remove visibility') %} {% else %} - {% bootstrap_form submit=_('Make visible') %} + {% bootstrap_form delete=_('Remove visibility') %} {% endif %} {% endblock %} From 475b6ac7260400b9ad1791dd83b8ab5594f41196 Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 17 Jan 2025 14:31:24 +0100 Subject: [PATCH 17/18] Fix visibility field in ProjectSerializer --- rdmo/projects/serializers/v1/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rdmo/projects/serializers/v1/__init__.py b/rdmo/projects/serializers/v1/__init__.py index 192f20aed8..26ea38402d 100644 --- a/rdmo/projects/serializers/v1/__init__.py +++ b/rdmo/projects/serializers/v1/__init__.py @@ -65,7 +65,7 @@ def get_queryset(self): last_changed = serializers.DateTimeField(read_only=True) - visibility = serializers.CharField(source='visibility.get_help_display') + visibility = serializers.CharField(source='visibility.get_help_display', read_only=True) class Meta: model = Project From 6ce3b14f4464aadf035ca16909c641046cf48eef Mon Sep 17 00:00:00 2001 From: Jochen Klar Date: Fri, 17 Jan 2025 14:33:16 +0100 Subject: [PATCH 18/18] Add visibility to select_related for ProjectViewSet --- rdmo/projects/viewsets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index c126171595..9eccecfa28 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -121,7 +121,7 @@ def get_queryset(self): 'snapshots', 'views', Prefetch('memberships', queryset=Membership.objects.select_related('user'), to_attr='memberships_list') - ).select_related('catalog') + ).select_related('catalog', 'visibility') # prepare subquery for last_changed last_changed_subquery = Subquery(