From 166fb140f1f4c6a5cf28be5b1e238696b436dc00 Mon Sep 17 00:00:00 2001 From: markhuot Date: Wed, 11 Oct 2023 00:26:13 -0400 Subject: [PATCH] better draft handling, save handling, and delete support --- src/actions/DeleteComponent.php | 20 +++ src/actions/DuplicateComponentTree.php | 201 ++++++++++++++++++----- src/controllers/ComponentsController.php | 75 +-------- src/fields/Keystone.php | 20 ++- src/models/Component.php | 8 +- src/resources/keystone.js | 5 +- src/templates/edit.twig | 4 + tests/ElementEventsTest.php | 76 +++++++++ tests/templates/ElementEventsTest.php | 22 --- 9 files changed, 292 insertions(+), 139 deletions(-) create mode 100644 src/actions/DeleteComponent.php create mode 100644 tests/ElementEventsTest.php delete mode 100644 tests/templates/ElementEventsTest.php diff --git a/src/actions/DeleteComponent.php b/src/actions/DeleteComponent.php new file mode 100644 index 0000000..b977ccf --- /dev/null +++ b/src/actions/DeleteComponent.php @@ -0,0 +1,20 @@ + new Expression('sortOrder - 1')], ['and', + ['elementId' => $component->elementId], + ['fieldId' => $component->fieldId], + ['>', 'sortOrder', $component->sortOrder] + ]); + + $component->delete(); + } +} diff --git a/src/actions/DuplicateComponentTree.php b/src/actions/DuplicateComponentTree.php index 684510b..cc946ed 100644 --- a/src/actions/DuplicateComponentTree.php +++ b/src/actions/DuplicateComponentTree.php @@ -14,63 +14,174 @@ class DuplicateComponentTree { static array $mapping = []; - public function handle(ElementInterface $source, ElementInterface $destination, Keystone $field) + /** + * The goal of this method is to performantly scan through two sets of components and copy the + * components from the source in to the destination. We do this by looping over the source and + * destination components in a single loop. We compare the ordered IDs and either create, update + * or delete them out of the destination. + * + * There's _a lot_ of extra code here just to make sure we're not deleting components that don't + * actually need to be deleted. Instead we're just bumping their "dateUpdated" so we get accurate + * tracking of changes. + * + * source: [1,4,5,7] + * destination: [1,2,3,4,6,7,8] + * 1==1 // nothing, advance both + * 2<4 // delete 2, advance destination + * 3<4 // delete 3, advance destination + * 4==4 // advance both + * 6>5 // insert five in to destination, advance source + * 6<7 // delete six, advance source + * 7==7 // advance both + * 8==null // delete 8, advance destination + * + * destination: [1,2] + * source: [3,4] + * 1<3 // delete 1, advance source + * 2<3 // delete 2, advance source + * null!=3 // insert 3, advance source + * null!=4 // insert 4, advance source + * + */ + public function handle(ElementInterface $sourceElement, ElementInterface $destinationElement, Keystone $field) { + $sourceQuery = Component::find()->where([ + 'elementId' => $sourceElement->id, + 'fieldId' => $field->id, + ])->orderBy(['id' => 'asc']); + $sourceBatch = $sourceQuery->each(); + $sourceBatch->next(); + + $destinationQuery = Component::find()->where([ + 'elementId' => $destinationElement->id, + 'fieldId' => $field->id, + ])->orderBy(['id' => 'asc']); + $destinationBatch = $destinationQuery->each(); + $destinationBatch->next(); + + while (true) { + /** @var Component $source */ + $source = $sourceBatch->current(); + /** @var Component $destination */ + $destination = $destinationBatch->current(); + + // if we've continued on past the end of our lists we can stop here + if ($source === false && $destination === false) { + break; + } + + else if ($source !== false && $destination === false) { + // insert source + \markhuot\craftpest\helpers\test\dump(2); + + $new = new Component; + $new->id = $source->id; + $new->elementId = $destinationElement->id; + $new->fieldId = $field->id; + $new->dataId = $source->dataId; + $new->sortOrder = $source->sortOrder; + $new->path = $source->path; + $new->level = $source->level; + $new->slot = $source->slot; + $new->dateCreated = DateTimeHelper::now(); + $new->dateUpdated = DateTimeHelper::now(); + $new->uid = StringHelper::UUID(); + $new->save(); + + $sourceBatch->next(); + } + + else if ($source === false && $destination !== false) { + // delete destination + \markhuot\craftpest\helpers\test\dump(3); + Component::deleteAll([ + 'id' => $destination->id, + 'elementId' => $destinationElement->id, + 'fieldId' => $field->id, + ]); + + + $destinationBatch->next(); + } + + // if the IDs are the same we can update in place + else if ($source->id === $destination->id) { + \markhuot\craftpest\helpers\test\dump(4); + $destination->dataId = $source->dataId; + $destination->sortOrder = $source->sortOrder; + $destination->path = $source->path; + $destination->level = $source->level; + $destination->slot = $source->slot; + $destination->save(); + + $sourceBatch->next(); + $destinationBatch->next(); + } + + // if the destination ID is missing from the source, delete it + else if ($source->id > $destination->id) { + \markhuot\craftpest\helpers\test\dump(5); + Component::deleteAll([ + 'id' => $destination->id, + 'elementId' => $destinationElement->id, + 'fieldId' => $field->id, + ]); + + $destinationBatch->next(); + } + + // if the source ID is missing from the destination, insert it + else if ($source->id < $destination->id) { + \markhuot\craftpest\helpers\test\dump(6); + $new = new Component; + $new->id = $source->id; + $new->elementId = $destinationElement->id; + $new->fieldId = $field->id; + $new->dataId = $source->dataId; + $new->sortOrder = $source->sortOrder; + $new->path = $source->path; + $new->level = $source->level; + $new->slot = $source->slot; + $new->dateCreated = DateTimeHelper::now(); + $new->dateUpdated = DateTimeHelper::now(); + $new->uid = StringHelper::UUID(); + $new->save(); + + $sourceBatch->next(); + } + } + } + + /** + * @deprecated + */ + public function simpleHandle(ElementInterface $source, ElementInterface $destination, Keystone $field) + { + // Delete existing components since the duplicated components will replace them + Craft::$app->db->createCommand()->delete(Table::COMPONENTS, [ + 'elementId' => $destination->id, + 'fieldId' => $field->id, + ])->execute(); + $query = Component::find()->where([ 'elementId' => $source->id, 'fieldId' => $field->id, - ])->orderBy(['path' => 'asc']); + ])->orderBy(['path' => 'asc', 'sortOrder' => 'asc']); - foreach ($query->each() as $component) { + foreach ($query->each() as $existing) { $duplicate = new Component; - $duplicate->id = $component->id; + $duplicate->id = $existing->id; $duplicate->elementId = $destination->id; $duplicate->fieldId = $field->id; - $duplicate->dataId = $component->dataId; - $duplicate->sortOrder = $component->sortOrder; - $duplicate->path = $this->remapPath($component->path); - $duplicate->level = $component->level; - $duplicate->slot = $component->slot; + $duplicate->dataId = $existing->dataId; + $duplicate->sortOrder = $existing->sortOrder; + $duplicate->path = $existing->path; + $duplicate->level = $existing->level; + $duplicate->slot = $existing->slot; $duplicate->dateCreated = DateTimeHelper::now(); $duplicate->dateUpdated = DateTimeHelper::now(); $duplicate->uid = StringHelper::UUID(); $duplicate->save(); - - static::$mapping[$component->id] = $duplicate->id; } - - // I'd love to use INSERT INTO components (elementId, fieldId, ...) SELECT * FROM components WHERE elementId= and fieldId= - // but Craft breaks this because they subclass the db->createCommand() and don't allow you to pass - // a query as the second parameter. They look for $columns[dateCreated] on that second param which - // works when setting raw values but not if a query is passed because it tries to $query[dateCreated] - // and Query isn't array-accessible. - // - // $query = Component::find()->select(['fieldId', 'componentId', 'sortOrder', 'path', 'level', 'slot'])->where([ - // 'elementId' => $source->id, - // 'fieldId' => $field->id, - // ]); - // $params = [ - // 'elementId' => $source->id, - // 'fieldId' => $field->id, - // ]; - // Craft::$app->db->createCommand(Craft::$app->db->getQueryBuilder()->insert(Table::COMPONENTS, $query, $params))->execute(); - // Craft::$app->db->createCommand()->insert(Table::COMPONENTS, $query); - } - - protected function remapPath(?string $path) - { - if ($path === null) { - return null; - } - - return collect(explode('/', $path)) - ->map(function ($segment) use ($path) { - if (! isset(static::$mapping[$segment])) { - throw new \RuntimeException('Could not remap ' . $path . ' because ' . $segment . ' could not be found'); - } - - return static::$mapping[$segment]; - }) - ->join('/'); } } diff --git a/src/controllers/ComponentsController.php b/src/controllers/ComponentsController.php index d947003..6155813 100644 --- a/src/controllers/ComponentsController.php +++ b/src/controllers/ComponentsController.php @@ -56,6 +56,7 @@ public function actionEdit() ->tabs([ ['label' => 'Content', 'url' => '#tab-content'], ['label' => 'Styles', 'url' => '#tab-styles'], + ['label' => 'Admin', 'url' => '#tab-admin'], ]) ->action('keystone/components/update') ->contentTemplate('keystone/edit', [ @@ -65,80 +66,22 @@ public function actionEdit() public function actionUpdate() { -// $id = $this->request->getRequiredBodyParam('id'); -// $data = $this->request->getBodyParam('fields', []); -// -// $component = Component::findOne(['id' => $id]); -// $component->data->merge($data); -// $component->data->save(); -// -// $element = Craft::$app->elements->getElementById($component->elementId); -// $field = Craft::$app->fields->getFieldById($component->fieldId); - return $this->asSuccess('Component saved', [ + 'action' => 'edit-component', 'id' => $this->request->getRequiredBodyParam('id'), 'elementId' => $this->request->getRequiredBodyParam('elementId'), + 'fieldId' => $this->request->getRequiredBodyParam('fieldId'), 'fields' => $this->request->getRequiredBodyParam('fields'), -// 'elementId' => $component->elementId, -// 'fieldId' => $component->fieldId, -// 'fieldHandle' => $field->handle, -// 'fieldHtml' => $field->getInputHtml(null, $element), ]); } - public function actionMove() + public function actionDelete() { - $sourceId = $this->request->getRequiredBodyParam('source'); - $source = Component::findOne(['id' => $sourceId]); - $targetId = $this->request->getRequiredBodyParam('target'); - $target = Component::findOne(['id' => $targetId]); - $position = $this->request->getRequiredBodyParam('position'); - - // remove ourselves from the list - Component::updateAll([ - 'sortOrder' => new Expression('sortOrder - 1') - ], ['and', - ['=', 'elementId', $source->elementId], - ['=', 'fieldId', $source->fieldId], - ['path' => $source->path], - ['>', 'sortOrder', $source->sortOrder] - ]); - - // Refresh our target to get the updated/correct sortOrder - $target->refresh(); - - // make room for the insertion - if ($position === 'above') { - Component::updateAll([ - 'sortOrder' => new Expression('sortOrder + 1') - ], ['and', - ['=', 'elementId', $target->elementId], - ['=', 'fieldId', $target->fieldId], - ['path' => $target->path], - ['>=', 'sortOrder', $target->sortOrder] - ]); - } - if ($position === 'below') - { - Component::updateAll([ - 'sortOrder' => new Expression('sortOrder + 1') - ], ['and', - ['=', 'elementId', $target->elementId], - ['=', 'fieldId', $target->fieldId], - ['path' => $target->path], - ['>', 'sortOrder', $target->sortOrder] - ]); - } - - $source->path = $target->path; - $source->sortOrder = $position == 'above' ? $target->sortOrder : $target->sortOrder + 1; - $source->save(); - - $element = Craft::$app->elements->getElementById($source->elementId); - $field = Craft::$app->fields->getFieldById($source->fieldId); - - return $this->asSuccess('Component moved', [ - 'fieldHtml' => $field->getInputHtml(null, $element), + return $this->asSuccess('Component deleted', [ + 'action' => 'delete-component', + 'id' => $this->request->getRequiredBodyParam('id'), + 'elementId' => $this->request->getRequiredBodyParam('elementId'), + 'fieldId' => $this->request->getRequiredBodyParam('fieldId'), ]); } diff --git a/src/fields/Keystone.php b/src/fields/Keystone.php index 6673025..232d42a 100644 --- a/src/fields/Keystone.php +++ b/src/fields/Keystone.php @@ -7,6 +7,7 @@ use craft\base\Field; use craft\web\View; use markhuot\keystone\actions\AddComponent; +use markhuot\keystone\actions\DeleteComponent; use markhuot\keystone\actions\DuplicateComponentTree; use markhuot\keystone\actions\EditComponentData; use markhuot\keystone\actions\GetComponentType; @@ -66,12 +67,19 @@ public function normalizeValueFromRequest(mixed $value, ?ElementInterface $eleme } if ($payload['name'] === 'edit-component') { - ['id' => $id, 'elementId' => $elementId, 'fields' => $fields] = $payload; - $component = Component::findOne(['id' => $id, 'elementId' => $elementId]); + ['id' => $id, 'elementId' => $elementId, 'fieldId' => $fieldId, 'fields' => $fields] = $payload; + $component = Component::findOne(['id' => $id, 'elementId' => $elementId, 'fieldId' => $fieldId]); (new EditComponentData)->handle($component, $fields); OverrideDraftResponseWithFieldHtml::override($element, $this); } + if ($payload['name'] === 'delete-component') { + ['id' => $id, 'fieldId' => $fieldId] = $payload; + $component = Component::findOne(['id' => $id, 'elementId' => $element->id, 'fieldId' => $fieldId]); + (new DeleteComponent)->handle($component); + OverrideDraftResponseWithFieldHtml::override($element, $this); + } + return null; } @@ -93,6 +101,12 @@ protected function inputHtml(mixed $value, ?ElementInterface $element = null): s */ public function normalizeValue(mixed $value, ?ElementInterface $element = null): mixed { + // If the value has already been normalized, return it + if ($value instanceof Component) { + return $value; + } + + // Otherwise fetch the components out of the database return $this->getFragment($element); } @@ -103,7 +117,7 @@ public function afterElementSave(ElementInterface $element, bool $isNew): void { // If we're duplicating an element to create a draft or revision, duplicate the component // tree as well - if ($element->duplicateOf && $isNew) { + if ($element->duplicateOf && ($element->isNewForSite || $element->updatingFromDerivative)) { (new DuplicateComponentTree)->handle($element->duplicateOf, $element, $this); } diff --git a/src/models/Component.php b/src/models/Component.php index 9c95c31..ae6752b 100644 --- a/src/models/Component.php +++ b/src/models/Component.php @@ -9,6 +9,12 @@ use markhuot\keystone\db\Table; use Twig\Markup; +/** + * @property int $id + * @property int $elementId + * @property int $fieldId + * @property int $dataId + */ class Component extends ActiveRecord { protected array $accessed = []; @@ -26,7 +32,7 @@ public function getData() public static function primaryKey() { - return ['id', 'elementId']; + return ['id', 'elementId', 'fieldId']; } public function __get($name) diff --git a/src/resources/keystone.js b/src/resources/keystone.js index 4421950..0099a7d 100644 --- a/src/resources/keystone.js +++ b/src/resources/keystone.js @@ -20,9 +20,10 @@ document.addEventListener('click', event => { input.type = 'hidden'; input.name = 'fields[' + handle + '][action]'; input.value = JSON.stringify({ - name: 'edit-component', + name: event.response.data.action, id: event.response.data.id, elementId: event.response.data.elementId, + fieldId: event.response.data.fieldId, fields: event.response.data.fields, }); Craft.cp.$primaryForm.get(0).appendChild(input); @@ -118,7 +119,7 @@ document.addEventListener('dragover', event => { } const row = el.querySelector('[data-draggable-row]'); - const mouse = event.pageY; + const mouse = event.clientY; const { top, height, left } = row.getBoundingClientRect(); const position = (mouse < top + (height / 2)) ? 'above' : 'below'; diff --git a/src/templates/edit.twig b/src/templates/edit.twig index 1f7f1aa..fcfb8a4 100644 --- a/src/templates/edit.twig +++ b/src/templates/edit.twig @@ -3,6 +3,7 @@
{{ hiddenInput('id', component.id) }} {{ hiddenInput('elementId', component.elementId) }} + {{ hiddenInput('fieldId', component.fieldId) }} {% namespace 'fields' %} {% for field in component.getType().getSchema().fields %} @@ -23,3 +24,6 @@ {# ], value: component.data._styles.display|default('block') })}}#} {% endnamespace %}
+ diff --git a/tests/ElementEventsTest.php b/tests/ElementEventsTest.php new file mode 100644 index 0000000..24b9e7e --- /dev/null +++ b/tests/ElementEventsTest.php @@ -0,0 +1,76 @@ +seed = function (array $sourceIds=[], array $destinationIds=[]) { + $source = new \craft\elements\Entry; + $source->id = 1; + $destination = new \craft\elements\Entry; + $destination->id = 2; + $field = new \markhuot\keystone\fields\Keystone; + $field->id = 1; + $data = new \markhuot\keystone\models\ComponentData; + $data->type = 'keystone/test'; + $data->save(); + + foreach ($sourceIds as $sortOrder => $sourceId) { + $sourceId = is_array($sourceId) ? $sourceId : ['id' => $sourceId]; + Component::factory()->create(['elementId' => $source->id, 'fieldId' => $field->id, 'dataId' => $data->id, 'sortOrder' => $sortOrder, ...$sourceId]); + } + foreach ($destinationIds as $sortOrder => $destinationId) { + $destinationId = is_array($destinationId) ? $destinationId : ['id' => $destinationId]; + Component::factory()->create(['elementId' => $destination->id, 'fieldId' => $field->id, 'dataId' => $data->id, 'sortOrder' => $sortOrder, ...$destinationId]); + } + + return [$source, $destination, $field]; + }; +}); + +it('inserts components during duplicate', function () { + [$source, $destination, $field] = ($this->seed)([1,2], []); + (new \markhuot\keystone\actions\DuplicateComponentTree)->handle($source, $destination, $field); + + $duplicates = Component::find()->where(['elementId' => $destination->id])->orderBy(['path' => 'asc'])->collect(); + expect($duplicates)->toHaveCount(2); + expect($duplicates[0])->id->toBe(1); + expect($duplicates[1])->id->toBe(2); +}); + +it('deletes components during duplicate', function () { + [$source, $destination, $field] = ($this->seed)([1], [1,2]); + (new \markhuot\keystone\actions\DuplicateComponentTree)->handle($source, $destination, $field); + + $duplicates = Component::find()->where(['elementId' => '2'])->orderBy(['path' => 'asc'])->collect(); + expect($duplicates)->toHaveCount(1); + expect($duplicates)->first()->id->toBe(1); +}); + +it('updates components during duplicate', function() { + [$source, $destination, $field] = ($this->seed)( + [['id' => 1, 'sortOrder' => 1], ['id' => 2, 'sortOrder' => 0]], + [['id' => 1, 'sortOrder' => 0], ['id' => 2, 'sortOrder' => 1]], + ); + (new \markhuot\keystone\actions\DuplicateComponentTree)->handle($source, $destination, $field); + + $duplicates = Component::find()->where(['elementId' => '2'])->orderBy(['path' => 'asc'])->collect(); + expect($duplicates)->toHaveCount(2); + expect($duplicates[0])->sortOrder->toBe(1); + expect($duplicates[1])->sortOrder->toBe(0); +}); + +it('deletes and updates components during duplicate', function() { + [$source, $destination, $field] = ($this->seed)( + [['id' => 8, 'sortOrder' => 1], ['id' => 9, 'sortOrder' => 0]], + [['id' => 7, 'sortOrder' => 0], ['id' => 8, 'sortOrder' => 1]], + ); + (new \markhuot\keystone\actions\DuplicateComponentTree)->handle($source, $destination, $field); + + $duplicates = Component::find()->where(['elementId' => '2'])->orderBy(['path' => 'asc'])->collect(); + expect($duplicates)->toHaveCount(2); + expect($duplicates[0])->id->toBe(8); + expect($duplicates[0])->sortOrder->toBe(1); + expect($duplicates[1])->sortOrder->toBe(0); +}); diff --git a/tests/templates/ElementEventsTest.php b/tests/templates/ElementEventsTest.php deleted file mode 100644 index 14558cc..0000000 --- a/tests/templates/ElementEventsTest.php +++ /dev/null @@ -1,22 +0,0 @@ -id = 1; - $destination = new \craft\elements\Entry; - $destination->id = 2; - $field = new \markhuot\keystone\fields\Keystone; - $field->id = 1; - $data = new \markhuot\keystone\models\ComponentData; - $data->type = 'keystone/test'; - $data->save(); - $component = Component::factory()->create(['dataId' => $data->id]); - $child = Component::factory()->create(['dataId' => $data->id, 'path' => $component->id, 'level' => 1]); - (new \markhuot\keystone\actions\DuplicateComponentTree)->handle($source, $destination, $field); - - $duplicates = Component::find()->where(['elementId' => 2])->orderBy(['path' => 'asc'])->collect(); - expect($duplicates[0])->path->toBeNull(); - expect($duplicates[1])->path->toBe((string)$duplicates[0]->id); -});