Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Data objects] Default value generator service for URL Slugs #6132

Closed
weisswurstkanone opened this issue Apr 1, 2020 · 7 comments · Fixed by #6246
Closed

[Data objects] Default value generator service for URL Slugs #6132

weisswurstkanone opened this issue Apr 1, 2020 · 7 comments · Fixed by #6246
Assignees

Comments

@weisswurstkanone
Copy link
Contributor

as an addition to #5069

@solverat
Copy link
Contributor

@weisswurstkanone @BlackbitNeueMedien: I just tested this feature and I'm not sure if I'm getting this right (No specific documentation about this).

This will trigger (getValue via DefaultValueGeneratorInterface service) only if a new object gets generated, right? But at this point, no information about the object is available.

Imagine a customer wants to create a blog entry - the (localized) title is available after an update because it's not possible to add those fields in an object creation state...

I came up with a proposal some years ago (#1048) and I think that's still a thing. The slug field is an "unsuable" field in a business point of view, since a user needs to add a complete path to it (/en/xxy).

@BlackbitDevs
Copy link
Contributor

BlackbitDevs commented May 19, 2020

I have not retested it now but as I remember I tried to implement it so that the default values are generated every time an object gets saved if the field is empty. This is done in getDataForResource() in all Pimcore\Model\DataObject\ClassDefinition\Data subclasses. getDataForResource() gets called in

if ($fd instanceof ResourcePersistenceAwareInterface) {
// pimcore saves the values with getDataForResource
if (is_array($fd->getColumnType())) {
$insertDataArray = $fd->getDataForResource($this->model->$getter(), $this->model,
[
'isUpdate' => $isUpdate,
'owner' => $this->model
]);
if (is_array($insertDataArray)) {
$data = array_merge($data, $insertDataArray);
}
} else {
$insertData = $fd->getDataForResource($this->model->$getter(), $this->model,
[
'isUpdate' => $isUpdate,
'owner' => $this->model
]);
$data[$key] = $insertData;
}
}

For example for the input fields:

public function getDataForResource($data, $object = null, $params = [])
{
$data = $this->handleDefaultValue($data, $object, $params);
return $data;
}

The handleDefaultValue() then returns the default value if the field is empty.

Sadly Pimcore\Model\DataObject\ClassDefinition\Data\UrlSlug does not implement Pimcore\Model\DataObject\ClassDefinition\Data\ResourcePersistenceAwareInterface and for this reason above description does not work for this datatype.
Is this on purpose, @weisswurstkanone ?

@solverat
Copy link
Contributor

UrlSlug

This save() method does not get executed if the field hasn't been marked as dirty - so this field will never gets prefilled by default.

public function save($object, $params = [])
{
if (isset($params['isUntouchable']) && $params['isUntouchable']) {
return;
}
$data = $this->getDataFromObjectParam($object, $params);
if (!is_array($data) || count($data) === 0) {

Also, the method getDataForResource is not available within the UrlSlug data type.

Handle Default Value

This method does not get dispatched in update processes:

/**
* 1. only for create, not on update. otherwise there is no way to null it out anymore.
*/
if ($isUpdate) {
return $data;
}

@weisswurstkanone
Copy link
Contributor Author

UrlSlug uses the CustomResourcePersistingInterface (not the ResourcePersistenceAwareInterface) so there is no getDataForResource.

@BlackbitDevs
Copy link
Contributor

Hmm, does a default value generator make any sense then for URL slug datatype, @weisswurstkanone ? Because - as @solverat said - originally default value generators had two intentions:

  1. Generate IDs like article numbers which do not depend on other fields' contents
  2. Generate values based on other fields' values

Imho, with the current implementation URL slugs can only fulfill 1. but not 2.

@dpfaffenbauer
Copy link
Contributor

Currently not, but the goal should be to make sense ;) URL Slugs is something a user will never manually input, so it has to be generated somehow.

@nicklassandell
Copy link
Contributor

I just stumbled upon this issue while trying to figure it out myself, and in the end I opted for a calculated data type. The benefit is that it runs on every save, but the downside is that the user can't manually edit the slug, which in my experience, they do want the option to do. A workaround for that would be to have a second input field where they can manually enter a slug, but that isn't very nice. Anyway, just thought I'd leave this here for anyone else who stumbles across the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants