-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
FIX: Don't count the current user in "Others with this badge." #6016
Conversation
You've signed the CLA, misaka4e21. Thank you! This pull request is ready for review. |
@@ -25,7 +25,7 @@ export default Ember.Controller.extend(BadgeSelectController, { | |||
|
|||
@computed("username", "model.grant_count", "userBadges.grant_count") | |||
grantCount(username, modelCount, userCount) { | |||
return username ? userCount : modelCount; | |||
return username ? userCount : modelCount - 1; |
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.
hmm how do we know if the user hass been granted that badge or not?
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.
I added a new computed variable othersCount
to exclude the current user. It will be shown only if canShowOthers
is true.
5207c40
to
0758a33
Compare
@@ -28,6 +28,11 @@ export default Ember.Controller.extend(BadgeSelectController, { | |||
return username ? userCount : modelCount; | |||
}, | |||
|
|||
@computed("model.grant_count") | |||
othersCount(grant_count) { | |||
return grant_count - 1; |
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.
Oh, this is not going to be a simple -1, you could have multiple badges of the type, so you need to subtract the number you have.
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.
Changed.
Should I consider that other people can also have multiple badges per person?
Don't count the current user in "Others with this badge".
b176eb6
to
087a458
Compare
This pull request has been mentioned on Discourse Meta. There might be relevant details there: https://meta.discourse.org/t/count-at-others-with-this-badge-includes-current-user/90089/4 |
https://meta.discourse.org/t/count-at-others-with-this-badge-includes-current-user/90089