-
Notifications
You must be signed in to change notification settings - Fork 0
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
Modified code in src/topics to allow users to mark a question as "Good." #15
Conversation
LGTM |
Code is well formatted! Maybe add some comments. |
Looks good, I agree with Jackson though - having comments would help the rest of us contribute later |
0c57c00
to
8d6a5a3
Compare
Pull Request Test Coverage Report for Build 13578493723Details
💛 - Coveralls |
@@ -116,6 +116,28 @@ module.exports = function (Posts) { | |||
} | |||
} | |||
|
|||
Posts.goodquestion = async function (pid, uid) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to make a comment here to explain what the Posts.goodquestion do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
nodebb-theme-harmony/scss/topic.scss
Outdated
[component="post/upvote"], | ||
[component="post/downvote"], | ||
[component="post/goodquestion"] { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove extra line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -6,7 +6,7 @@ | |||
"group-privileges": "Group Privileges", | |||
"user-privileges": "User Privileges", | |||
"edit-privileges": "Edit Privileges", | |||
"select-clear-all": "Select/Clear All", | |||
"select-clear-all": "Select/Clear All", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could probably mimic these changes in the en-UK version of the .json too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! You can go ahead and merge.
Files edited:
nodebb-theme-harmony/scss/topic.scss - Added the post/goodquestion topic.
nodebb-theme-harmony/templates/partials/topic/post.tpl - Added the good question button on the front-end.
public/language/en-US/admin/manage/privileges.json and public/language/en-US/topic.json - Added button name.
src/posts/votes.js - Added the ability to mark the question as Good as a function.
src/privileges/topics.js, src/upgrades/1.7.4/vote_privilege.js - Give users admin access to mark a question as Good.
test/goodQuestion.js - Added comprehensive testing for this feature (marking, unmarking).
Feature was tested by running npm run lint and test to ensure that this did not break any other functionality in NodeBB.
Resolves #2