adding a `filter:user.create` hook, I have some uncertainties

bentael

see comments, sorry some typos
https://github.com/akhoury/NodeBB/blob/3f844efd70e1bf108ecc3ecd3b11b8c28bc3b367/src/routes/authentication.js#L218

@julian @baris @psychobunny

bentael

or is it better to just pass the IP to User.create and deal with hook in there?
I am a little worried since this would change the method signature, unless you're ok with adding an optional argument at the end. but that's not your style no

User.create = function(username, password, email, options, callback) {
# or just do this
if (typeof options == 'function') {
  callback = options;
  options = {};
}

julian

What's this hook going to accomplish? (Sorry if I'm dense today -- it's Saturday :D)

psychobunny

He wants to create a plugin that detects spambots on registration, in order to do that he needs the user's IP. Ideally (imo) the hook would be inside User.create and not where the route is defined, so he's suggesting adding an optional argument at the end of User.create where he passes in req.ip

I agree that the typeof optionalArg === 'function' thing is something we don't do at all in NodeBB but I'm not against it personally. However, I'm pretty sure User.create is only called in one place so you adding that argument probably won't break anything except your importer maybe

<baris>

@psychobunny there are at least 6 places that call user.create with that said I prefer User.create = function(username, password, email, ip, callback) to passing in options and checking typeof.

psychobunny

haha (totally didn't check lol) yeah what you said there is exactly how either of us three would have gone about doing it I think

julian

Could we refactor User.create so we pass in an options hash instead?

Mostly because twitter reg doesn't send in email, so I feel weird passing in null as an arg... plus, now that's... 5 arguments?

psychobunny

doesn't matter to me. whoever does it first gets to call how it gets done imo (time spent talking about it already > than time it would take to implement it)

bentael

pull#820