fedora-infra / fas

Fedora Account System
https://admin.fedoraproject.org/accounts
GNU General Public License v2.0
40 stars 50 forks source link

show the currently logged in user in the groups user list #228

Closed ryanlerch closed 7 years ago

ryanlerch commented 7 years ago

fixes: #187

Previously, if a user is logged in and looking at a group that they are a member of, they would not be shown in the members list

This makes the user shown in the list of all the users for the current group

pypingou commented 7 years ago

While the fix looks fine in itself, the entire block of code in which it is seems odd to me, especially that else block setting authenticated = member if member != autenticated, somehow this sounds odd.

Maybe that first if could be dropped as well as its corresponding else block.

laxathom commented 7 years ago

Correct, this if statement is here to filter out the logged user who is a member of the group being iterated. As we want to display authenticated's user as well, we can factor this block.

pypingou commented 7 years ago

I'm not even sure how it used to work, since it overrides authenticated https://github.com/ryanlerch/fas/blob/9220a583e9a4e4dfa8ff3e31b7180a5b5d674120/fas/views/groups.py#L207

laxathom commented 7 years ago

member provides relationship that get_user doesn't thus the override.

ryanlerch commented 7 years ago

okies, had a go at refactoring it, but not sure if it is cleaner or not this way. The main issue is that we still need to set is_member and authenticated_membership which are variables we pass to the template.

pypingou commented 7 years ago

Looks good to me :)