Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access to instance inside LifeCycle Callback #5764

Closed
shavyg2 opened this issue Nov 10, 2015 · 5 comments
Closed

Access to instance inside LifeCycle Callback #5764

shavyg2 opened this issue Nov 10, 2015 · 5 comments

Comments

@shavyg2
Copy link

shavyg2 commented Nov 10, 2015

I have an issue that i thought would be pretty simple to get around but i am having some trouble. I don't know if this is the fault of the waterline or i am misunderstanding something.

I have a use case like this

{
   attributes:{
      admin:{
          type:"boolean",
      },

      makeAdmin:function(){
          //some validation if this is allowed
          //if allowed
          this.admin=true
          this._admin=true
      }
},

   beforeValidate:function(values,cb){
       if(values.admin){
            if(!this._admin){
               /* 
               a user can't simply make a another 
               user admin without being checked inside makeadmin
               they would need to have both admin and _admin 
               set which the function does.
               however the 'this' is not the instance of the model
               */
                values.admin=false 
            }
       }
}   
}

is there a way to have access to the instance of the model? this is because there are values that are on the model that are not persisted but are needed for the validation of the model.

This approach may not be the greatest and it can be done in a controller yes. But this is just to validate my model.

I can do validation in the controller and then validation on the model, but it seems unneeded to do this. I would much rather do the validation all in one place. Waterline Validation of the model is already pretty good and allows so much flexibility why would i off shoot one extra validation outside. Doesn't make much sense.

@shavyg2
Copy link
Author

shavyg2 commented Nov 10, 2015

I have gotten around this by doing this

admin:{
      type:"boolean",
      defaultsTo:false,
      boolean:function(){
        log.info("using ugly method to valid");
        if(!this._admin){
          log.info("the user doesn't have permission to change admin status");
          this.admin=false;
        }else{
          log.info("the user has permission to change the status");
        }
        return true;
      }
},

this was done on the admin attribute because i have access to the instances here. but it's a improper use of the attributes validation because i am not checking if it is boolean and i am always returning true.

I just don't understand why i can have access to the instance here but not inside the beforeValidate function

@particlebanana
Copy link
Contributor

So the this context in instance methods (makeAdmin) is the values. This allows you to do this.userName, etc. The this context in the lifecycle callback is the generic collection. This could be changed in the future, I don't see a large issue with that but would be a breaking change. Mind submitting a PR to the Roadmap file with this on the backlog? Could be a fairly simple PR for someone to tackle.

@sailsbot
Copy link

Thanks for posting, @shavyg2. I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

  • review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
  • create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

@devinivy
Copy link

Reopening until this goes into the backlog.

@devinivy devinivy reopened this Dec 22, 2015
@particlebanana
Copy link
Contributor

Added to the roadmap

@johnabrams7 johnabrams7 transferred this issue from balderdashy/waterline May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants