Improved vote accessability for packages

[Baxter900]

This is a modification which adds various callback hooks for the voting system.

It adds the callbacks:

cancelUpvoteCallbacks
cancelDownvoteCallbacks
upvoteMethodCallbacks
downvoteMethodCallbacks
cancelUpvoteMethodCallbacks
cancelDownvoteMethodCallbacks

To the existing callbacks of:

upvoteCallbacks
downvoteCallbacks

The advantage of the Method callbacks over the original ones is that the original ones ran asynchronously server-side, wheras the Method callbacks will hold up the vote action until the code in the callbacks finishes, which may be useful in some cases, and they run on both the server and client.

The original names were left consistent for backwards compatibility, but it may be worth considering changing the names to something that makes more sense, like upvoteServerCallbacks or upvoteAsyncCallbacks.

The other modification is a hook which allows for one to set an equation which will determine the power of a vote from within a package. This is done by, within the package, setting votePowerEq equal to a function which returns the value, such as this:

votePowerEq = function (user) {
  var power = user.karma/10 + 1;
  if(isAdmin(user)){
    power *= 2;
  }
  return Math.round(power);
};

Then the package must re-export votePowerEq. It's worth noting that the last package to load will be the one that has the equation chosen, if there are multiple ones, but really you should be designing your packages modularly enough that you should be able to remove the package which is conflicting with the one you want to work. If a function is not defined, then the vote power defaults to one. This can be changed from within the vote.js code easily.

[SachaG]

Really cool, thanks!

I agree the current naming scheme for callbacks could be improved. Let's leave it like this for now, and rethink the whole thing down the road, including the names of callbacks for post submission and edition.

[SachaG]

Actually, I'm not so sure about upvoteMethodCallbacks and downvoteMethodCallbacks. Do you have a specific use case for wanting to hold up the upvote or downvote operation?

Also, if you do want to hold it up, wouldn't it make more sense to have these callbacks run before the whole update operation?

Unless I'm missing something, the way you have it now you can't really affect or prevent the operation from within the callback, since it runs afterwards.

[SachaG]

(I'll keep the code as it is until we decide something, but I'll wait a little before documenting these callbacks in case we change them).

[Baxter900]

Alright, sounds good.

I can think of a few cases where I'd essentially want to know that the voting hadn't finished before doing something. For example, if I wanted to implement the ability to be able to choose the power of my vote (I.E I can choose to vote with a vote of 3 power to give my full support, 2 for partial, 1 for a little bit) then I would need to use this function, as I'd need to be able ensure that the voting operation hasn't finished yet.

Now that I think about it though, I think I implemented it afterwards, didn't I....?

The other big benefit of a method callback existing is it allows client side operations to be called. Ideally there would be a client only callback, but I couldn't find a way to implement that which I thought would retain the usefulness of the command.

Maybe moving the method callback to be before the voting and adding a client callback for after the voting might make the callbacks more relevant.

Regardless, I think you're right about needing to move it before the voting function to be useful.

[SachaG]

Makes sense, glad we're on the same page :)

Regarding client-side only callbacks, I'd probably add them in https://github.com/TelescopeJS/Telescope/blob/devel/client/views/posts/modules/post_upvote.js somewhere. I'm not sure they're entirely necessary right now though, if someone really wants to do a custom interaction they can just implement their own event helper.

[Baxter900]

Sounds good!

Would you like me to move the method callback to occur before the voting takes place and then you can do another merge? Or do you want to just move it, since it's pretty much a copy and paste.

One thing I thought of though, if we want one to be able to change the vote power with the methodCallback, it would make more sense to do it entirely through that. For example, once the upvoteMethodCallback is called, the way it is set up now you would run whatever function you want to check before, then you would create a function which would return that variable, then set votePowerEq to that function. It's a bit convoluted. However, once upvoteMethodCallback is before the vote, you could instead have votePowerEq be a number instead of a function, then run your function to determine what you want, as the upvoteMethodCallback has the User variable, and other variables. and set votePowerEq (probably better to just call it votePower if it's no longer a function) to the end result. Anyway, tell me what you think, I can implement that change easily enough if you think it's a good idea. 

[SachaG]

Sure, another PR would be great.

I kind of like having votePowerEq be its own function (although I'd probably just rename it to getVotePower) since it gives people a single place in which to define the vote power algorithm.

Then again, I agree it's a bit redundant if you can also modify the vote power in the callback. It's a bit hard to say which approach is best because so far nobody has really needed to modify that part of the app.

So we can always leave both methods in for now and see how things evolve.

[Baxter900]

So the main reason I turned votePowerEq into another equation, instead of just having it override getVotePower was that it allowed me to check that it was actually a function. That made sense at the time, but looking back, I'm realizing that it would be better to just let it throw an error, so I think I'll do that. I'll also look and see if I can make a slight tweak so setting getVotePower to a number instead of a function would still work. That way it supports both.

List for my own convenience of things todo:

• Move the method callbacks to before the voting code
• Replace votePowerEq with getVotePower, setting getVotePower to naturally be set to one, and allowing to be overridden via a package.
• Change the code which calls getVotePower to run it as a function expecting a number if it is one, to simply take the number if it's already a number.
• Create a unique error to throw for conveniences sake to notify people if getVotePower is not either a function, or a type that can be converted to a number.