Re: $this->propertyValues['is_active'] shouldn't that be rather an overridable method?

Georg Gell wrote:

> i am still playing with the propel container.
> I found this code in common.php::login():
> 
>         // If login is successful (user data has been read)
>         // ...we still need to check if this user is declared active
>         if (!array_key_exists('is_active', $this->propertyValues)
>             || $this->propertyValues['is_active']
>         ) {
> 
> I think that this is not good style, because
> $this->propertyValues['is_active'] is not set in common.php, but in a
> child class. I think that this should be in a method that can be
> overridden by the child class,

I understand your point. But we access propertyValues in several places. 
Especially in the freeze() and unfreeze() method where anything else 
would be rather convulted, especially since the structure of 
propertyValues is defined by the configured table fields.

> without passing return values through undocumented properties. What do
> you think?

well they are not documented because they are configurable .. however 
some optional parameters have special meaning like is_active and lastlogin.

> Ok, another point:
> in LiveUser::readConfig() the defaults for the cookies are set.
> Shouldn't this be in globals.php?

the Globals.php just deals with the storage layer. Most options have 
either defaults inside the LiveUser or the relevant container base classes.

> And a last thing:
> in LiveUser::fileExists(), the function file_exists is used. But this
> function checks the uid of the file, so when you are running
> safe_mode=on, the containers will not be found, even if you could
> include them. is_readable() can be used to prevent to prevent this.

I never dealt with safe_mode. From looking at the documentation I dont 
see any problem changing this. Can anyone else comment on this?

regards,
Lukas