Fix comment endpoints to match ember

[begedin]

This should resolve #340

This one eliminates the tasks/:task_id/comments/:comment_id endpoint, since we don't really need it.

It also fixes the tasks/:task_id/comments endpoint to return all comments for the specified task. Additionally, this change eliminates the comments endpoint (the one that returns all comments for all tasks) by not defining a catch all handle_index function for it. We should not be needing that one..

However, I'm not sure we should merge this. This is what our model hook looks like on the task route in ember:

model(params) {
  let projectId = this.modelFor('project').id;
  let queryParams = {
    projectId,
    number: params.number
  };

  let userId = this.get('currentUser.user.id');

  return RSVP.hash({
    task: this.store.queryRecord('task', queryParams),
    user: isPresent(userId) ? this.store.find('user', userId) : null
  }).then((result) => {
    return RSVP.hash({
      task: result.task,
      comment: this.store.createRecord('comment', { task: result.task, user: result.user }),
      comments: this.store.query('comment', { taskId: result.task.id })
    });
  });
},

So, there are two steps here

Step 1:

• Fetch the task by projectId and number
• Reload the current user, if one already exists - should be no need for that; pass through null otherwise

Step 2

• Pass through the fetched task
• Fetch all comments for fetched task
• Init a new, unsaved comment, and assign it to fetched user

This is atypical behaviour we have here. A simpler approach would be to

• Fetch task by projectId and number
• Later, asynchronously fetch comments using coalesced ids from task.comments relationship, same as any other place we do it in.
• Use current user to init new comment. Can do it via a promise as well, if needed.
• Template can hide new comment form if there is no current user.

Due to all this, I propose that we

• Eliminate the filtering by task id from comments endpoint
• Add filtering by coalesced ids
• Simplify ember so the task route works similarly to other routes that handle relationships (as described above).

@joshsmith Let me know what you think about it. If you don't think such a simplification is needed, then this PR is mergeable.

[coveralls]

Coverage increased (+0.2%) to 95.533% when pulling a08f5b5 on 340-fix-comment-endpoints-to-match-ember into da5867b on develop.

[begedin]

@joshsmith Could use a look here. Not sure if we should merge as is, or switch to what I described.

[joshsmith]

@begedin I think we should switch to what you described. Is that your recommendation?

[begedin]

@joshsmith I would say it is, yes. It's more in line with the rest of the app and simplifies things to a good degree.

[joshsmith]

Okay do you want to update labels then and create an issue in Ember?

[begedin]

Created ember issue code-corps/code-corps-ember#644

[begedin]

Made the modifications here and opened a PR for the ember side of the issue - code-corps/code-corps-ember#647.

Both are ready for review.

[joshsmith]

I don't think this includes changes to the API Blueprint documentation.

[coveralls]

Coverage increased (+0.5%) to 95.364% when pulling 5f0682a on 340-fix-comment-endpoints-to-match-ember into 533668f on develop.

[coveralls]

Coverage increased (+0.2%) to 95.082% when pulling 5f0682a on 340-fix-comment-endpoints-to-match-ember into 533668f on develop.