In api.content.create, avoid renaming object if "id" was not given

[gbastien]

Hi @jensens @ale-rt

we discovered that when creating an element using content.create without specifying an "id", it was created then renamed, this cause ContainerModified event to be called 2 times (and some other events too).

I wondered why this was done in 2 steps? Could it be done before creating the element?
https://github.com/plone/plone.api/blob/master/src/plone/api/content.py#L118
Actually the element is created and if id is not given, then it is renamed to a computed id.
I think we could check if "id" is given, then generate it and pass it to invokeFactory instead renaming it after?
Or is there some reason why it is done after?

We could propose a PR "solving" this.

We avoid this by passing a generated id to content.create now.

Thank you,

Gauthier

[ale-rt]

👍 that is a thing which is also annoying me.
I also almost always pass the id to avoid the object to be moved after creation.

But I fear that in some cases this can be hard.

The auto generated id might be based:

• the title (e.g. pages or folders)
• file
• image
• whatever your custom logic wants

Now the object is created using container.invokeFactory(type, content_id, **kwargs).
The parameter content_id is either the id which is passed by the user or a generated one.
I bet some lower level method that does not attach the object to the container could be used.

Anyway if you could make a PR to fix this I will be super happy to review it.
If we do not manage to fix this, we should document what is happening very precisely.

IIRC this is not the first time this topic is dicussed.

[jensens]

+1000

[abhishekdubey369]

Is this issue resolved if not I would like to work on this as in create its logic is if not id or (safe_id and id) which bascially works even if there is no id where as it should check if id not present than it should create a temp id and do.

[davisagli]

@abhishekdubey369 You haven't understood the issue. There is already a temporary id created. The point of the issue is to avoid doing that by generating the final id. But this is quite complicated to do correctly in every case, and sometimes impossible.

[abhishekdubey369]

hello @davisagli ,
my approach is like :

chooser = INameChooser(container)

if not id:
content_id = chooser.chooseName(title, container) # Generate ID from title
elif safe_id:
content_id = chooser.chooseName(id, container)
else:
content_id = id

if title:
kwargs["title"] = title

try:
    container.invokeFactory(type, content_id, **kwargs)
    transaction.savepoint(optimistic=True)
except UnicodeDecodeError:
    # UnicodeDecodeError is a subclass of ValueError,
    # so will be swallowed below unless we re-raise it here
    raise
except ValueError as e:
    types = [fti.getId() for fti in container.allowedContentTypes()]

    raise InvalidParameterError(
        "Cannot add a '{obj_type}' object with id={obj_id} to the container {container_path}.\n"
        "Allowed types are:\n"
        "{allowed_types}\n"
        "{message}".format(
            obj_type=type,
            obj_id=content_id,
            container_path="/".join(container.getPhysicalPath()),
            allowed_types="\n".join(sorted(types)),
            message=str(e),
        ),
    )

return container[content_id]

what mess can it create ?

[davisagli]

@abhishekdubey369 The next step needed here is a careful review of @ale-rt's existing PR, not a new implementation.

[jensens]

existing PR is #484