Add Password Field, drop multiple support in Email Field

[mvorisek]

No description provided.

[mvorisek]

multiple should be modelled by standard DB relation or using ContainsMany

[mkrecek234]

Can you keep allow_multiple in please? Even though databases should store them in 1:n child tables (agree), a list of email recipients stored in a single text field is a use case that is valid and existing.

[mvorisek]

Can you keep allow_multiple in please? Even though databases should store them in 1:n child tables (agree), a list of email recipients stored in a single text field is a use case that is valid and existing.

that is what ContainsMany is for when you want 1:n inlined in a single field ;-)

[mkrecek234]

Can you keep allow_multiple in please? Even though databases should store them in 1:n child tables (agree), a list of email recipients stored in a single text field is a use case that is valid and existing.

that is what ContainsMany is for when you want 1:n inlined in a single field ;-)

Cool. Every day something new to learn ;-)

[DarkSide666]

Can you keep allow_multiple in please? Even though databases should store them in 1:n child tables (agree), a list of email recipients stored in a single text field is a use case that is valid and existing.

that is what ContainsMany is for when you want 1:n inlined in a single field ;-)

But will ContainsMany store them as comma separated list? I guess it will store them as PHP array or JSON instead. Meaning - ContainsMany solution will not be compatible with already existing DB field with comma separated values.

[DarkSide666]

1. Not sure if we should remove allow_multiple functionality of Email field. It was handy and quite widely used. ContainsMany is not the same - it will encode array values or use json, but not plain CSV. Maybe we should introduce some kind of CSV field instead which will serialize array (values) as simple comma separated list?
2. I think Password field should use default set() method instead of setPassword(). And after model loading it should not show hash as value for security. In previous Password field implementation we stored hash value in separate fields property after model load and was able to use it for validating password, but it was never exposed to user.
3. Also think about this example:

$m = $user->load();
$m->set('p', $m->get('p')); // will hash password, your setPassword() method, so it get dirty
$m->save();

Theoretically if you get value and set it, then it shouldn't change and shouldn't become dirty. But if it hashes, then it'll change and save (overwrite old password).

So there are things i don't like about this.

[mkrecek234]

1. Not sure if we should remove allow_multiple functionality of Email field. It was handy and quite widely used. ContainsMany is not the same - it will encode array values or use json, but not plain CSV.

Fully agree - the developer should have the option to still use comma-separated values. There definitely are valid use cases for that - not every field with multiple values justifies a 1:n table, or the need to render it in JSON

[DarkSide666]

1. Not sure if we should remove allow_multiple functionality of Email field. It was handy and quite widely used. ContainsMany is not the same - it will encode array values or use json, but not plain CSV.

Fully agree - the developer should have the option to still use comma-separated values. There definitely are valid use cases for that - not every field with multiple values justifies a 1:n table, or the need to render it in JSON

Yeah. Imagine also list of phone numbers for person for example. Of course, in super normalized DB structure we should store them in separate table, but in reality 95% of time they will be stored as CSV in one field. So maybe we should have ContainsMany or something similar which serializes as simple CSV. Then we could use it + single Email field to get what we want - CSV list in one field in DB.
Also in some tag list implementations i've seen CSV.

[mvorisek]

Not sure if we should remove allow_multiple functionality of Email field. It was handy and quite widely used. ContainsMany is not the same - it will encode array values or use json, but not plain CSV. Maybe we should introduce some kind of CSV field instead which will serialize array (values) as simple comma separated list?

containsManyField will be handy to define 1:n field, currently, containsMany + addField must be used

I think Password field should use default set() method instead of setPassword(). And after model loading it should not show hash as value for security. In previous Password field implementation we stored hash value in separate fields property after model load and was able to use it for validating password, but it was never exposed to user.

tell me how :)

values are newly normalized using stateless (non-entity-aware) normalize method and calling this method twice on un-hashed password would rehash the password thus prevent us to compare it as unchanged

the only option to prevent a rehash is to remember all newly seen passwords and store them globaly

now, why you even think so? The Field class provides an interface to hash and verify the password hash which is stored in the underlaying type. Why should a non-hashed value be even stored somewhere?

So maybe the field should be named PasswordHash to stress that. If someone want to operate on a native password, standard Field can be used.

Theoretically if you get value and set it, then it shouldn't change and shouldn't become dirty. But if it hashes, then it'll change and save (overwrite old password).

What?