Improvement on models
Posted 07 July 2009 - 03:31 AM
I kind of liked the previous approach of saying: all attributes are considered unsafe on default (when using the safeAttributes function), you quickly notice when you forgot to set an attribute as safe. The other way around you do not quickly notice an error.
Yii uses functions that return arrays (function rules()) which is nice since it has advantages over using object variables ($rules). So I might actually generate a standard rules array automatically in my base model using table meta data. The problem with the new approach would then be that its not obvious which attributes are safe in my AR classes that inherit from the base class.
Have you considered not including the new approach at all? It looks nice at first but introduces more problems in the long run I think.
A compromise would be to still consider all attributes as unsafe even if they have a rule assigened and then require all safe attributes to have the safe-rule.
Posted 07 July 2009 - 07:35 AM
The fact is, why you would declare a validation rule if the corresponding attribute doesn't come from user input?
Posted 07 July 2009 - 08:55 AM
Example for rules on unsafe attributes
I have a table "users" and a corresponding AR "users":
- I set all standard rules (length, exists, required, etc.) without a scenario
- I have a scenario for admins that makes all fields safe
- I have another "user" scenario for normal users that makes only some fields safe (e.g. so that a user can change his email address)
So in this situation I have rules for fields that I dont want to be safe in certain situations. In the current Yii version this is expressed quite clearly. It works well this way and I also like the fact that for all the standard rules I am not forced to set a different scenario to prevent the "safe" implication.
In version 1.1 I would first have to make sure I add safe rules for all fields that do not have a rule, then I have to have a number of unsafe rules for my "user" scenario.
I tried Yii 1.1 and I felt that the new approach was less clear, maybe you write a little less code but I think it can get confusing and errornous.
In Yii1.1 I would end up writing unsafe rules for my scenarios even if they are not necessary because I would be affraid I overlooked a rule for a certain attribute. Also when you add a rule you have to check carefully if the safe implication has unexpected consequences.
Ok, to sum this up: the problem lies with the fact that a normal rule becomes a double rule, i.e. 'length' means 'length' and 'safe' (but 'safe' only means 'safe'). This is too much magic, maybe ok for small table, but confusing for large ones.
Breaking BC for this change that in my opinion is not really improving Yii might not be such a good idea.
Posted 07 July 2009 - 02:13 PM
// rules shared by all scenarios
// rules specific for 'admin' scenario
// rules specific for 'user' scenario
As you may tell, such organization is very similar to the current safeAttributes() design. The difference is that we no longer need to deal with attribute safety in two places in 1.1.
Yes, this will require you to repeat the same rule (except 'on' option) several times if the rule appears in several scenarios. But then, you can sacrifice the clarity by merging these rules into a single one whose 'on' option contains those scenarios.
In most of the time, we really don't need 'unsafe' rules. It is included mainly for completeness.
Posted 08 July 2009 - 09:48 AM
Especially when I override the rules() function to automatically generate standard rules based on DB meta data - these rules do not have a scenario.
As I said, I have tried the new Yii1.1 way and it felt somewhat messy after a while. Is anybody using it yet? Any positive feedback?