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

Choose valid id if id is provided for content creation #1613

Open
ksuess opened this issue Mar 31, 2023 · 9 comments
Open

Choose valid id if id is provided for content creation #1613

ksuess opened this issue Mar 31, 2023 · 9 comments

Comments

@ksuess
Copy link
Member

ksuess commented Mar 31, 2023

By now content creation, or concrete adding created content to its container, fails, if content with this id provided by the api call already exists in its container.

Steps to reprouce:
POST request to /mypage with id "foo", etc
Another POST request to /mypage with id "foo", etc

The reason, the use case, for this is:
In Volto you can create content, that gets an id that clashes with for example routes that only Volto knows, not the backend. Create a page with title "Register". You are redirected after creation to the register form.
So if in Volto the id is explicitly provided, like for example by appending "-1": "register-1", on REST api call, this would solve the problem, nearly. Do it twice, and it fails, cause plone.restapi does not apply INameChooser(container).chooseName for a call WITH id.

@davisagli
Copy link
Member

davisagli commented Mar 31, 2023

Note: This only happens when id is included in the POST, not when it is being created automatically from the title.

I don't think we have consensus on whether it is better to throw an error or normalize the id if the client explicitly requested an id that is not available. It might depend on the use case. There's some related discussion in #1487.

@ksuess
Copy link
Member Author

ksuess commented Mar 31, 2023

@davisagli In general, no id should be provided at all. Generating an id is the job of the backend.
By now I do not see another solution, than explicitly providing an id to solve issue plone/volto#4486

@davisagli
Copy link
Member

In the long run I think a better solution for that issue would be to give frontend routes a path like /:register that cannot collide with content ids. It's a complicated topic. Some names are reserved because they have a special use on the frontend and some are reserved because they have a special use on the backend.

@davisagli
Copy link
Member

My own opinion: I think it is best by default for the API to throw an error if the client specifies an id and it is not available exactly as specified. Then the situation is clear. But, I would support adding an option (header or querystring) to tell the API to normalize the id and choose a different one if needed, instead of throwing an error. Then the client has a way to opt in to that if it doesn't care about getting a precise id.

@ksuess
Copy link
Member Author

ksuess commented Mar 31, 2023

Great input.
Me personnally, for the moment, would opt for your suggestion to prevent name clashes at all by
"give frontend routes a path like /:register that cannot collide with content ids."

@davisagli
Copy link
Member

The problem with my suggestion is that it's not backwards-compatible. So that's why I said "in the long run". But it could happen for Volto 17, now, if the @plone/volto-team is okay with it. Not sure if I'm stirring up some longstanding bikeshed debate...

@BhuvaneshPatil
Copy link

In the long run I think a better solution for that issue would be to give frontend routes a path like /:register that cannot collide with content ids. It's a complicated topic. Some names are reserved because they have a special use on the frontend and some are reserved because they have a special use on the backend.

This is great idea but it has some limitations -
For route like - /:register, we consider register as path param.
example -
'/controlpanel/rules/:id/configure'
in this path id is parameter.

@davisagli
Copy link
Member

No, I don't mean a variable parameter, I mean the literal string /:register

@BhuvaneshPatil
Copy link

Another suggestion for this is,
we can add extra property in POST request, that will decide the behavior of id creation ( object creation ).

It can have two values (of course names can be changed) -

  • "GEN_ID_FORCEFULLY"
  • "GEN_ID_SIMPLE"

second value will be default value.
first one will be sent when the generated id will be in routes that clashes with default paths in volto application.
For example - register

This value will tell backend that it should consider that id generated by default is duplicate, and generate it again.

This may be not so robust implementation for the use case, various factor can be taken into consideration.

Please share your thought on the same.

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

Successfully merging a pull request may close this issue.

3 participants