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

Vote for ideas #51

Merged
merged 5 commits into from
Mar 27, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions apidoc.raml
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,14 @@ types:
403:
404:
get:
/votes:
post:
description: Vote for an idea.
responses:
201:
400:
403:
404:
/comments:
/{id}:
patch:
Expand Down
3 changes: 3 additions & 0 deletions app.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ app.use('/messages', require('./routes/messages'));
app.use('/account', require('./routes/account'));
app.use('/contacts', require('./routes/contacts'));
app.use('/ideas', require('./routes/ideas'));
// vote for ideas, ... TODO to be generalized for not only ideas
app.use('/ideas', require('./routes/votes'));
app.use('/comments', require('./routes/votes'));

// following are route factories
// they need to know what is the primary object (i.e. idea, comment, etc.)
Expand Down
13 changes: 13 additions & 0 deletions collections.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,19 @@ module.exports = {
fields: ['creator']
}
]
},

votes: {
type: 'edge',
from: ['users'],
to: ['ideas', 'comments'],
indexes: [
{
type: 'hash',
fields: ['_from', '_to'],
unique: true
}
]
}

};
Expand Down
17 changes: 17 additions & 0 deletions controllers/comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ module.exports = function controllersFactory(primary) {
* Middleware to read comments of a primary object (i.e. idea)
*/
async function get(req, res, next) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is very nice to have this factory function for this.

const { username } = req.auth;

try {
// gather data
const { id } = req.params;
Expand All @@ -67,6 +70,20 @@ module.exports = function controllersFactory(primary) {
// read the comments from database
const comments = await models.comment.readCommentsOf(primary, { offset, limit, sort });

// read votes of comments
if (comment === 'comment') {
const ids = comments.map(comment => comment.id);
// read votes of the comments from database
const votes = await models.vote.readVotesToMany({ type: 'comments', ids });
const myVotes = await models.vote.readMany({ from: username, to: { type: 'comments', ids } });

// add the votes to their comments
comments.forEach((comment, i) => {
comment.votes = votes[i];
comment.myVote = myVotes[i];
});
}

// serialize the comments
const serializedComments = serialize.comment(comments);

Expand Down
5 changes: 5 additions & 0 deletions controllers/ideas.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,17 @@ async function get(req, res, next) {
try {
// gather data
const { id } = req.params;
const { username } = req.auth;

// read the idea from database
const idea = await models.idea.read(id);

if (!idea) return res.status(404).json({ });

// see how many votes were given to idea and if/how logged user voted (0, -1, 1)
idea.votes = await models.vote.readVotesTo({ type: 'ideas', id });
idea.myVote = await models.vote.read({ from: username, to: { type: 'ideas', id } });

// serialize the idea (JSON API)
const serializedIdea = serialize.idea(idea);

Expand Down
1 change: 1 addition & 0 deletions controllers/validators/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ exports.account = require('./account');
exports.users = require('./users');
exports.tags = require('./tags');
exports.userTags = require('./user-tags');
exports.votes = require('./votes');
exports.params = require('./params');
5 changes: 5 additions & 0 deletions controllers/validators/schema/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ module.exports = {
pattern: '\\S' // at least one non-space character
}
},
vote: {
value: {
enum: [-1, 1]
}
},
query: {
page: {
properties: {
Expand Down
5 changes: 3 additions & 2 deletions controllers/validators/schema/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ const account = require('./account'),
params = require('./params'),
tags = require('./tags'),
userTags = require('./user-tags'),
users = require('./users');
users = require('./users'),
votes = require('./votes');


module.exports = Object.assign({ definitions }, account, authenticate, avatar,
comments, contacts, ideas, ideaTags, messages, params, tags, users, userTags);
comments, contacts, ideas, ideaTags, messages, params, tags, users, userTags, votes);
3 changes: 2 additions & 1 deletion controllers/validators/schema/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ module.exports = {
title: { $ref: 'sch#/definitions/idea/titl' },
detail: { $ref: 'sch#/definitions/idea/detail' },
content: { $ref: 'sch#/definitions/comment/content' },
id: { $ref: 'sch#/definitions/shared/objectId' }
id: { $ref: 'sch#/definitions/shared/objectId' },
voteValue: { $ref: 'sch#/definitions/vote/value' },
};
32 changes: 32 additions & 0 deletions controllers/validators/schema/votes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use strict';

const { id, voteValue: value } = require('./paths');

const postVotes = {
properties: {
params: {
properties: { id },
required: ['id'],
additionalProperties: false
},
body: {
properties: { value },
required: ['value'],
additionalProperties: false
},
required: ['body', 'params']
}
};

const deleteVote = {
properties: {
params: {
properties: { id },
required: ['id'],
additionalProperties: false
},
required: ['params']
}
};

module.exports = { deleteVote, postVotes };
8 changes: 8 additions & 0 deletions controllers/validators/votes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';

const validate = require('./validate-by-schema');

const del = validate('deleteVote');
const post = validate('postVotes');

module.exports = { del, post };
73 changes: 73 additions & 0 deletions controllers/votes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
const path = require('path'),
models = require(path.resolve('./models')),
serializers = require(path.resolve('./serializers'));

/**
* Middleware to POST a vote to idea (and other objects in the future)
*/
async function post(req, res, next) {

// read data from request
const { id } = req.params;
const { value } = req.body;
const { username } = req.auth;

// what is the type of the object we vote for (i.e. ideas, comments, ...)
const primarys = req.baseUrl.substring(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there such a word?

Copy link
Member Author

@mrkvon mrkvon Mar 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt that. Do you have some alternative suggestions?
primary is supposed to mean "object to which the vote belongs" and s is supposed to mean plural of that.

Perhaps objectTypePlural, typeVotedObjects, votedCollectionsName? Any other ideas?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me primarys can stay it doesn't make a confusion (just lingual one) and it is quite clear. I was mostly curious.


try {
// save the vote to database
const vote = await models.vote.create({ from: username, to: { type: primarys, id }, value });
// respond
const serializedVote = serializers.serialize.vote(vote);
return res.status(201).json(serializedVote);
} catch (e) {
// handle errors
switch (e.code) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we not returning any comment for those errors? Is it because it is boring to write or somehow unnecessary with the frontend?

Copy link
Member Author

@mrkvon mrkvon Mar 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added error response body.

To some extent the error codes are self-explanatory. You're right that it is boring to write (and test) and that some error info would actually be useful. I'll fix it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking more generally if there is any standard in ditup regarding this and what is the explanation.
Yes, it is quite clear what I am getting if there is 409 or 201, maybe some 400 validation explanation is useful (and that is double boring).

Copy link
Member Author

@mrkvon mrkvon Mar 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • we are displaying validation errors as they're generated by json-schema
  • 404 errors can be confusing (is it a nonexistent path, or user or what?) and error messages can be useful.
  • there are probably other errors with neglected but potentially useful error body reporting

// duplicate vote
case 409: {
return res.status(409).end();
}
// missing idea
case 404: {
return res.status(404).end();
}
default: {
return next(e);
}
}
}
}

/**
* Middleware to DELETE a vote from an idea (and other objects in the future).
*/
async function del(req, res, next) {

// read data from request
const { id } = req.params;
const { username } = req.auth;

// what is the type of the object we vote for (i.e. ideas, comments, ...)
const primarys = req.baseUrl.substring(1);

try {
// remove the vote from database
await models.vote.remove({ from: username, to: { type: primarys, id } });
// respond
return res.status(204).end();
} catch (e) {
// handle errors
switch (e.code) {
// missing idea
case 404: {
return res.status(404).end();
}
default: {
return next(e);
}
}
}
}

module.exports = { del, post };
5 changes: 3 additions & 2 deletions models/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ const comment = require('./comment'),
model = require('./model'),
tag = require('./tag'),
user = require('./user'),
userTag = require('./user-tag');
userTag = require('./user-tag'),
vote = require('./vote');


const models = {
Expand All @@ -21,4 +22,4 @@ const models = {
}
};

module.exports = Object.assign(models, { comment, contact, idea, ideaTag, message, model, tag, user, userTag });
module.exports = Object.assign(models, { comment, contact, idea, ideaTag, message, model, tag, user, userTag, vote });
Loading