From 9ba7ffe6bb8e246451adeac743a29d9608571fbf Mon Sep 17 00:00:00 2001 From: abaisero Date: Sun, 10 Apr 2022 17:56:43 -0400 Subject: [PATCH 1/3] fix: vastly reduces number of db queries Problem: The creation of some tables was causing a large number of queries to be run which itself scaled with the size of the database, i.e., larger databases (such as the one in production) caused more queries to be made the the db itself (a problem which componds on itself). This issue was not apparent in development because of the smaller size of the development db. First, we can verify the above problem by increasing the size of the development db. We can do this by temporarily editing `scripts/entrypoint.sh`, chanding line python make_fake_fixtures.py 1000 1000 1000 > /tmp/fake_agagd_data.json with python make_fake_fixtures.py 100 10000 100 > /tmp/fake_agagd_data.json and then deleting and recreating the docker images/containers/volumes. Before the change, the development database was creating data for 1000 players, 1000 games, and 1000 tournaments. Note that this meant that, on average, each player only played one game and each tournament only contained one game. This was helping mask the primary issue. After the change, the development database is creating 100 players, 10_000 games, and 100 tournaments, meaning that, on average, each player played 100 games, and each tournament contained 100 games. We note that, although this new development db is still significantly smaller than the one in production, the local development app is already having trouble responding to requests in involving player and tournament pages. With the new larger development database in place, we observe the following concerning facts: * a random player page requires ~1000 queries to be made. * a random tournament page requires ~100 queries to be made. Solution: This commit addresses the above issues by making 2 primary changes in how some tables are computed or rendered: 1. The first involves the way player names and ids were rendered in the game tables using the custom `agagd_core.tables.players.LinkFullMembersNameColumn`. This custom column had a `.render()` method which internally made a query to the db. Then, for the rendering of the whole table, the `.render()` method was being called for every entry in the table data, causing many queries to be made. To solve this issue, we removed `LinkFullMembersNameColumn` altogether, and constructed the appropriate queries to construct the "player-name (player-id)" label once for all entries in the table. 2. The second involves the way opponent and tournament data was collected to create the opponent and tournament tables in the player_profile view. This data was collected and manipulated in pure python, using explicit python loops over the Games model, and at least one db query per iteration. To solve this issue, we created appropriate queries which gathered and aggregated the required data using a couple of large queries (rather than many small ones). After the above changes: * the number of queries made in a random player page dropped from ~1000 to ~10. * the number of queries made in a random tournament page dropped from ~100 to ~5. --- agagd/agagd_core/tables/games.py | 63 ++------- agagd/agagd_core/tables/players.py | 25 +++- agagd/agagd_core/tables/tournaments.py | 30 +++-- agagd/agagd_core/views/frontpage.py | 17 +++ agagd/agagd_core/views/players_profile.py | 136 ++++++++++++-------- agagd/agagd_core/views/tournament_detail.py | 24 +++- 6 files changed, 172 insertions(+), 123 deletions(-) diff --git a/agagd/agagd_core/tables/games.py b/agagd/agagd_core/tables/games.py index dcf0fc8d..9a801c01 100644 --- a/agagd/agagd_core/tables/games.py +++ b/agagd/agagd_core/tables/games.py @@ -3,47 +3,14 @@ import django_tables2 as tables -# Column for the Winner of the Game -class LinkFullMembersNameColumn(tables.Column): - def __init__( - self, - color="W", - viewname=None, - urlconf=None, - args=None, - kwargs=None, - current_app=None, - attrs=None, - **extra, - ): - super().__init__( - attrs=attrs, - linkify=dict( - viewname=viewname, - urlconf=urlconf, - args=args, - kwargs=kwargs, - current_app=current_app, - ), - **extra, - ) - self.color = color - - def render(self, value, record): - if record["result"] == self.color: - self.attrs["td"] = {"class": "winner"} - elif record["result"] != self.color: - self.attrs["td"] = {"class": "runner-up"} +def class_player_1(record): + """returns td class for player 1 (white)""" + return "winner" if record["result"] == "W" else "runner-up" - try: - member_name_and_id = agagd_models.Member.objects.values("full_name").get( - member_id=value - ) - value = f"{member_name_and_id['full_name']} ({value})" - except ObjectDoesNotExist: - value = None - return value +def class_player_2(record): + """returns td class for player 2 (black)""" + return "winner" if record["result"] == "B" else "runner-up" # Basic table which is use as as base for many of the game layouts. @@ -54,17 +21,15 @@ class GamesTable(tables.Table): handicap = tables.Column( attrs=django_tables2_styles.default_bootstrap_column_attrs, orderable=False ) - pin_player_1 = LinkFullMembersNameColumn( - color="W", + full_name_and_id_1 = tables.Column( verbose_name="White", - viewname="players_profile", - kwargs={"player_id": tables.A("pin_player_1")}, + linkify=("players_profile", [tables.A("pin_player_1")]), + attrs={"td": {"class": class_player_1}}, ) - pin_player_2 = LinkFullMembersNameColumn( - color="B", + full_name_and_id_2 = tables.Column( verbose_name="Black", - viewname="players_profile", - kwargs={"player_id": tables.A("pin_player_2")}, + linkify=("players_profile", [tables.A("pin_player_2")]), + attrs={"td": {"class": class_player_2}}, ) tournament_code = tables.Column( verbose_name="Tournament", @@ -74,8 +39,8 @@ class GamesTable(tables.Table): class Meta: attrs = django_tables2_styles.default_bootstrap_header_column_attrs fields = ( - "pin_player_1", - "pin_player_2", + "full_name_and_id_1", + "full_name_and_id_2", "tournament_code", "handicap", "game_date", diff --git a/agagd/agagd_core/tables/players.py b/agagd/agagd_core/tables/players.py index a8dfa8c7..a64e1ba8 100644 --- a/agagd/agagd_core/tables/players.py +++ b/agagd/agagd_core/tables/players.py @@ -20,11 +20,12 @@ class Meta: class PlayersOpponentTable(tables.Table): - opponent = tables.Column( + opponent_id = tables.Column( + verbose_name="Opponent", orderable=False, linkify={ "viewname": "players_profile", - "args": [tables.A("opponent.member_id")], + "args": [tables.A("opponent_id")], }, ) total = tables.Column(verbose_name="Games") @@ -32,9 +33,13 @@ class PlayersOpponentTable(tables.Table): lost = tables.Column(verbose_name="Lost") ratio = tables.Column(verbose_name="Rate", default=0, empty_values=(-1,)) + def render_opponent_id(self, record): + opponent_full_name = record["opponent_full_name"] + opponent_id = record["opponent_id"] + return f"{opponent_full_name} ({opponent_id})" + def render_ratio(self, record): ratio = record["won"] / record["total"] - return f"{ratio:.2f}" class Meta: @@ -44,16 +49,24 @@ class Meta: class PlayersTournamentTable(tables.Table): - tournament = tables.Column( - linkify=("tournament_detail", [tables.A("tournament.pk")]) + tournament_code = tables.Column( + verbose_name="Tournament", + linkify=("tournament_detail", [tables.A("tournament_code")]), ) date = tables.Column(default="Unknown") won = tables.Column(verbose_name="Won", default=0) lost = tables.Column(verbose_name="Lost", default=0) + def render_tournament_code(self, record): + tournament_code = record["tournament_code"] + tournament_date = record["tournament_date"] + tournament_total_players = record["tournament_total_players"] + + return f"{tournament_code} - on {tournament_date} with {tournament_total_players} players" + class Meta: attrs = django_tables2_styles.default_bootstrap_header_column_attrs - fields = ("date", "tournament", "won", "lost") + fields = ("date", "tournament_code", "won", "lost") orderable = False template_name = "django_tables2/bootstrap4.html" sequence = fields diff --git a/agagd/agagd_core/tables/tournaments.py b/agagd/agagd_core/tables/tournaments.py index 73a50264..ea754f00 100644 --- a/agagd/agagd_core/tables/tournaments.py +++ b/agagd/agagd_core/tables/tournaments.py @@ -1,7 +1,6 @@ import agagd_core.defaults.styles.django_tables2 as django_tables2_styles import agagd_core.models as agagd_models import django_tables2 as tables -from agagd_core.tables.games import LinkFullMembersNameColumn class TournamentsTable(tables.Table): @@ -51,19 +50,26 @@ class Meta: template_name = "tournament_detail_information.html" +def class_player_1(record): + """returns td class for player 1 (white)""" + return "winner" if record["result"] == "W" else "runner-up" + + +def class_player_2(record): + """returns td class for player 2 (black)""" + return "winner" if record["result"] == "B" else "runner-up" + + class TournamentsGamesTable(tables.Table): - pin_player_1 = LinkFullMembersNameColumn( - color="W", + full_name_and_id_1 = tables.Column( verbose_name="White", - viewname="players_profile", - kwargs={"player_id": tables.A("pin_player_1")}, + linkify=("players_profile", [tables.A("pin_player_1")]), + attrs={"td": {"class": class_player_1}}, ) - - pin_player_2 = LinkFullMembersNameColumn( - color="B", + full_name_and_id_2 = tables.Column( verbose_name="Black", - viewname="players_profile", - kwargs={"player_id": tables.A("pin_player_2")}, + linkify=("players_profile", [tables.A("pin_player_2")]), + attrs={"td": {"class": class_player_2}}, ) def render_result(self, value): @@ -77,8 +83,8 @@ class Meta: attrs = django_tables2_styles.default_bootstrap_header_column_attrs fields = ( "game_date", - "pin_player_1", - "pin_player_2", + "full_name_and_id_1", + "full_name_and_id_2", "handicap", "komi", "result", diff --git a/agagd/agagd_core/views/frontpage.py b/agagd/agagd_core/views/frontpage.py index 64dff6a7..96ec712f 100644 --- a/agagd/agagd_core/views/frontpage.py +++ b/agagd/agagd_core/views/frontpage.py @@ -9,6 +9,9 @@ from agagd_core.tables.players import PlayersTournamentTable from agagd_core.tables.top_players import TopDanTable, TopKyuTable from agagd_core.tables.tournaments import TournamentsTable +from django.db.models import CharField +from django.db.models import Value as V +from django.db.models.functions import Concat from django.shortcuts import render from django.template.response import TemplateResponse @@ -27,6 +30,20 @@ def __get_latest_games(self): "pin_player_2", "tournament_code", "result", + full_name_and_id_1=Concat( + "pin_player_1__full_name", + V(" ("), + "pin_player_1", + V(")"), + output_field=CharField(), + ), + full_name_and_id_2=Concat( + "pin_player_2__full_name", + V(" ("), + "pin_player_2", + V(")"), + output_field=CharField(), + ), ).order_by("-game_date")[:20] return latest_games diff --git a/agagd/agagd_core/views/players_profile.py b/agagd/agagd_core/views/players_profile.py index 728f9f8a..efbb983a 100644 --- a/agagd/agagd_core/views/players_profile.py +++ b/agagd/agagd_core/views/players_profile.py @@ -1,6 +1,3 @@ -# Date Imports -from datetime import date - # AGAGD Models Imports import agagd_core.models as agagd_models from agagd_core.tables.games import GamesTable @@ -14,7 +11,10 @@ # Django Imports from django.core.exceptions import ObjectDoesNotExist -from django.db.models import Q +from django.db.models import Case, CharField, Count, F, IntegerField, Q +from django.db.models import Value as V +from django.db.models import When +from django.db.models.functions import Concat from django.http import Http404 from django.template.response import TemplateResponse from django.views.generic.detail import DetailView @@ -45,56 +45,68 @@ def get(self, request, *args, **kwargs): Q(pin_player__exact=player_id) ).values("pin_player", "rating", "sigma") - # compute additional tables for opponents & tournament info. here - # TODO: refactor this into something nicer. - opponent_data = {} - tourney_data = {} - for game in player_games: - try: - t_dat = tourney_data.get(game.tournament_code.pk, {}) - t_dat["tournament"] = game.tournament_code - t_dat["won"] = t_dat.get("won", 0) - t_dat["lost"] = t_dat.get("lost", 0) - - # Set default game_date to None - game_date = None - - # Check for 0000-00-00 dates - if game.game_date != u"0000-00-00": - game_date = game.game_date - - t_dat["date"] = t_dat.get("date", game_date) - - op = game.player_other_than(player) - opp_dat = opponent_data.get(op, {}) - opp_dat["opponent"] = op - opp_dat["total"] = opp_dat.get("total", 0) + 1 - opp_dat["won"] = opp_dat.get("won", 0) - opp_dat["lost"] = opp_dat.get("lost", 0) - if game.won_by(player): - opp_dat["won"] += 1 - t_dat["won"] += 1 - else: - opp_dat["lost"] += 1 - t_dat["lost"] += 1 - opponent_data[op] = opp_dat - tourney_data[game.tournament_code.pk] = t_dat - except ObjectDoesNotExist: - print("failing game_id: %s" % game.pk) - - opp_table = PlayersOpponentTable(opponent_data.values()) - RequestConfig(request, paginate={"per_page": 10}).configure(opp_table) - - t_table = PlayersTournamentTable( - tourney_data.values(), - sorted( - tourney_data.values(), - key=lambda d: d.get("date", date.today()) or date.today(), - reverse=True, - ), - prefix="ts_played", + # Q objects to select games played by the player ... + Q_played = Q(pin_player_1__exact=player_id) | Q(pin_player_2__exact=player_id) + # ... won by the player ... + Q_won = Q(pin_player_1__exact=player_id, result__exact="W") | Q( + pin_player_2__exact=player_id, result__exact="B" + ) + # ... and lost by the player + Q_lost = Q(pin_player_1__exact=player_id, result__exact="B") | Q( + pin_player_2__exact=player_id, result__exact="W" + ) + + opponents_queryset = ( + agagd_models.Game.objects.filter(Q_played) + .annotate( + opponent_id=Case( + When( + pin_player_1__exact=player_id, + then=F("pin_player_2"), + ), + When( + pin_player_2__exact=player_id, + then=F("pin_player_1"), + ), + output_field=IntegerField(), + ), + opponent_full_name=Case( + When( + pin_player_1__exact=player_id, + then=F("pin_player_2__full_name"), + ), + When( + pin_player_2__exact=player_id, + then=F("pin_player_1__full_name"), + ), + output_field=CharField(), + ), + ) + .values("opponent_id", "opponent_full_name") + .annotate( + won=Count("game_id", filter=Q_won), + lost=Count("game_id", filter=Q_lost), + total=F("won") + F("lost"), + ) + .order_by("-total", "-won") + ) + opponents_table = PlayersOpponentTable(opponents_queryset) + RequestConfig(request, paginate={"per_page": 10}).configure(opponents_table) + + tournaments_queryset = ( + agagd_models.Game.objects.filter(Q_played) + .values("tournament_code") + .annotate( + tournament_date=F("tournament_code__tournament_date"), + tournament_total_players=F("tournament_code__total_players"), + date=F("game_date"), + won=Count("game_id", filter=Q_won), + lost=Count("game_id", filter=Q_lost), + ) + .order_by("-date") ) - RequestConfig(request, paginate={"per_page": 10}).configure(t_table) + tournaments_table = PlayersTournamentTable(tournaments_queryset) + RequestConfig(request, paginate={"per_page": 10}).configure(tournaments_table) player_games_table = GamesTable( player_games.values( @@ -104,6 +116,20 @@ def get(self, request, *args, **kwargs): "pin_player_2", "tournament_code", "result", + full_name_and_id_1=Concat( + "pin_player_1__full_name", + V(" ("), + "pin_player_1", + V(")"), + output_field=CharField(), + ), + full_name_and_id_2=Concat( + "pin_player_2__full_name", + V(" ("), + "pin_player_2", + V(")"), + output_field=CharField(), + ), ) ) @@ -126,7 +152,7 @@ def get(self, request, *args, **kwargs): context["player_rating"] = player_rating[0] context["player_games_table"] = player_games_table context["players_information_table"] = players_information_table - context["player_opponents_table"] = opp_table - context["player_tournaments_table"] = t_table + context["player_opponents_table"] = opponents_table + context["player_tournaments_table"] = tournaments_table return TemplateResponse(request, self.template_name, context) diff --git a/agagd/agagd_core/views/tournament_detail.py b/agagd/agagd_core/views/tournament_detail.py index 7da15778..27d9ebcc 100644 --- a/agagd/agagd_core/views/tournament_detail.py +++ b/agagd/agagd_core/views/tournament_detail.py @@ -3,6 +3,9 @@ TournamentsGamesTable, TournamentsInformationTable, ) +from django.db.models import CharField +from django.db.models import Value as V +from django.db.models.functions import Concat from django.http import Http404 from django.template.response import TemplateResponse from django.views.generic.detail import DetailView @@ -24,7 +27,26 @@ def get(self, request, *args, **kwargs): ) tournament_games = tournament.games_in_tourney.values( - "game_date", "pin_player_1", "pin_player_2", "handicap", "komi", "result" + "game_date", + "pin_player_1", + "pin_player_2", + "handicap", + "komi", + "result", + full_name_and_id_1=Concat( + "pin_player_1__full_name", + V(" ("), + "pin_player_1", + V(")"), + output_field=CharField(), + ), + full_name_and_id_2=Concat( + "pin_player_2__full_name", + V(" ("), + "pin_player_2", + V(")"), + output_field=CharField(), + ), ) tournament_games_table = TournamentsGamesTable(tournament_games) From 4e4b6d61e2195c2814ad98cdbba33410b94c898b Mon Sep 17 00:00:00 2001 From: abaisero Date: Tue, 12 Apr 2022 19:19:20 -0400 Subject: [PATCH 2/3] refactor: simplifies rendering of "player_name (player_id)" in tables Before this commit, the construction of "player_name (player_id)" was performed at the db query level by creating a new composite data field. After this commit, the construction of "player_name (player_id)" is performed post-query as a simpler rendering function based on given fields. --- agagd/agagd_core/tables/games.py | 50 +++++++++++---------- agagd/agagd_core/tables/tournaments.py | 33 +++++++++----- agagd/agagd_core/views/frontpage.py | 28 +++--------- agagd/agagd_core/views/players_profile.py | 29 +++--------- agagd/agagd_core/views/tournament_detail.py | 26 +++-------- 5 files changed, 67 insertions(+), 99 deletions(-) diff --git a/agagd/agagd_core/tables/games.py b/agagd/agagd_core/tables/games.py index 9a801c01..035a533c 100644 --- a/agagd/agagd_core/tables/games.py +++ b/agagd/agagd_core/tables/games.py @@ -3,47 +3,49 @@ import django_tables2 as tables -def class_player_1(record): - """returns td class for player 1 (white)""" +def class_white(record): + """returns td class for white""" return "winner" if record["result"] == "W" else "runner-up" -def class_player_2(record): - """returns td class for player 2 (black)""" +def class_black(record): + """returns td class for black""" return "winner" if record["result"] == "B" else "runner-up" # Basic table which is use as as base for many of the game layouts. class GamesTable(tables.Table): - game_date = tables.Column( - verbose_name="Date", attrs=django_tables2_styles.default_bootstrap_column_attrs + white = tables.Column( + linkify=("players_profile", [tables.A("white")]), + attrs={"td": {"class": class_white}}, ) - handicap = tables.Column( - attrs=django_tables2_styles.default_bootstrap_column_attrs, orderable=False + black = tables.Column( + linkify=("players_profile", [tables.A("black")]), + attrs={"td": {"class": class_black}}, ) - full_name_and_id_1 = tables.Column( - verbose_name="White", - linkify=("players_profile", [tables.A("pin_player_1")]), - attrs={"td": {"class": class_player_1}}, - ) - full_name_and_id_2 = tables.Column( - verbose_name="Black", - linkify=("players_profile", [tables.A("pin_player_2")]), - attrs={"td": {"class": class_player_2}}, - ) - tournament_code = tables.Column( + tournament = tables.Column( verbose_name="Tournament", - linkify=("tournament_detail", [tables.A("tournament_code")]), + linkify=("tournament_detail", [tables.A("tournament")]), ) + def render_white(self, record): + name = record["white_name"] + pin = record["white"] + return f"{name} ({pin})" + + def render_black(self, record): + name = record["black_name"] + pin = record["black"] + return f"{name} ({pin})" + class Meta: attrs = django_tables2_styles.default_bootstrap_header_column_attrs fields = ( - "full_name_and_id_1", - "full_name_and_id_2", - "tournament_code", + "date", + "white", + "black", + "tournament", "handicap", - "game_date", ) model = agagd_models.Game orderable = False diff --git a/agagd/agagd_core/tables/tournaments.py b/agagd/agagd_core/tables/tournaments.py index ea754f00..8728adc9 100644 --- a/agagd/agagd_core/tables/tournaments.py +++ b/agagd/agagd_core/tables/tournaments.py @@ -50,28 +50,37 @@ class Meta: template_name = "tournament_detail_information.html" -def class_player_1(record): +def class_white(record): """returns td class for player 1 (white)""" return "winner" if record["result"] == "W" else "runner-up" -def class_player_2(record): +def class_black(record): """returns td class for player 2 (black)""" return "winner" if record["result"] == "B" else "runner-up" class TournamentsGamesTable(tables.Table): - full_name_and_id_1 = tables.Column( - verbose_name="White", - linkify=("players_profile", [tables.A("pin_player_1")]), - attrs={"td": {"class": class_player_1}}, + white = tables.Column( + linkify=("players_profile", [tables.A("white")]), + attrs={"td": {"class": class_white}}, ) - full_name_and_id_2 = tables.Column( + black = tables.Column( verbose_name="Black", - linkify=("players_profile", [tables.A("pin_player_2")]), - attrs={"td": {"class": class_player_2}}, + linkify=("players_profile", [tables.A("black")]), + attrs={"td": {"class": class_black}}, ) + def render_white(self, record): + name = record["white_name"] + pin = record["white"] + return f"{name} ({pin})" + + def render_black(self, record): + name = record["black_name"] + pin = record["black"] + return f"{name} ({pin})" + def render_result(self, value): if value == "W": return "White Wins" @@ -82,9 +91,9 @@ def render_result(self, value): class Meta: attrs = django_tables2_styles.default_bootstrap_header_column_attrs fields = ( - "game_date", - "full_name_and_id_1", - "full_name_and_id_2", + "date", + "white", + "black", "handicap", "komi", "result", diff --git a/agagd/agagd_core/views/frontpage.py b/agagd/agagd_core/views/frontpage.py index 96ec712f..9e3e2a59 100644 --- a/agagd/agagd_core/views/frontpage.py +++ b/agagd/agagd_core/views/frontpage.py @@ -9,9 +9,7 @@ from agagd_core.tables.players import PlayersTournamentTable from agagd_core.tables.top_players import TopDanTable, TopKyuTable from agagd_core.tables.tournaments import TournamentsTable -from django.db.models import CharField -from django.db.models import Value as V -from django.db.models.functions import Concat +from django.db.models import F from django.shortcuts import render from django.template.response import TemplateResponse @@ -24,26 +22,14 @@ class FrontPageView(DetailView): def __get_latest_games(self): latest_games = agagd_models.Game.objects.values( - "game_date", "handicap", - "pin_player_1", - "pin_player_2", - "tournament_code", "result", - full_name_and_id_1=Concat( - "pin_player_1__full_name", - V(" ("), - "pin_player_1", - V(")"), - output_field=CharField(), - ), - full_name_and_id_2=Concat( - "pin_player_2__full_name", - V(" ("), - "pin_player_2", - V(")"), - output_field=CharField(), - ), + date=F("game_date"), + tournament=F("tournament_code"), + white=F("pin_player_1"), + black=F("pin_player_2"), + white_name=F("pin_player_1__full_name"), + black_name=F("pin_player_2__full_name"), ).order_by("-game_date")[:20] return latest_games diff --git a/agagd/agagd_core/views/players_profile.py b/agagd/agagd_core/views/players_profile.py index efbb983a..7301b7a3 100644 --- a/agagd/agagd_core/views/players_profile.py +++ b/agagd/agagd_core/views/players_profile.py @@ -11,10 +11,7 @@ # Django Imports from django.core.exceptions import ObjectDoesNotExist -from django.db.models import Case, CharField, Count, F, IntegerField, Q -from django.db.models import Value as V -from django.db.models import When -from django.db.models.functions import Concat +from django.db.models import Case, CharField, Count, F, IntegerField, Q, When from django.http import Http404 from django.template.response import TemplateResponse from django.views.generic.detail import DetailView @@ -110,26 +107,14 @@ def get(self, request, *args, **kwargs): player_games_table = GamesTable( player_games.values( - "game_date", "handicap", - "pin_player_1", - "pin_player_2", - "tournament_code", "result", - full_name_and_id_1=Concat( - "pin_player_1__full_name", - V(" ("), - "pin_player_1", - V(")"), - output_field=CharField(), - ), - full_name_and_id_2=Concat( - "pin_player_2__full_name", - V(" ("), - "pin_player_2", - V(")"), - output_field=CharField(), - ), + date=F("game_date"), + tournament=F("tournament_code"), + white=F("pin_player_1"), + black=F("pin_player_2"), + white_name=F("pin_player_1__full_name"), + black_name=F("pin_player_2__full_name"), ) ) diff --git a/agagd/agagd_core/views/tournament_detail.py b/agagd/agagd_core/views/tournament_detail.py index 27d9ebcc..d6e1d628 100644 --- a/agagd/agagd_core/views/tournament_detail.py +++ b/agagd/agagd_core/views/tournament_detail.py @@ -3,9 +3,7 @@ TournamentsGamesTable, TournamentsInformationTable, ) -from django.db.models import CharField -from django.db.models import Value as V -from django.db.models.functions import Concat +from django.db.models import F from django.http import Http404 from django.template.response import TemplateResponse from django.views.generic.detail import DetailView @@ -27,26 +25,14 @@ def get(self, request, *args, **kwargs): ) tournament_games = tournament.games_in_tourney.values( - "game_date", - "pin_player_1", - "pin_player_2", "handicap", "komi", "result", - full_name_and_id_1=Concat( - "pin_player_1__full_name", - V(" ("), - "pin_player_1", - V(")"), - output_field=CharField(), - ), - full_name_and_id_2=Concat( - "pin_player_2__full_name", - V(" ("), - "pin_player_2", - V(")"), - output_field=CharField(), - ), + date=F("game_date"), + white=F("pin_player_1"), + black=F("pin_player_2"), + white_name=F("pin_player_1__full_name"), + black_name=F("pin_player_2__full_name"), ) tournament_games_table = TournamentsGamesTable(tournament_games) From 65649caa4ea047b31c81b7ed2d00a429abb10d26 Mon Sep 17 00:00:00 2001 From: abaisero Date: Sun, 1 May 2022 20:13:57 -0400 Subject: [PATCH 3/3] fix: tournaments table in player profile the previous commit which reduces the number of db queries was constructing tournament tables in player profiles with wrong numbers of won/lost games. This commit fixes that issue. --- agagd/agagd_core/tables/players.py | 8 +-- agagd/agagd_core/views/players_profile.py | 60 ++++++++++++++--------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/agagd/agagd_core/tables/players.py b/agagd/agagd_core/tables/players.py index a64e1ba8..649ecfc7 100644 --- a/agagd/agagd_core/tables/players.py +++ b/agagd/agagd_core/tables/players.py @@ -53,20 +53,20 @@ class PlayersTournamentTable(tables.Table): verbose_name="Tournament", linkify=("tournament_detail", [tables.A("tournament_code")]), ) - date = tables.Column(default="Unknown") + tournament_date = tables.Column(default="Unknown") won = tables.Column(verbose_name="Won", default=0) lost = tables.Column(verbose_name="Lost", default=0) def render_tournament_code(self, record): tournament_code = record["tournament_code"] tournament_date = record["tournament_date"] - tournament_total_players = record["tournament_total_players"] + total_players = record["total_players"] - return f"{tournament_code} - on {tournament_date} with {tournament_total_players} players" + return f"{tournament_code} - on {tournament_date} with {total_players} players" class Meta: attrs = django_tables2_styles.default_bootstrap_header_column_attrs - fields = ("date", "tournament_code", "won", "lost") + fields = ("tournament_date", "tournament_code", "won", "lost") orderable = False template_name = "django_tables2/bootstrap4.html" sequence = fields diff --git a/agagd/agagd_core/views/players_profile.py b/agagd/agagd_core/views/players_profile.py index 7301b7a3..a02c250e 100644 --- a/agagd/agagd_core/views/players_profile.py +++ b/agagd/agagd_core/views/players_profile.py @@ -42,19 +42,10 @@ def get(self, request, *args, **kwargs): Q(pin_player__exact=player_id) ).values("pin_player", "rating", "sigma") - # Q objects to select games played by the player ... - Q_played = Q(pin_player_1__exact=player_id) | Q(pin_player_2__exact=player_id) - # ... won by the player ... - Q_won = Q(pin_player_1__exact=player_id, result__exact="W") | Q( - pin_player_2__exact=player_id, result__exact="B" - ) - # ... and lost by the player - Q_lost = Q(pin_player_1__exact=player_id, result__exact="B") | Q( - pin_player_2__exact=player_id, result__exact="W" - ) - opponents_queryset = ( - agagd_models.Game.objects.filter(Q_played) + agagd_models.Game.objects.filter( + Q(pin_player_1__exact=player_id) | Q(pin_player_2__exact=player_id) + ) .annotate( opponent_id=Case( When( @@ -81,8 +72,16 @@ def get(self, request, *args, **kwargs): ) .values("opponent_id", "opponent_full_name") .annotate( - won=Count("game_id", filter=Q_won), - lost=Count("game_id", filter=Q_lost), + won=Count( + "game_id", + filter=Q(pin_player_1__exact=player_id, result__exact="W") + | Q(pin_player_2__exact=player_id, result__exact="B"), + ), + lost=Count( + "game_id", + filter=Q(pin_player_1__exact=player_id, result__exact="B") + | Q(pin_player_2__exact=player_id, result__exact="W"), + ), total=F("won") + F("lost"), ) .order_by("-total", "-won") @@ -91,16 +90,33 @@ def get(self, request, *args, **kwargs): RequestConfig(request, paginate={"per_page": 10}).configure(opponents_table) tournaments_queryset = ( - agagd_models.Game.objects.filter(Q_played) - .values("tournament_code") + agagd_models.Tournament.objects.filter() + .values("tournament_code", "tournament_date", "total_players") .annotate( - tournament_date=F("tournament_code__tournament_date"), - tournament_total_players=F("tournament_code__total_players"), - date=F("game_date"), - won=Count("game_id", filter=Q_won), - lost=Count("game_id", filter=Q_lost), + won=Count( + "games_in_tourney__game_id", + filter=Q( + games_in_tourney__pin_player_1__exact=player_id, + games_in_tourney__result__exact="W", + ) + | Q( + games_in_tourney__pin_player_2__exact=player_id, + games_in_tourney__result__exact="B", + ), + ), + lost=Count( + "games_in_tourney__game_id", + filter=Q( + games_in_tourney__pin_player_1__exact=player_id, + games_in_tourney__result__exact="B", + ) + | Q( + games_in_tourney__pin_player_2__exact=player_id, + games_in_tourney__result__exact="W", + ), + ), ) - .order_by("-date") + .order_by("-tournament_date") ) tournaments_table = PlayersTournamentTable(tournaments_queryset) RequestConfig(request, paginate={"per_page": 10}).configure(tournaments_table)