philipbrown/magniloquent

DEPRECATED, USE https://github.com/dwightwatson/validating

magniloquent's Issues

Nice names in validation

In some case is desirable to have a nice attribute names

/**
 * @var array nice attribute names
 */
protected $niceNames = array();

and in the validator...

public function validate()
{
    // Merge the rules arrays into one array
    $this->mergeRules();

    $validation = Validator::make($this->attributes, $this->mergedRules, $this->customMessages);

    $validation->setAttributeNames($this->niceNames);

    if ($validation->passes())
    {
        $this->valid = true;

        return true;
    }

    $this->validationErrors = $validation->messages();

    return false;
}

Attributes without rules don't get saved

When there is a field that does not have any validation rules associated with it, then it doesn't get saved at all due to the use of $this->rules on this line in hydrate():

$attributes = array_intersect_key($attributes, $this->rules);

Don't have the time to look for what else we could use here at the moment as I'm travelling right now. Am getting to India day after tomorrow and will investigate further. As a quick fix I'm just declaring a min rule for the attributes without rules.

Extending this class breaks Laravel remember_token functionality

The User model, that is.

If a 'unique' rule is included all other rules are lost

src/Magniloquent/Magniloquent/Magniloquent.php line 441 needs to be

temp_rule[$rule_key]=implode(',', $rule_params);

Otherwise all other rules get overwritten.

e.g
required|email|unique:users
ends up as
unique:users,id,5

it should be
required|email|unique:users,id,5

Validation auto hash double hashes database seeders

Hi,

I have picked up that the private function autoHash() will double hash password attributes when added via the database seeder operation (php artisan db:seed)

Is this something you could look at? The workaround for the time being is removing the hash maker from the seed class like this:

Instead of this: $user->password = Hash::make('pass'),
just do $user->password = 'pass'

What i'm missing in autoHash()?

This way is used here:

private function autoHash()
  {
    if (isset($this->attributes['password']))
    {
      if ($this->attributes['password'] != $this->getOriginal('password'))
        $this->attributes['password'] = Hash::make($this->attributes['password']);
      }
}

$this->attributes['password'] is the new one in plain, right?
$this->getOriginal('password') is the old one hashed, right?

wouldn't be better to use:

if ( ! Hash::check($this->attributes['password'], $this->getOriginal('password'))) 
{
  $this->attributes['password'] = Hash::make($this->attributes['password']);                
}

Nooo, I'm missing something, right?

As autoHash() is private I can't overwrite it...

Saving a single value

Hey there, seems I am unable to update a single value of a model using standard method per Laravel docs. I'm guessing the overwritten save() doesn't support some of the standard implementation?

$user = User::find(1);
$user->email = '[email protected]';
$user->save();

Found public function setAttribute($key, $value) in the source and attempted using it as well with no luck.

Weird isValid behaviour

When running the User::create($input). The isValid property is true all the way through the magniloquent model save() but sets it back to false in the UserRepository create()

//magniloquent.php

public function save(array $new_attributes = array(), $forceSave = false)
{
    ...
    var_dump($user->isValid()); // true
    return parent::save($options);
}

// EloquentUserRepository.php

public function create($input)
{
    $user = User::create($input);
    var_dump($user->isValid()); // false
    return $user;
}

autoHash - Doesn't Hash Password When Set To Same Plaintext

I noticed an issue where autoHash didn't appear to be hashing a User's password and it ended up being stored in plaintext in my database. I did some investigation and determined that the user tried to change their password to the same plaintext string. Here's a simple test case:

$user = User::find(1);
// This user currently has a different password, that is currently hashed in the database
var_dump($user->password);

// Then we change the password
$user->password = '12345';
$user->save();

// The password has been hashed and saved successfully
var_dump($user->password);
var_dump($user->errors());

// We then try to change the password again, but to the same plaintext string
$user->password = '12345';
$user->save();

// This time, the password is not autohashed and is stored in plaintext
// There are still no errors however
var_dump($user->password);
var_dump($user->errors());

This results in output something like:

string '$2y$10$txasGsFjds0Er/MXzwgi/ue0/FO02txUFMpxm9eLILJveYLYpPsD.' (length=60)
string '$2y$10$zSc9EKVoqbtnlcsCujdkfO8fFs3IJonRU5saUBbD5.2C9WdPiTteK' (length=60)
object(Illuminate\Support\MessageBag)[137]
  protected 'messages' => 
    array (size=0)
      empty
  protected 'format' => string ':message' (length=8)
string '12345' (length=5)
object(Illuminate\Support\MessageBag)[137]
  protected 'messages' => 
    array (size=0)
      empty
  protected 'format' => string ':message' (length=8)

I think this is due to having a combination of the isDirty check along with the Hash::check that was introduced with pull request #40. Wouldn't it be better to always hash the password if it is considered "dirty"? Is there another scenario or use case I'm missing?

For a work around right now, I've added a Hash::check in my controller that prevents the user from changing their password to the same string to prevent this behavior.

composer.json and "dev-master"

I noticed that this repository doesn't have any releases. Is there a way to specify a specific version (commit) of magniloquent? I don't want my application to break when installing/updating unrelated packages or when deploying.

Issue with Mockery

I'm not 100% sure if this is an issue with Magniloquent itself or Mockery, but attempting to mock a class that extends Magniloquent results in a fatal error.

My composer.json's requirements:

"require": {
    "laravel/framework": "4.0.*",
     "magniloquent/magniloquent": "dev-master"
  },
 "require-dev": {
    "mockery/mockery": "0.8.*",
    "way/generators": "dev-master",
    "way/laravel-test-helpers": "dev-master",
    "phpunit/phpunit": "3.7.*"
},

My model:

use Magniloquent\Magniloquent\Magniloquent;

class TestModel extends Magniloquent {

    public static $rules = array(
        'create'    => array(),
        'update'    => array(),
        'save'      => array()
    );
}

And my test:

class ExampleTest extends TestCase {

    /**
     * A basic functional test example.
     *
     * @return void
     */
    public function testBasicExample()
    {
        $crawler = $this->client->request('GET', '/');

        $this->assertTrue($this->client->getResponse()->isOk());
    }

    public function __construct()
    {
        $mock = Mockery::mock('TestModel');
    }

}

Running $ vendor/bin/phpunit results in the following:

Cannot redeclare Mockery_2112532308::offsetUnset() in /home/tnsosnet/elections/laravel/vendor/mockery/mockery/library/Mockery/Generator.php(129) : eval()'d code on line 1031

Make mergeRules protected?

Can Magniloquent::mergeRules be made a protected method, instead of private? I'm trying to solve the unique ID problem (#22, #23, #24), and while I have a simple, decent solution, I'm not sure it's perfect for everyone. Nor am I clear if this is something you necessarily want to solve in this package. Anyway, I'd be able to solve it myself if I could just override mergeRules() in my own base model.

p.s. This is what I'm using, if it were implemented in Magniloquent. Feel free to use it, or I'll gladly open a pull request if you want. (It does change the rules in $this->mergedRules from strings to arrays to make the parsing less complicated, but that's fully supported by Validator.) But a simple change of mergeRules to protected will allow me to add this logic to my own model class.

/**
 * Merges saving validation rules in with create and update rules
 * to allow rules to differ on create and update.
 *
 * @return array
 */
protected function mergeRules()
{
    $rules = static::$rules;
    $output = array();

    if ($this->exists)
        $merged = array_merge_recursive($rules['save'], $rules['update']);
    else
        $merged = array_merge_recursive($rules['save'], $rules['create']);

    foreach ($merged as $field => $rules)
    {
        if (is_string($rules)) {
            $rules = explode('|', $rules);
        }
        $output[$field] = $this->replaceUniqueKey($rules);
    }

    $this->mergedRules = $output;
}

/**
 * Adds this model's primary key to unique rules.
 *
 * In order for unique rules to be altered, the rule must have an
 * "@key" placeholder. This should only be used in an 'update' rule.
 *
 * @example
 *     public static $rules = array(
 *         'save' => array(
 *             'email' => ['email'],
 *         ),
 *         'create' => array(
 *             'email' => ['unique:users,email'],
 *         ),
 *         'update' => array(
 *             'email' => ['unique:users,email,@key,id'],
 *         ),
 *     );
 *
 * @param array $rules
 * @return array
 */
protected function replaceUniqueKey(array $rules)
{
    array_walk($rules, function (&$rule) {
        // Only search in unique rules, as '@key' could be a literal string in another rule.
        if (strpos($rule, 'unique:') === 0) {
            $rule = str_replace('@key', $this->getKey(), $rule);
        }
    });
    return $rules;
}

Support for multiple database connections

Add
$validation->getPresenceVerifier()->setConnection($this->connection);
in Validate function after creating Validator.

Cheers.

Support for Laravel 4.1

Can magniloquent support Laravel for 4.1.x? or 4.*?
I'm not sure when (or how rather) my Laravel got set/changed to 4.1 as I don't see a 4.1 release anywhere but I know I didn't do it manually.

I tried just dropping it down to 4.0 but that seems to cause other issues.

touch passed as an attribute

When you do something like this the touch parameter is set in the $new_attributes array.

$post = Post::find(1);

$user->posts()->save($post);

A dirty fix is to just add the following into the save method:

if(isset($new_attributes['touch']))
{
  unset($new_attributes['touch']);
}

I'm going to look into a better solution now.

On Update, no returned validation messages

I'm a bit of a PHP n00b still, so this might be a dumb question. On create, the proper objects are being returned so that I can parse the validation rules and re-route/display validation rules, if necessary. However, on save/update, if validation fails, I just get a false value returned.

This appears to be coming from line 134 of Magniloquent.php. What I'm unsure about is where create is configured to pass the correct information, and if I'm just simply missing some configuration somewhere.

Here's some default code, I'm using.

Model

Validation

Create Function

Update Function

Controller

Create Function

Update Function

What's Returned

Create

Update

Anyway, I'm not sure if I'm doing something incorrectly, or if this is a bug. I think it has to be me, but just wanted to check.