rename Meeting#can_give_kudo? to given_kudo?

[pdg137]

...since it does not actually check whether they can currently give a kudo (which depends on whether open_kudos! has been run), and to make it parallel to Topic#given_kudo?

[pdg137]

(The logic was also correspondingly inverted, of course)

[alexpeachey]

I'm not a fan of this change. With this rename we introduce the need for a !.

[pdg137]

That's because the previous function name, can_give_kudo? was wrong - it should have actually been called has_not_given_kudo?, but I think the ! is preferable to a name like that. Do you have a better suggestion for a name?

[alexpeachey]

I don't see why can_give_kudo? is a wrong name. We want to show the buttons if the user can give a kudo so it makes sense to ask can_give_kudo? and makes less sense to ask !given_kudo?.

So at the outer layer, it makes sense to ask can the user give a kudo. Then inside, we need to implement that. Ok, so how do we define if a user can give a kudo? We define a user as able to give a kudo if they have not given a kudo to any of the topics. So of course it makes sense to define this as topics.none? {|t| t.given_kudo?(user)}.

So inside asking on a particular topic if a user has given a kudo is a proper thing to ask while outside, since we are discussing things at a meeting level and checking whether a user can give a kudo, the proper question is to ask if the user can_give_kudo?, rather than check if they have !given_kudo?.

[pdg137]

I totally agree that "can the user give a kudo?" is the right question to ask, but the method we have that actually asks that question happens to be called kudos_available?. When can_give_kudo? returns true, the user usually still can't give a kudo, because the meeting is not open for kudos, which is why I say the name is wrong.

[jarhart]

I'm with Paul on this one, but kudos_available? should actually be called can_give_kudo?.

[pdg137]

It is fine with me if you want to do that too; it just seemed like too much for one pull request.