Closed joepagan closed 2 years ago
The update method should probably support a possible ID being passed in. Similar to the fields.
Patch for id support and error display of missing sections: https://gist.github.com/spAnser/c73e84d417b74c918d116f604c050fcc
Running into some trouble getting the example code to import.
Error:
- Creating default object from empty value
Error:
- Call to a member function getEntryTypes() on null
If I comment out your concerning continue
I run into:
Error:
- Section parent "home" was not imported successfully.
Error:
- Creating default object from empty value
Error:
- Call to a member function getEntryTypes() on null
There may be some things to consider about running an update and not having imported the section in this run.
Thanks for the review, I'll investigate passing an ID in.
RE the example code, did you use my example json? Or did you test on an app with different section handles json?
I presume you are not expecting the contents of the entryTypes
array to not go off and build a section if one does not exist? Leaving that completely up to the dedicated sections array as shown in my other PR https://github.com/Pennebaker/craft-architect/pull/41
Can certainly improve the error logging to say section x doesn't exist
etc
On a clean install without any sections at all.
Okay looking through the code, I see the problem concerning continue
.
So in the FieldProcessor: public function update(array &$itemObj)
This function is changing and updating the itemObj
, ultimately all it really needs to do is set the id if it isn't already set. The update function shouldn't actually be performing a save of the element.
I think I could rename it but it is a preprocessor for performing an update. For matrix fields this is pretty in depth as we do not want to lose content that already exists by recreating the field layout. This pre-processing is necessary to build the changed parts from the updated structure onto the older structure before running the normal import / save functions. I think for some of the parts it may only need an ID assignment in this update
function.
I think if I was to rewrite the name of this function it would be updatePreProcessing
, processForUpdate
, prepForUpdate
or updatePrep
. Having a better name for what this function should've performed might've helped you here.
I think a good starting point for adding update functionality to an existing processor would be to start with just assigning the ID if it isn't already assigned. Then add on any necessary pre processing to make sure it doesn't de-link content. You can assign the ID only and check what breaks from the normal processing and find what would keep things from breaking.
My quick example for a base update function for sections:
/**
* @param array $itemObj
*
* @return array|null
*
* @throws \yii\base\InvalidConfigException
*/
public function update(array &$itemObj)
{
/* @var Section $section */
if (isset($itemObj['id'])) {
$section = Craft::$app->sections->getSectionById($itemObj['id']);
} else {
$section = Craft::$app->sections->getSectionByHandle($itemObj['handle']);
if ($section) {
$itemObj['id'] = $section->id;
}
}
if ($section) {
// TODO: Maybe allow swapping between Channel and Structure by just making sure the old/new type isn't Section::TYPE_SINGLE
if ($itemObj['type'] !== $section->type) {
$errors = [
'type' => [
Architect::t('Type does not match existing type: "{structure}".', ['fieldType' => $section->type ])
]
];
return $errors;
}
switch ($itemObj['type']) {
case Section::TYPE_SINGLE:
// Single Pre Processing
break;
case Section::TYPE_CHANNEL:
// Channel Pre Processing
break;
case Section::TYPE_STRUCTURE:
// Structure Pre Processing
break;
}
// Generic Pre Processing
}
return null; // No errors
}
What changes made
Why
EntryTypes cannot be updated via the cli
Concerns
I am concerned about adding
continue
here as I am not sure if there is anything necessary further down in the method that this could skip. Previously, without thiscontinue
, the method could reach the bottom, and in turn running bothupdate
andsave
methods for a processor. This would result in console errors because of the 2ndsave
running, even though the update was successful along the lines of:Testing
/var/www/site/update.json
:Run with:
./var/www/site/craft/craft architect/import/update /var/www/site/update.json