Skip to content

[Feature Request] Add New Attributes @createdBy and @updatedBy #1505

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

Open
milimyname opened this issue Jun 13, 2024 · 4 comments
Open

[Feature Request] Add New Attributes @createdBy and @updatedBy #1505

milimyname opened this issue Jun 13, 2024 · 4 comments

Comments

@milimyname
Copy link

At first, I wanna thank u for creating and working on a great library! I got a request about new attributes to manage spaces/memberships better. Also, it was mentioned in discord.

Is your feature request related to a problem? Please describe.
Introduce new attributes @updatedBy and @createdBy similar to Prisma's @updtedAt to avoid for creating manual relationships between schemas

Describe the solution you'd like

abstract model Base {
  id        String   @id @default(cuid())
  createdAt DateTime @default(now())
  updatedAt DateTime @updatedAt
}

abstract model BaseUser extends Base {
  createdById  String @createdBy @default(auth().id)
  updatedById String? @default(auth().id) @default(auth().id)
}

Describe alternatives you've considered

abstract model BaseUser extends Base {
  createdById String  @default(auth().id) @deny("update", true)
  createdBy   User    @relation("createdBy", fields: [createdById], references: [id], onDelete: SetNull)
  updatedById String? @default(auth().id)
  updatedBy   User?   @relation("updatedBy", fields: [updatedById], references: [id], onDelete: SetNull)
}

Additional context

The conversion in discord:

Mr. Zero5Um

ok, the best way I've come up with is to use an access policy to make sure >the updatedBy can't be set to an arbitrary user id. i can do this like this >@@Allow('update', future().lastUpdatedBy == auth().id. Then with it >validating the update I can update lastUpdatedBy as part of any normal >update on a model w/o worrying about a malicious actor trying to update it >to another user.
if anyone knows any more info or a better way to do this, it would be much >appreciated. btw- LOVE zenstack. when my company finally makes a dollar >i will be donating it to you

The comment from Mr. @ymc9

Hey @Zero5um , I think the post-update rule as you showed is the way to >go and there isn't a simpler way to do that today. Maybe we can introduce ?>an attribute @updatedBy in future releases (similar to Prisma's >@updtedAt). Please help file a feature request if you feel it's important. Thanks!

@baenio
Copy link

baenio commented Jun 13, 2024

@createdBy is not necessary, because neither Prisma is having it - using @default(auth().id) is already very nice

@Azzerty23
Copy link
Contributor

Azzerty23 commented Jun 14, 2024

This would be a nice addition! Perhaps this more generic approach would be a better fit:

The idea would be to let Zenstack disambiguate the relations by generating the opposite fields.

  1. First step ( [Feature request] Support implicit inversed relationship #613)
model User {
  id    Int    @id @default(autoincrement())
 // createdPosts Post[] @relation("createdPosts")     -> no need to specify it in Zmodel,  ZenStack will generate it in the schema.prisma file.
 // updatedPosts Post[] @relation("updatedPosts")
}

model Post {
  id       Int  @id @default(autoincrement())
  createdBy   User @relation("createdPosts", fields: [createdById], references: [id])
  createdById Int @default(auth().id)
  updatedBy   User @relation("updatedPosts", fields: [updatedById], references: [id])
  updatedById Int @default(auth().id)
}
  1. Then, to make it really useful, the model name could be inherited by using a {model} syntax (the idea is not mine but I can't find the original issue):
abstract model Base {
  id    Int    @id @default(autoincrement())
  createdBy   User @relation("created{model}s", fields: [createdById], references: [id])  //  <- here
  createdById Int @default(auth().id)
  updatedBy   User @relation("updated{model}s", fields: [updatedById], references: [id])  //  <- here
  updatedById Int @default(auth().id)
}

model Post extends Base {
  id    Int    @id @default(autoincrement())
}

The remaining question is how to define the type of relation: new field attributes like @to-many and @to-one?

I saw that this issue was addressed through polymorphism (#653 (comment), #613 (comment)), but I would prefer not to be required to have an additional table for this purpose (or did I miss something?):

Edition
id post_id
createdBy user_id
updatedBy user_id
type Post

@ymc9
Copy link
Member

ymc9 commented Jun 16, 2024

Hi @Azzerty23, thanks for the input!

I just wanted to to add a bit more background about the relationship between polymorphism and "implicit opposite relation fields". The observation was that the need to declare opposite relation fields is especially cumbersome when you have some kind of "hub model" which links to many other models. User is quite often such case. The User model can easily become:

model User {
  createdPosts Post[]
  updatedPosts Post[]
  createdVideos Video[]
  updatedVideos Video[]
  ...
}

This also happens to be a good care where you want to model a polymorphic hierarchy:

model User {
  createdAsset Asset[]
  updatedAsset Asset[]
  ...
}

model Asset {
  ...
  type String
  owner User @relation(...)
  ownerId String
}

model Post extends Asset {
  ...
}

model Video extends Asset {
  ...
}

Modeling a polymorphic hierarchy solves three problems in one shot:

  1. Sharing common fields and access policies
  2. Having a "generic" way to query all assets (Asset)
  3. Avoiding the relation field "explosion" in the User model

I understand that not everyone wants the complexity of polymorphism just to solve #3. However, I think Prisma also had good reasons to require an explicit opposite relation. Maybe one of the biggest reasons is to make one-to-one, one-to-many distinctions very obvious. Another downside specific to ZenStack is that there'll be no way to access those implicit opposite relation fields in access policy rules because they're not declared.

@Azzerty23
Copy link
Contributor

I appreciate your feedback @ymc9, always with so much pedagogy. I wasn't convinced at first, mainly because I didn't want to change my habits (yes, even in database design ^^), but the opportunity of #2 (having a "generic" way to query all assets) can prove useful. Thanks for the detailed explanation.

@ymc9 ymc9 changed the title Add New Attributes @createdBy and @updatedBy [Feature Request] Add New Attributes @createdBy and @updatedBy Nov 8, 2024
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

No branches or pull requests

4 participants