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

feat: Add a new readonly property to Model fields definition #4603

Open
diosney opened this issue Oct 2, 2015 · 25 comments
Open

feat: Add a new readonly property to Model fields definition #4603

diosney opened this issue Oct 2, 2015 · 25 comments
Labels
existing workaround For issues. There is a known workaround for this issue. P4: nice to have For issues that are not bugs. status: understood For issues. Applied when the issue is understood / reproducible. type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@diosney
Copy link
Contributor

diosney commented Oct 2, 2015

readonly will be a boolean property by default set to false.

Its operation can be something like:

  • when an instance is created or updated, if the field haven't a value yet, then can be set without triggering a readonly error.
  • once the field has a value, any further modification attempt will trigger a readonly error.

This come in handy when manipulating this kind of fields, which is a common use case I think (I've the need of this kind of model fields across several different projects).

So, what do you think?

@diosney
Copy link
Contributor Author

diosney commented Oct 2, 2015

@mickhansen
Copy link
Contributor

Good idea. Good candidate for a plugin

@janmeier janmeier added type: feature For issues and PRs. For new features. Never breaking changes. good first issue For issues. An issue that is a good choice for first-time contributors. labels Oct 4, 2015
@diosney
Copy link
Contributor Author

diosney commented Oct 5, 2015

@mickhansen I want to do it, but I don't know where to start (I didn't found in the docs how to do sequelize plugins).

Can you point me towards the right direction so I can do it?

Thanks

@mickhansen
Copy link
Contributor

I would simply wrap Instance.prototype.set, on call check model attribute for readonly, if set, check current value, if current value throw, else call original set.

@diosney
Copy link
Contributor Author

diosney commented Oct 5, 2015

@mickhansen Thanks, I will give it a try.

@mickhansen
Copy link
Contributor

https://github.com/mickhansen/ssacl-attribute-roles does some of the same stuff, you can check out the code there.

@diosney
Copy link
Contributor Author

diosney commented Oct 9, 2015

Sorry, for the delay on this, I was busy on other things.

I've the doubt now, what approach will you prefer?:

  • the setter method you advised
  • though global hooks with beforeValidation or the combination [beforeCreate and beforeUpdate] or the combination of three of them?

Thing is that with the setter approach the readonly check can be skipped if the user set a custom setter or using raw: true right? And I would prefer to always enforce the check.

Thanks!

@mickhansen
Copy link
Contributor

If you wrap the setter method you can choose to ignore the raw option.

@diosney
Copy link
Contributor Author

diosney commented Oct 9, 2015

@mickhansen Nice! I will continue with that option then, thanks!

@TonyNLewis
Copy link

I came across this looking for a way to create a model based on a view (db-level) that would only attempt to save the fields in the underlying base table. For example, the view might have a part number and description that are associated with a PartID. When I save the record, I don't want the related fields to be saved, just the PartID itself. It seems to me that this is truly "readonly" and the implementation being suggested is more "noupdate".

@mickhansen
Copy link
Contributor

@TonyNLewis are you looking to disallow all writes (i.e. both initial and update) or just updates? Is part number allowed to be updated if it is currently null?

You could take a look at: https://github.com/mickhansen/ssacl-attribute-roles
It's not that mature yet but could be worth a try.

@diosney
Copy link
Contributor Author

diosney commented Aug 17, 2016

Finally got the time to do this 😄

@TonyNLewis I accepted your suggestion and named the plugin sequelize-noupdate-attributes, although readonly capabilities can be added too if someone asks for it.

I added the new plugin at the wiki https://github.com/sequelize/sequelize/wiki/Add-ons-&-Plugins

npm location: https://www.npmjs.com/package/sequelize-noupdate-attributes

Thanks @mickhansen for your help.

@diosney diosney closed this as completed Aug 17, 2016
@reinert
Copy link

reinert commented May 19, 2017

This definetly should be in core. ReadOnly option is a must have for ORMs.

@leodutra
Copy link

leodutra commented Nov 22, 2017

Please consider it as core feature, gentlemen.

@jeremybradbury
Copy link

jeremybradbury commented Feb 16, 2018

why not something more standard like user.removeDataValue('email') inside update hooks?

Model says "oh you got an email update for me?... https://www.youtube.com/watch?v=lWtPFv194GU

@MaximilianLloyd
Copy link

Any update on this?

@OrenSchwartz
Copy link

?

@ado-astpos
Copy link

Any update?

@svendeckers
Copy link

new year, new update? :-)

@papb papb removed the good first issue For issues. An issue that is a good choice for first-time contributors. label Jan 17, 2020
@papb papb reopened this Jan 17, 2020
@papb papb added existing workaround For issues. There is a known workaround for this issue. P4: nice to have For issues that are not bugs. status: understood For issues. Applied when the issue is understood / reproducible. labels Jan 17, 2020
@papb
Copy link
Member

papb commented Jan 17, 2020

I have reopened this but it is going to be low priority, since there is already a plugin to do this

@svendeckers
Copy link

svendeckers commented Jan 20, 2020

@diosney @papb Thank you!
I'm afraid the plugin doesn't work anymore, or at least I couldn't get it to work.
So really looking forward to this :-)

@Daspy11
Copy link

Daspy11 commented Apr 2, 2021

One year later, still would like this :)

@ShubhamPalriwala
Copy link

Let's implement this 😌

@bkimminich
Copy link

@juice-shop could really use this feature, as we're currently doing it via a third party module, but that doesn't work with TypeScript and kind of blocks our migration now.

@micodls
Copy link

micodls commented Mar 21, 2023

Bumping this feature request :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
existing workaround For issues. There is a known workaround for this issue. P4: nice to have For issues that are not bugs. status: understood For issues. Applied when the issue is understood / reproducible. type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests