Code Monkey home page Code Monkey logo

Comments (11)

laravelbook avatar laravelbook commented on May 19, 2024

@mogilny - Unlike Eloquent models which persist all attributes to the database, Ardent models are picky. You must explicitly define a rule-set which will determine the fields/attributes to validate and save. I agree, not all model classes may depend on user generated content and thus may not require validation - in such scenario, you may derive your models straight from Eloquent. Ardent is appropriate for situations where you need to sanitize user supplied data before saving to database.

There are two ways to provide validation rules to Ardent models -

You can bake the rule-set inside your model classes:

class Post extends Ardent {

    public static $rules = array(
        'title' => 'required|between:4,64'
    );

}

Additionally, you may provide custom rules to validate() or save() methods.

If you do not define any rules, Ardent simply will discard all fields and not save anything. That's where line 139 comes into play - it filters out all those attributes that do not have corresponding rules.

As for "password_confirmation" not validating - I suspect it occurs due to non-existent/faulty ruleset? Can you kindly share the snippet?

from ardent.

mogilny avatar mogilny commented on May 19, 2024

Thank you for your fast reply. Got that with the rules, and it makes sense (even for me, lol :-).
But I still have a problem with the confirmation. Even if I register a rules set with the password_confirmation

    public static $rulesUpdate = array(
        'username'  => 'required',
        'email'     => 'required|email',
        'password'  => 'confirmed|min:6',
        'password_confirmation'     => 'min:6',
        'role'      => 'integer|min:2',
        'active'        => 'integer'
    );  

it still not gets validated correctly. If I check the data the is passed to the validator (at least in my case), there is no password_confirmation value, as I do not have this field in the database and therefor is not present in the $this->attributes on line 147.

The data and rules passed to the validator is:

    $data = array(
        'user_id' => integer 9
        'username' => string (5) "test2"
        'email' => string (12) "[email protected]"
        'password' => string (6) "123456"
        'role' => integer 3
        'active' => integer 1
        'created_at' => string (19) "2013-02-25 10:55:48"
        'updated_at' => string (19) "2013-02-25 17:25:39"
    );
    $rules = array(
        'username' => string (8) "required"
        'email' => string (14) "required|email"
        'password' => string (15) "confirmed|min:6"
        'password_confirmation' => string (5) "min:6"
        'role' => string (13) "integer|min:2"
        'active' => string (7) "integer"
    );

so with this type of settings it seems clear that the validation will fail, as the data does not contain the password_confirmation and therefore the validation will fail. If I modify the $this->attributes with Input:all() the the $data array will contain the password_confirmation value.

Pls do not spend a lot of time for investigation, as it will be probably a problem that only occurs in my environment. The only things that would interest me if you were able to successfully validate a form with a confirmation where the model does not have the field in the database.
If that is the case, the pls close the ticket ...

Again thank you for your support and keep up the good work !!

from ardent.

andheiberg avatar andheiberg commented on May 19, 2024

Can I give my five cents as to why I think this new implementation is a bad idea?

Either way here it goes:

  • Updating arden just broke my code and just lost a lot of time finding the problem.
  • People wouldn't no longer just be able to use ardent for it's other features and not for validation (automaticly hashed fileds, beforeSave(), afterSave(), etc.)
  • Doing this breaks something that does not need breaking! You force people in to using the class in a specific way with out gaining any code benefits as I see it. Clarity and Size both seam to have been poorly affected.
  • Last and most I'm portant reason! Cause I say so :D haha

Thanks for Ardent :)

from ardent.

laravelbook avatar laravelbook commented on May 19, 2024

@mogilny : Thanks for the prompt reply and the code snippet.

I tried to recreate your code. Here's my model class:

class Entity extends \LaravelBook\Ardent\Ardent
{
    public static $rules = array(
        'username' => 'required',
        'email' => 'required|email',
        'password' => 'confirmed|min:6',
        'password_confirmation' => 'min:6',
        'role' => 'integer|min:2',
        'active' => 'integer'
    );
}

And here's the route file:

Route::get('/', function () {

    $data = array(
        'user_id' => 9,
        'username' => "test2",
        'email' => "[email protected]",
        'password' => "123456",
        'role' => 3,
        'active' => 1,
        'created_at' => "2013-02-25 10:55:48",
        'updated_at' => "2013-02-25 17:25:39"
    );

    $entity = new Entity($data);
    var_dump($entity->validate());
});

As expected, the validation returns false (since $data doesn't contain the "password_confirmation" field)

As for "*_confirmation" fields you do not need to worry since these redundant fields will automatically be stripped away by Ardent before persisting the model object to database. Kindly refer to the addBasicPurgeFilters() method to see how this is done.

I'm closing this issue now. Please feel free to reopen it if you have more queries (I agree the documentation needs a bit of polishing ;) )

Thanks and regards!

from ardent.

laravelbook avatar laravelbook commented on May 19, 2024

@AndreasHeiberg - Thank you for the candid input!

Lately, Ardent has been under-going bug-fixing/enhancement spree. i.e. getErrors() was renamed to errors() (changeset: b025307) to make the class conformant to Laravel 4. Apart from that, there haven't been that many breaking changes.

People wouldn't no longer just be able to use ardent for it's other features and not for validation (automaticly hashed fileds, beforeSave(), afterSave(), etc.)

Ardent is very customizable and most of the features can be turned on/off by toggling the appropriate boolean field.

Doing this breaks something that does not need breaking! You force people in to using the class in a specific way with out gaining any code benefits as I see it. Clarity and Size both seam to have been poorly affected.

Ardent had merged many pull requests recently that presented clearly evident use cases. I'm sorry if you feel that way, but all recent enhancements and additions to Ardent actually provide value to the end-user (the developer).

Updating arden just broke my code and just lost a lot of time finding the problem.

I'd love to learn more about it. Would it be possible for you share the snippet that is not functioning after upgrading to the latest version?

Thanks and regards.

from ardent.

andheiberg avatar andheiberg commented on May 19, 2024

@laravelbook

I'd love to learn more about it. Would it be possible for you share the snippet that is not functioning after upgrading to the latest version?

Well the problem is exactly as stated. If you don't have $rules set you code breaks.

Lately, Ardent has been under-going bug-fixing/enhancement spree. i.e. getErrors() was renamed to errors() (changeset: b025307) to make the class conformant to Laravel 4.

Yes I submitted that pull request :)

Apart from that, there haven't been that many breaking changes.

No, except this very issue that we are talking about

Ardent is very customizable and most of the features can be turned on/off by toggling the appropriate boolean field.

That is correct, but by implementing it this way means that you can only customize features after you have created $rules with valid validation rules. If you had nothing to validate you would not be able to use Ardent.

Ardent had merged many pull requests recently that presented clearly evident use cases. I'm sorry if you feel that way, but all recent enhancements and additions to Ardent actually provide value to the end-user (the developer).

Yes of cause it does. It's all good. I'm just saying that this specific change doesn't add value as I see it.

Closing remarks
No need for heated debate, I just wanted to point out to you the reasons why I think this very specific change is for the worse.

Take it or leave it, as they say.

from ardent.

laravelbook avatar laravelbook commented on May 19, 2024

@AndreasHeiberg - thanks for the prompt reply.

Yes I submitted that pull request :)

Oops! ;)

Well the problem is exactly as stated. If you don't have $rules set you code breaks.

Thanks for finding this bug (your earlier message was vague). Kindly upgrade to 116b3d6

from ardent.

andheiberg avatar andheiberg commented on May 19, 2024

@laravelbook okay cool that we understand each other. Thanks for fixing this. I was so tired the day I wrote the original comment so I could find a good implementation for the fix. Apparently also to tired to make my self understandable :D.

Great that you implemented it so promptly!

from ardent.

markalanevans avatar markalanevans commented on May 19, 2024

@laravelbook So when i want to use update my users model which has password|required|confirmed

It fails because one, of course there is no password_confirmed field as i'm just for example wanting to change the email address.

What configuration do i set so that the rules are only validating on create no on update. Or something along those lines...

from ardent.

greatwitenorth avatar greatwitenorth commented on May 19, 2024

@ markalanevans I'm having the exact same issue. Did you ever solve this?

from ardent.

markalanevans avatar markalanevans commented on May 19, 2024

@greatwitenorth

I told it to purge redundent attributes.
https://github.com/laravelbook/ardent#purge

from ardent.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.