Skip to content
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

Displaying Historical Information on a user view #365

Merged
merged 10 commits into from
Dec 5, 2023
Merged

Conversation

AndrewM131
Copy link
Contributor

This pull request addresses issue #300 where it aims to display the correct historical information such as games won and such to a view where any view can access it.

@AndrewM131 AndrewM131 added this to the 2023/Sprint 3 milestone Dec 1, 2023
@AndrewM131 AndrewM131 self-assigned this Dec 1, 2023
@AndrewM131 AndrewM131 linked an issue Dec 1, 2023 that may be closed by this pull request
@AndrewM131
Copy link
Contributor Author

Update: I implemented the code such that there is statistics for matches played, matches won, tournaments played, tournaments won. I added the attributes in views.py in the users folder. I also went into userprofile_detail.html and added a link to the user's stats. The button works and goes to the user's stats. This can be accessed by any user if they go on the user's profile.

@@ -34,6 +34,8 @@
<a href="{% url 'users:add-friend' object.user.pk %}"
class="btn btn-primary">Add Friend</a>
{% endif %}
<a href="{% url 'users:user-history' object.user.pk %}"
class="btn btn-primary">Stats</a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you merge dev, you will see that the Edit Profile and Account buttons are sectioned off separately from the general profile buttons such as the Friend Request buttons and the Stats button. The Friend Request and stats buttons should be wrapped such that they either appear alongside the other buttons or such that there is space between the two rows of buttons.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, I'll add them into the if statements just like I did with the account button so it looks cleaner

Copy link
Contributor

@24raniwalar 24raniwalar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to views and user_history align. The logic for displaying user stats in the user_history page works well, but the button placement is a little odd in userprofile_detail.html. I have added specific comments there to address the button and where it should be placed; other than that this PR LGTM!

Copy link
Contributor

@24raniwalar 24raniwalar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I think the if/else logic in the userprofile_detail.html page can be abbreviated since Stats shows up for all users, but that can be left for future developers to clean up in my opinion. Nice work!

Copy link
Contributor

@elizabethli31 elizabethli31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding back the account info and the stats page display looks really nice! A couple changes related to the way matches and tournaments played is being counted right now.

match_count = Match.objects.filter(players__in=[user]).count()
match_wins = Player.objects.filter(Q(user=user, outcome=Player.WIN) | Q(team=user, outcome=Player.WIN)).count()

tournament_count = Tournament.objects.filter(players__in=[user]).count()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm this isn't quite right because it's just counting if the player is currently in a tournament (could be on going). If you could add another filter where the tournament_end_date < timezone.now() that should make this work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good filter I wasn't aware of, I'll use that

@@ -72,7 +74,23 @@ def user_history(request, pk):
try:
user = User.objects.get(pk=pk)

return render(request, "users/user_history.html", {"user": user})
match_count = Match.objects.filter(players__in=[user]).count()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as for tournaments. I think you might want to use Lobby here instead of match and filter for match_status = 3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll implement it since it has a match_status I can take advantage of

@AndrewM131
Copy link
Contributor Author

I implemented the requested changes, but my only comment is that a user can be in a match if assigned by an admin, and not be in a lobby. I think there should be some functionality in the future to require a lobby for a match just so a person can't have matches won, but no matches played due to it being tied to lobbies.

@24raniwalar
Copy link
Contributor

Added back remove friend functionality; got removed elsewhere in another PR.

Copy link
Contributor

@elizabethli31 elizabethli31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove tournaments related files (just to avoid any accidental merge issues), and I'll approve.

Copy link
Contributor

@elizabethli31 elizabethli31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now! Good work fixing up the matches/tournaments so they are taking in the proper info

@elizabethli31 elizabethli31 merged commit 3b03340 into dev Dec 5, 2023
2 checks passed
@AndrewM131
Copy link
Contributor Author

Closing comment: In this PR, we created a view that can be accessed in the profile of a user that displays the stats of a user, so for example, how many matches a user has been in, how many matches they've won, how many tournaments they've participated in and how many have they won. This is visible to any user when looking at another user's profile. One thing is that match count is linked to lobby, and since an admin can manually add a user to a match without a lobby, there should be something in the future that mandates a lobby for a match such that the numbers stay consistent. Also in the future, there should be more stats, such as rank in Chigame website, and maybe win rate, and things like that.
Team contributions can be found in issue closing statement.

@AndrewM131 AndrewM131 deleted the users/hist-info branch December 6, 2023 00:06
@elizabethli31
Copy link
Contributor

Issue Score: Excellent

Comments:
Great work using models from other teams to complete this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Implement historical information from view
5 participants