mongoose-voting icon indicating copy to clipboard operation
mongoose-voting copied to clipboard

Add voteScore, fix documentation, make read-only methods virtual

Open idolize opened this issue 10 years ago • 6 comments

  • Addresses issue #5 (vote scores)
  • Adds documentation for the .unvote() method (which I did not know exists until I examined the source code)
  • Switches the methods which just return a count to be virtual getters
    • Provides a nicer syntax. Ex: comment.upvotes instead of comment.upvotes()
  • Updated tests accordingly

idolize avatar Oct 28 '14 00:10 idolize

Thanks man! This is real awesome stuff!

I'm not sure about the virtuals, but it's not much a concern.

This would be a major release, since the API changes.

Would you give me a couple of days to take a look at it?

I may want to address a few ideas and make a 2in1.

If that's ok to you @idolize, ofc.

cristiandouce avatar Oct 28 '14 01:10 cristiandouce

No prob! Yeah, I realize the virtual getters would be a breaking change. You could always support both types, but that may be overkill. Personally (and I realize it is personal preference) I vastly prefer the virtual getter way.

Also, at some point it may make sense to actually serialize the "voteScore" to the DB for querying purposes (rather than using a method or virtual property), but that opens a whole new can of worms. If you can figure that out though it'd be awesome!

idolize avatar Oct 28 '14 01:10 idolize

any updates on this?

bonesoul avatar Nov 02 '15 09:11 bonesoul

Hey @cristiandouce have you had a chance to review this?

timelf123 avatar Dec 17 '15 16:12 timelf123

@timelf123 not really, no... would you like to?

One idea I wanted to explore is to have some sort of scoreFormatter set by the schema configuration. That way the comment.score or whatever is called would return whatever anyone wants. The default would be a simple upvotes - downvotes... although you can notice the simplicity of this operation...

var score = comment.upvotes.length - comment.downvotes.length;

cristiandouce avatar Dec 17 '15 17:12 cristiandouce

@cristiandouce will this ever get merged?

bonesoul avatar May 09 '16 10:05 bonesoul