From 6e289f03f9aaed420c1d24a9cd190d25dec65d07 Mon Sep 17 00:00:00 2001 From: Tim Willig <92720438+twilligls@users.noreply.github.com> Date: Wed, 15 Nov 2023 09:52:03 +0100 Subject: [PATCH] QE-513: tempId for newly created objects (#3594) * QuestionGroups return tempId - real Id mapping * Questions return tempId - real Id mapping * Fix unit test and code check * SubQuestions return tempId - real Id mapping * If question with subquestions was created, their mapping will also be returned * Fixed issue: OpHandlerAnswer does a proper update when aid is provided * Answers return tempId - real Id mapping * If question with answers was created, their mapping will now also be returned * Non DB attributes of Question should be camelcase * Avoided nesting in if statement * Response output now handled with classes * Codestyle fix * ignore method length warning for PatcherSurvey __construct() * fixes for psalm * fixes for psalm * fixes for psalm * Dev: Refactor SurveyPatch/PatcherSurvey --------- Co-authored-by: Kevin Foster --- .../libraries/Api/Command/V1/SurveyPatch.php | 4 +- .../V1/SurveyPatch/OpHandlerAnswer.php | 106 +++++++--- .../SurveyPatch/OpHandlerQuestionCreate.php | 65 +++++-- .../V1/SurveyPatch/OpHandlerQuestionGroup.php | 21 +- .../V1/SurveyPatch/OpHandlerQuestionTrait.php | 61 +++++- .../V1/SurveyPatch/OpHandlerSubQuestion.php | 19 +- .../V1/SurveyPatch/OpHandlerSurveyTrait.php | 15 ++ .../Command/V1/SurveyPatch/PatcherSurvey.php | 183 +++++++++++++----- .../Command/V1/SurveyPatch/TempIdMapItem.php | 34 ++++ .../Command/V1/SurveyPatch/TempIdMapping.php | 61 ++++++ .../Input/TransformerInputAnswer.php | 3 +- .../Input/TransformerInputQuestion.php | 5 +- .../Input/TransformerInputQuestionGroup.php | 3 +- application/libraries/ObjectPatch/Patcher.php | 15 +- .../models/services/QuestionGroupService.php | 13 ++ tests/unit/objectpatch/PatcherTest.php | 6 +- 16 files changed, 499 insertions(+), 115 deletions(-) create mode 100644 application/libraries/Api/Command/V1/SurveyPatch/TempIdMapItem.php create mode 100644 application/libraries/Api/Command/V1/SurveyPatch/TempIdMapping.php diff --git a/application/libraries/Api/Command/V1/SurveyPatch.php b/application/libraries/Api/Command/V1/SurveyPatch.php index 5c3df745190..cb067bbaa33 100644 --- a/application/libraries/Api/Command/V1/SurveyPatch.php +++ b/application/libraries/Api/Command/V1/SurveyPatch.php @@ -65,7 +65,7 @@ public function run(Request $request) PatcherSurvey::class ); try { - $patcher->applyPatch($patch, ['id' => $id]); + $returnedData = $patcher->applyPatch($patch, ['id' => $id]); } catch (ObjectPatchException $e) { return $this->responseFactory->makeErrorBadRequest( $e->getMessage() @@ -73,6 +73,6 @@ public function run(Request $request) } return $this->responseFactory - ->makeSuccess(true); + ->makeSuccess($returnedData); } } diff --git a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerAnswer.php b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerAnswer.php index 690ec3862d5..df0ee7e9062 100644 --- a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerAnswer.php +++ b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerAnswer.php @@ -54,42 +54,50 @@ public function canHandle(OpInterface $op): bool * "id" is the qid, so we are only allowing answers for a single question in * the patch. * Attention: Currently all answers not provided in the patch - * will be deleted by the service. - * All existing answers are needed independently from "update" or "create". + * will be deleted by the service. Doesn't matter if + * create or update was chosen + * You could also mix updated and newly created answers in one patch. + * The difference is that new created answers need "tempId" + * and updated answers need the "aid" provided. + * The OpHandler doesn't care if you choose create or update as "op"!!! + * + * Example for "update": * { * "patch": [{ * "entity": "answer", - * "op": "create", // "update" - * "id": "726", // qid(!) + * "op": "update", + * "id": "809", * "props": { * "0": { - * "code": "XX01", - * "sortOrder": 0, + * "aid": 465, + * "code": "AO01", + * "sortOrder": 2, * "assessmentValue": 0, - * "scaleId": 0, + * "scaleId": 1, * "l10ns": { * "de": { - * "answer": "antwort1", + * "answer": "ANTW1 scale 1", * "language": "de" * }, * "en": { - * "answer": "answer1", + * "answer": "ANS1 scale 1", * "language": "en" * } * } * }, * "1": { - * "code": "YY02", - * "sortOrder": 1, + * "aid": 467, + * "code": "AO02", + * "sortOrder": 3, * "assessmentValue": 0, - * "scaleId": 0, + * "scaleId": 1, * "l10ns": { * "de": { - * "answer": "antwort1.2", + * "answer": "ANTW2 scale 1", * "language": "de" * }, * "en": { - * "answer": "answer1.2", + * "answer": "ANS2 scale 1", * "language": "en" * } * } @@ -99,8 +107,53 @@ public function canHandle(OpInterface $op): bool * ] * } * + * Example for "create": + * { + * "patch": [{ + * "entity": "answer", + * "op": "create", + * "id": "809", + * "props": { + * "0": { + * "tempId": "222", + * "code": "AO11", + * "sortOrder": 4, + * "assessmentValue": 0, + * "scaleId": 1, + * "l10ns": { + * "de": { + * "answer": "ANTW11 scale 1", + * "language": "de" + * }, + * "en": { + * "answer": "ANS11 scale 1", + * "language": "en" + * } + * } + * }, + * "1": { + * "tempId": "223", + * "code": "AO12", + * "sortOrder": 5, + * "assessmentValue": 0, + * "scaleId": 1, + * "l10ns": { + * "de": { + * "answer": "ANTW12 scale 1", + * "language": "de" + * }, + * "en": { + * "answer": "ANS12 scale 1", + * "language": "en" + * } + * } + * } + * } + * } + * ] + * } * @param OpInterface $op - * @return void + * @return array * @throws OpHandlerException * @throws \DI\DependencyException * @throws \DI\NotFoundException @@ -108,21 +161,28 @@ public function canHandle(OpInterface $op): bool * @throws \LimeSurvey\Models\Services\Exception\PermissionDeniedException * @throws \LimeSurvey\Models\Services\Exception\PersistErrorException */ - public function handle(OpInterface $op): void + public function handle(OpInterface $op): array { $question = $this->questionService->getQuestionBySidAndQid( $this->getSurveyIdFromContext($op), $op->getEntityId() ); + $preparedData = $this->prepareAnswers( + $op, + $op->getProps(), + $this->transformerAnswer, + $this->transformerAnswerL10n, + ['answer', 'answerL10n'] + ); $this->answersService->save( $question, - $this->prepareAnswers( - $op, - $op->getProps(), - $this->transformerAnswer, - $this->transformerAnswerL10n, - ['answer', 'answerL10n'] - ) + $preparedData + ); + + return $this->getSubQuestionNewIdMapping( + $question, + $preparedData, + true ); } diff --git a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionCreate.php b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionCreate.php index a4b0e3eac05..0f8c7f5bbe5 100644 --- a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionCreate.php +++ b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionCreate.php @@ -76,26 +76,27 @@ public function canHandle(OpInterface $op): bool * "qid": "0", * "title": "G01Q06", * "type": "1", - * "question_theme_name": "arrays\/dualscale", + * "questionThemeName": "arrays\/dualscale", * "gid": "1", * "mandatory": false, * "relevance": "1", * "encrypted": false, - * "save_as_default": false + * "saveAsDefault": false, + * "tempId": "XXX321" * }, * "questionL10n": { * "en": { * "question": "Array Question", * "help": "Help text" * }, - * "de-informal": { + * "de": { * "question": "Array ger", * "help": "help ger" * } * }, * "attributes": { * "dualscale_headerA": { - * "de-informal": { + * "de": { * "value": "A ger" * }, * "en": { @@ -103,7 +104,7 @@ public function canHandle(OpInterface $op): bool * } * }, * "dualscale_headerB": { - * "de-informal": { + * "de": { * "value": "B ger" * }, * "en": { @@ -122,6 +123,7 @@ public function canHandle(OpInterface $op): bool * "sortOrder": 0, * "assessmentValue": 0, * "scaleId": 0, + * "tempId": "111", * "l10ns": { * "de": { * "answer": "antwort1", @@ -138,6 +140,7 @@ public function canHandle(OpInterface $op): bool * "sortOrder": 1, * "assessmentValue": 0, * "scaleId": 0, + * "tempId": "112", * "l10ns": { * "de": { * "answer": "antwort1.2", @@ -155,8 +158,9 @@ public function canHandle(OpInterface $op): bool * "title": "SQ001", * "sortOrder": 0, * "relevance": "1", + * "tempId": "113", * "l10ns": { - * "de-informal": { + * "de": { * "question": "subger1", * "language": "de" * }, @@ -170,8 +174,9 @@ public function canHandle(OpInterface $op): bool * "title": "SQ002", * "sortOrder": 1, * "relevance": "1", + * "tempId": "114", * "l10ns": { - * "de-informal": { + * "de": { * "question": "subger2", * "language": "de" * }, @@ -188,7 +193,7 @@ public function canHandle(OpInterface $op): bool * } * * @param OpInterface $op - * @return void + * @return array * @throws OpHandlerException * @throws \DI\DependencyException * @throws \DI\NotFoundException @@ -196,16 +201,54 @@ public function canHandle(OpInterface $op): bool * @throws \LimeSurvey\Models\Services\Exception\PermissionDeniedException * @throws \LimeSurvey\Models\Services\Exception\PersistErrorException */ - public function handle(OpInterface $op): void + public function handle(OpInterface $op): array { + $transformedProps = $this->prepareData($op); + if ( + !is_array($transformedProps) || + !array_key_exists( + 'question', + $transformedProps + ) + ) { + throw new OpHandlerException( + sprintf( + 'no question entity provided within props for %s with id "%s"', + $this->entity, + print_r($op->getEntityId(), true) + ) + ); + } + $tempId = $this->extractTempId($transformedProps['question']); $diContainer = \LimeSurvey\DI::getContainer(); $questionService = $diContainer->get( QuestionAggregateService::class ); - $questionService->save( + $question = $questionService->save( $this->getSurveyIdFromContext($op), - $this->prepareData($op) + $transformedProps + ); + + return array_merge( + [ + 'questionsMap' => [ + new TempIdMapItem( + $tempId, + $question->qid, + 'qid' + ) + ] + ], + $this->getSubQuestionNewIdMapping( + $question, + $transformedProps['subquestions'] + ), + $this->getSubQuestionNewIdMapping( + $question, + $transformedProps['answeroptions'], + true + ) ); } diff --git a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionGroup.php b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionGroup.php index c922b54fe37..cc3ca724f4d 100644 --- a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionGroup.php +++ b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionGroup.php @@ -70,19 +70,19 @@ public function canHandle(OpInterface $op): bool * @param OpInterface $op * @throws OpHandlerException */ - public function handle(OpInterface $op): void + public function handle(OpInterface $op): array { switch (true) { case $this->isUpdateOperation: $this->update($op); break; case $this->isCreateOperation: - $this->create($op); - break; + return $this->create($op); case $this->isDeleteOperation: $this->delete($op); break; } + return []; } /** @@ -210,6 +210,7 @@ private function update(OpInterface $op) * "op": "create", * "props":{ * "questionGroup": { + * "tempId": 777, * "randomizationGroup": "", * "gRelevance": "" * }, @@ -235,19 +236,27 @@ private function update(OpInterface $op) * * @param OpInterface $op * @param QuestionGroupService $groupService - * @return void + * @return array * @throws OpHandlerException * @throws \LimeSurvey\Models\Services\Exception\NotFoundException * @throws \LimeSurvey\Models\Services\Exception\PersistErrorException */ - private function create(OpInterface $op) + private function create(OpInterface $op): array { $surveyId = $this->getSurveyIdFromContext($op); $transformedProps = $this->getTransformedProps($op); - $this->questionGroupService->createGroup( + $tempId = $this->extractTempId($transformedProps['questionGroup']); + $questionGroup = $this->questionGroupService->createGroup( $surveyId, $transformedProps ); + $questionGroup->refresh(); + return [ + 'questionGroupsMap' => [ + 'tempId' => $tempId, + 'gid' => $questionGroup->gid + ] + ]; } /** diff --git a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionTrait.php b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionTrait.php index 7a3118a680c..029be0758a6 100644 --- a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionTrait.php +++ b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerQuestionTrait.php @@ -9,6 +9,7 @@ use LimeSurvey\Api\Command\V1\Transformer\Input\TransformerInputQuestionL10ns; use LimeSurvey\ObjectPatch\Op\OpInterface; use LimeSurvey\ObjectPatch\OpHandler\OpHandlerException; +use Question; trait OpHandlerQuestionTrait { @@ -32,12 +33,12 @@ public function prepareAnswers( $preparedAnswers = []; if (is_array($data)) { foreach ($data as $index => $answer) { - $transformedAnswer = $transformerAnswer->transform( + $tfAnswer = $transformerAnswer->transform( $answer ); $this->checkRequiredData( $op, - $transformedAnswer, + $tfAnswer, 'answers', $additionalRequiredEntities ); @@ -47,7 +48,7 @@ public function prepareAnswers( $answer ) && is_array($answer['l10ns']) ) { - $transformedAnswer['answeroptionl10n'] = $this->prepareAnswerL10n( + $tfAnswer['answeroptionl10n'] = $this->prepareAnswerL10n( $op, $answer['l10ns'], $transformerAnswerL10n, @@ -59,9 +60,13 @@ public function prepareAnswers( */ $scaleId = array_key_exists( 'scale_id', - $transformedAnswer - ) ? $transformedAnswer['scale_id'] : 0; - $preparedAnswers[$index][$scaleId] = $transformedAnswer; + $tfAnswer + ) ? $tfAnswer['scale_id'] : 0; + $index = array_key_exists( + 'aid', + $tfAnswer + ) ? $tfAnswer['aid'] : $index; + $preparedAnswers[$index][$scaleId] = $tfAnswer; } } return $preparedAnswers; @@ -288,4 +293,48 @@ private function getScaleIdFromData(array $questionData) $questionData ) ? (int)$questionData['scale_id'] : 0; } + + /** + * Maps the tempIds of new subquestions or answers to the real ids. + * @param Question $question + * @param array $data + * @param bool $answers + * @return array + */ + private function getSubQuestionNewIdMapping( + Question $question, + array $data, + bool $answers = false + ): array { + $tempIds = []; + $mapping = []; + $title = $answers ? 'code' : 'title'; + $object = $answers ? 'answers' : 'subquestions'; + $idField = $answers ? 'aid' : 'qid'; + foreach ($data as $subQueDataArray) { + foreach ($subQueDataArray as $subQueData) { + if ( + isset($subQueData['tempId']) + && isset($subQueData[$title]) + ) { + $tempIds[$subQueData[$title]] = $subQueData['tempId']; + } + } + } + if (count($tempIds) > 0) { + $question->refresh(); + foreach ($question->$object as $subquestion) { + if (array_key_exists($subquestion->$title, $tempIds)) { + $mapping[$object . 'Map'][] = [ + new TempIdMapItem( + $tempIds[$subquestion->$title], + $subquestion->$idField, + $idField + ) + ]; + } + } + } + return $mapping; + } } diff --git a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerSubQuestion.php b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerSubQuestion.php index 9fb96fdea32..95362547bdc 100644 --- a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerSubQuestion.php +++ b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerSubQuestion.php @@ -2,6 +2,7 @@ namespace LimeSurvey\Api\Command\V1\SurveyPatch; +use Question; use LimeSurvey\Api\Command\V1\Transformer\Input\{ TransformerInputQuestion, TransformerInputQuestionL10ns @@ -116,6 +117,7 @@ public function canHandle(OpInterface $op): bool * "id": 722, * "props": { * "0": { + * "tempId": "456789", * "title": "SQ011", * "l10ns": { * "de": { @@ -130,6 +132,7 @@ public function canHandle(OpInterface $op): bool * }, * "1": { * "title": "SQ012", + * "tempId": "345678", * "l10ns": { * "de": { * "question": "germanized2", @@ -152,7 +155,7 @@ public function canHandle(OpInterface $op): bool * @throws PermissionDeniedException * @throws PersistErrorException */ - public function handle(OpInterface $op): void + public function handle(OpInterface $op): array { $surveyId = $this->getSurveyIdFromContext($op); $this->questionAggregateService->checkUpdatePermission($surveyId); @@ -166,16 +169,20 @@ public function handle(OpInterface $op): void //be careful here! if for any reason the incoming data is not prepared //as it should, all existing subquestions will be deleted! if (count($preparedData) === 0) { - throw new OpHandlerException('No data to create or update a subquestion'); + throw new OpHandlerException( + 'No data to create or update a subquestion' + ); } $questionId = $op->getEntityId(); + $question = $this->questionService->getQuestionBySidAndQid( + $surveyId, + $questionId + ); $this->subQuestionsService->save( - $this->questionService->getQuestionBySidAndQid( - $surveyId, - $questionId - ), + $question, $preparedData ); + return $this->getSubQuestionNewIdMapping($question, $preparedData); } /** diff --git a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerSurveyTrait.php b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerSurveyTrait.php index 4f35e91bab0..7c0c2afe5a7 100644 --- a/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerSurveyTrait.php +++ b/application/libraries/Api/Command/V1/SurveyPatch/OpHandlerSurveyTrait.php @@ -68,4 +68,19 @@ public function getTransformedLanguageProps( } return $dataSet; } + + /** + * returns and removes tempId from dataset + * @param array $dataSet + * @return int|mixed + */ + public function extractTempId(array &$dataSet) + { + if (isset($dataSet['tempId'])) { + $tempId = $dataSet['tempId']; + unset($dataSet['tempId']); + return $tempId; + } + return 0; + } } diff --git a/application/libraries/Api/Command/V1/SurveyPatch/PatcherSurvey.php b/application/libraries/Api/Command/V1/SurveyPatch/PatcherSurvey.php index ec5717a3871..030b3a533e9 100644 --- a/application/libraries/Api/Command/V1/SurveyPatch/PatcherSurvey.php +++ b/application/libraries/Api/Command/V1/SurveyPatch/PatcherSurvey.php @@ -2,68 +2,149 @@ namespace LimeSurvey\Api\Command\V1\SurveyPatch; -use Answer; -use LimeSurvey\ObjectPatch\{ - OpHandler\OpHandlerActiveRecordUpdate, +use LimeSurvey\ObjectPatch\{ObjectPatchException, + Op\OpStandard, + OpHandler\OpHandlerException, Patcher }; -use LimeSurvey\Api\Command\V1\Transformer\Input\{ - TransformerInputAnswer -}; -use DI\FactoryInterface; use Psr\Container\ContainerInterface; class PatcherSurvey extends Patcher { + protected TempIdMapping $tempIdMapping; + /** * Constructor * - * @param FactoryInterface $diFactory * @param ContainerInterface $diContainer + * @param TempIdMapping $tempIdMapping + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function __construct( + ContainerInterface $diContainer, + TempIdMapping $tempIdMapping + ) { + $this->tempIdMapping = $tempIdMapping; + $this->addOpHandler( + $diContainer->get( + OpHandlerSurveyUpdate::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerLanguageSettingsUpdate::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerQuestionGroup::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerQuestionGroupL10n::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerQuestionDelete::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerQuestionCreate::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerQuestionUpdate::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerQuestionL10nUpdate::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerAnswer::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerQuestionAttributeUpdate::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerQuestionGroupReorder::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerSubquestionDelete::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerAnswerDelete::class + ) + ); + $this->addOpHandler( + $diContainer->get( + OpHandlerSubQuestion::class + ) + ); + } + + /** + * Apply patch + * + * @param ?mixed $patch + * @param ?array $context + * @return array + * @throws ObjectPatchException + * @throws OpHandlerException + */ + public function applyPatch($patch, $context = []): array + { + if (is_array($patch) && !empty($patch)) { + foreach ($patch as $patchOpData) { + $op = OpStandard::factory( + $patchOpData['entity'] ?? '', + $patchOpData['op'] ?? '', + $patchOpData['id'] ?? '', + $patchOpData['props'] ?? [], + $context ?? [] + ); + $this->tempIdMapping->incrementOperationsApplied(); + $handleResponse = $this->handleOp($op); + foreach ($handleResponse as $groupName => $mappingItem) { + $this->addTempIdMapItem($mappingItem, $groupName); + } + } + } + return $this->tempIdMapping->getMappingResponseObject(); + } + + /** + * Recursive function to extract TempIdMapItems from the $mappingItem + * @param TempIdMapItem|array $mappingItem + * @param string $groupName + * @return void + * @throws \LimeSurvey\ObjectPatch\OpHandler\OpHandlerException */ - public function __construct(FactoryInterface $diFactory, ContainerInterface $diContainer) + private function addTempIdMapItem($mappingItem, string $groupName) { - $this->addOpHandler($diContainer->get( - OpHandlerSurveyUpdate::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerLanguageSettingsUpdate::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerQuestionGroup::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerQuestionGroupL10n::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerQuestionDelete::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerQuestionCreate::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerQuestionUpdate::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerQuestionL10nUpdate::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerAnswer::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerQuestionAttributeUpdate::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerQuestionGroupReorder::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerSubquestionDelete::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerAnswerDelete::class - )); - $this->addOpHandler($diContainer->get( - OpHandlerSubQuestion::class - )); + if ($mappingItem instanceof TempIdMapItem) { + $this->tempIdMapping->addTempIdMapItem( + $mappingItem, + $groupName + ); + } else { + foreach ($mappingItem as $item) { + $this->addTempIdMapItem($item, $groupName); + } + } } } diff --git a/application/libraries/Api/Command/V1/SurveyPatch/TempIdMapItem.php b/application/libraries/Api/Command/V1/SurveyPatch/TempIdMapItem.php new file mode 100644 index 00000000000..7b0d339d587 --- /dev/null +++ b/application/libraries/Api/Command/V1/SurveyPatch/TempIdMapItem.php @@ -0,0 +1,34 @@ +tempId = $tempId; + $this->id = $id; + $this->field = $field; + } +} diff --git a/application/libraries/Api/Command/V1/SurveyPatch/TempIdMapping.php b/application/libraries/Api/Command/V1/SurveyPatch/TempIdMapping.php new file mode 100644 index 00000000000..6d31ffa14eb --- /dev/null +++ b/application/libraries/Api/Command/V1/SurveyPatch/TempIdMapping.php @@ -0,0 +1,61 @@ +itemGroupNames)) { + throw new OpHandlerException( + sprintf( + 'Invalid map item group name "%s"', + $itemGroupName + ) + ); + } + $this->mapItems[$itemGroupName][] = $tempIdMapItem; + } + + /** + * Returns the whole response array including all the added tempId mappings + * and the number of applied operations. + * @return array + */ + public function getMappingResponseObject(): array + { + return array_merge( + [ + 'operationsApplied' => $this->operationsApplied, + ], + $this->mapItems + ); + } + + + public function incrementOperationsApplied(): void + { + $this->operationsApplied++; + } +} diff --git a/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputAnswer.php b/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputAnswer.php index 67168f062ce..ed3e1b5214f 100644 --- a/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputAnswer.php +++ b/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputAnswer.php @@ -15,7 +15,8 @@ public function __construct() 'code' => true, 'sortOrder' => ['key' => 'sortorder', 'type' => 'int'], 'assessmentValue' => ['key' => 'assessment_value', 'type' => 'int'], - 'scaleId' => ['key' => 'scale_id', 'type' => 'int'] + 'scaleId' => ['key' => 'scale_id', 'type' => 'int'], + 'tempId' => true ]); } } diff --git a/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputQuestion.php b/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputQuestion.php index 1be87858063..4ed5a0093aa 100644 --- a/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputQuestion.php +++ b/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputQuestion.php @@ -28,10 +28,13 @@ public function __construct() 'scaleId' => ['key' => 'scale_id', 'type' => 'int'], 'sameDefault' => ['key' => 'same_default', 'formatter' => $formatterYn], 'questionThemeName' => 'question_theme_name', + 'saveAsDefault' => 'save_as_default', + 'clearDefault' => 'clear_default', 'moduleName' => 'modulename', 'gid' => ['type' => 'int'], 'relevance' => true, - 'sameScript' => ['key' => 'same_script', 'formatter' => $formatterYn] + 'sameScript' => ['key' => 'same_script', 'formatter' => $formatterYn], + 'tempId' => true ]); } } diff --git a/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputQuestionGroup.php b/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputQuestionGroup.php index b84b518fa33..f8653154ebb 100644 --- a/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputQuestionGroup.php +++ b/application/libraries/Api/Command/V1/Transformer/Input/TransformerInputQuestionGroup.php @@ -14,7 +14,8 @@ public function __construct() 'groupOrder' => ['key' => 'group_order', 'type' => 'int'], 'sortOrder' => ['key' => 'group_order', 'type' => 'int'], 'randomizationGroup' => 'randomization_group', - 'gRelevance' => 'grelevance' + 'gRelevance' => 'grelevance', + 'tempId' => true ]); } } diff --git a/application/libraries/ObjectPatch/Patcher.php b/application/libraries/ObjectPatch/Patcher.php index aac5d812268..df4be6861f5 100644 --- a/application/libraries/ObjectPatch/Patcher.php +++ b/application/libraries/ObjectPatch/Patcher.php @@ -15,8 +15,9 @@ class Patcher * * @throws ObjectPatchException */ - public function applyPatch($patch, $context = []): int + public function applyPatch($patch, $context = []): array { + $returnedData = []; $operationsApplied = 0; if (is_array($patch) && !empty($patch)) { foreach ($patch as $patchOpData) { @@ -27,11 +28,11 @@ public function applyPatch($patch, $context = []): int $patchOpData['props'] ?? null, $context ?? null ); - $this->handleOp($op); + $returnedData[] = $this->handleOp($op); $operationsApplied++; } } - return $operationsApplied; + return ['operationsApplied' => $operationsApplied, $returnedData]; } /** @@ -47,17 +48,20 @@ public function addOpHandler(OpHandlerInterface $opHandler): void * Apply operation * * @param OpInterface $op + * @return array * @throws ObjectPatchException */ - private function handleOp(OpInterface $op): void + public function handleOp(OpInterface $op): array { $handled = false; + $returnedData = []; foreach ($this->opHandlers as $opHandler) { if (!$opHandler->canHandle($op)) { continue; } if ($opHandler->isValidPatch($op)) { - $opHandler->handle($op); + $return = $opHandler->handle($op); + $returnedData = is_array($return) ? $return : []; } else { throw new ObjectPatchException( sprintf( @@ -80,5 +84,6 @@ private function handleOp(OpInterface $op): void ) ); } + return $returnedData; } } diff --git a/application/models/services/QuestionGroupService.php b/application/models/services/QuestionGroupService.php index cb121da2adf..8ccb72ac1bc 100644 --- a/application/models/services/QuestionGroupService.php +++ b/application/models/services/QuestionGroupService.php @@ -484,6 +484,7 @@ public function updateQuestionGroup( public function newQuestionGroup(int $surveyId, array $aQuestionGroupData = null) { $survey = $this->getSurvey($surveyId); + $this->refreshModels(); $aQuestionGroupData = array_merge([ 'sid' => $survey->sid, ], $aQuestionGroupData); @@ -535,6 +536,18 @@ private function getSurvey(int $surveyId) 'Survey does not exist', ); } + $survey->refresh(); return $survey; } + + /** + * Resets questionGroup model for cases + * when multiple groups are created simultaneously + * @return void + */ + private function refreshModels() + { + $this->modelQuestionGroup->unsetAttributes(); + $this->modelQuestionGroup->setIsNewRecord(true); + } } diff --git a/tests/unit/objectpatch/PatcherTest.php b/tests/unit/objectpatch/PatcherTest.php index c9c45d218e5..bc67bfd0767 100644 --- a/tests/unit/objectpatch/PatcherTest.php +++ b/tests/unit/objectpatch/PatcherTest.php @@ -39,7 +39,8 @@ public function testApplyPatchExecuteMatchedOperation() $patcher = new Patcher(); $patcher->addOpHandler($opHandler); - $operationsApplied = $patcher->applyPatch($patch); + $returnedData = $patcher->applyPatch($patch); + $operationsApplied = $returnedData['operationsApplied']; $this->assertEquals(1, $operationsApplied); } @@ -61,7 +62,8 @@ public function testApplyPatchThrowsObjectPatchExceptionOnMissingOpHandler() ]; $patcher = new Patcher(); - $operationsApplied = $patcher->applyPatch($patch); + $returnedData = $patcher->applyPatch($patch); + $operationsApplied = $returnedData['operationsApplied']; $this->assertEquals(0, $operationsApplied); }