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

implement AllowClientUUIDs for record creation #241

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bartke
Copy link

@bartke bartke commented Aug 2, 2023

Add a new configuration option AllowClientUUIDs. When this option is enabled, we can use a UUID supplied in the request payload as the primary key for the record, providing it's not nil. This addition supports client-supplied UUIDs, enabling idempotency in entity creation. It could be more lenient and allow strings alongside uuids too to address #107. Not sure if there is appetite for either.

@bartke
Copy link
Author

bartke commented Oct 26, 2023

@masseelch I think you might be the right person to prod here. A corresponding PR was merged in ent/contrib/entoas recently, would love to hear your feedback on this and if this is possible.

Copy link
Member

@masseelch masseelch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bartke, thank you for your contribution. Sorry I missed this before. Instead of allowing this only for UUIDs, I think we can allow this option for any ID, right?

Comment on lines +26 to +28
"allowClientUUIDs": func() bool {
return cfg.AllowClientUUIDs
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this I prefer to use extend and add the flag there. Have a look at how it is done inside Ent: https://github.com/ent/ent/blob/3f1063c77ec98d1b1d5d1cdc39659cd13931ab8f/entc/gen/func.go#L265

@@ -21,6 +21,9 @@ type (
Target string
// The Views created by entoas.
Views map[string]*entoas.View
// Whether to allow the client to supply IDs in case uuids are used.
// AllowClientUUIDs, when enabled, allows the built-in "id" field as part of the payload for create, allowing the client to supply UUIDs as primary keys and for idempotency.
AllowClientUUIDs bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about making it more generic and allow to set any ID field, if given?

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 this pull request may close these issues.

2 participants