Skip to content

Commit

Permalink
Fix issue where users could comment on accepted/rejected submissions fix
Browse files Browse the repository at this point in the history 
 #31
  • Loading branch information
@aaronbushnell
aaronbushnell committed Aug 7, 2014
1 parent a744500 commit a5bee68
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions client/views/posts/post_page.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,14 @@ Template.post_page.helpers({
return html_body.autoLink();
},
canComment: function(){
return canComment(Meteor.user());
var isAccepted = this.status === STATUS_IMPLEMENTED;

This comment has been minimized.

Sign in to view

Copy link
@jakerella

jakerella Aug 7, 2014

Where are these variables coming from? (STATUS_*)

This comment has been minimized.

Sign in to view

Copy link
@aaronbushnell

aaronbushnell Aug 7, 2014

Author

These are just global variables for the numeric values for each status. Found here

This comment has been minimized.

Sign in to view

Copy link
@jakerella

jakerella Aug 7, 2014

I guess it's confusing since these globals are not defined in this file. Let's add a comment identifying where they came from.
var isRejected = this.status === STATUS_REJECTED;

if (canComment(Meteor.user()) && !isAccepted && !isRejected) {
return true;

This comment has been minimized.

Sign in to view

Copy link
@jakerella

jakerella Aug 7, 2014

Should we be returning false otherwise?

This comment has been minimized.

Sign in to view

Copy link
@aaronbushnell

aaronbushnell Aug 7, 2014

Author

Good question! I just assumed false was implied. Would it be better practice for this to be written as:

if (canComment(Meteor.user()) && !isAccepted && !isRejected) {
  return true;
} else {
  return false;
}

This comment has been minimized.

Sign in to view

Copy link
@jakerella

jakerella Aug 7, 2014

Nope, false is not implied. If a function has no return statement (as in your case when the condition is falsy) then the returned value is undefined. This is usually fine as it resolves to a "falsy" condition, but not if the calling code is checking explicitly for true or false.

The code you have in your comment should be good.
}
}
});
});

Template.post_page.rendered = function(){
if((scrollToCommentId=Session.get('scrollToCommentId')) && !this.rendered && $('#'+scrollToCommentId).exists()){
Expand Down

0 comments on commit a5bee68

Please sign in to comment.