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

Enhancements to Tournament Creation and Editing for Non-Staff Users #265

Merged
merged 24 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/chigame/games/migrations/0013_tournament_created_by.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Generated by Django 4.2.4 on 2023-11-22 17:59

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):
dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
("games", "0012_update_tournaments"),
]

operations = [
migrations.AddField(
model_name="tournament",
name="created_by",
field=models.ForeignKey(
null=True,
on_delete=django.db.models.deletion.SET_NULL,
related_name="created_tournaments",
to=settings.AUTH_USER_MODEL,
),
),
]
1 change: 1 addition & 0 deletions src/chigame/games/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ class Tournament(models.Model):
winners = models.ManyToManyField(User, related_name="won_tournaments", blank=True) # allow multiple winners
num_winner = models.PositiveIntegerField(default=1) # number of possible winners for the tournament
players = models.ManyToManyField(User, related_name="joined_tournaments", blank=True)
created_by = models.ForeignKey(User, on_delete=models.SET_NULL, null=True, related_name="created_tournaments")

def get_all_matches(self):
return self.matches.all()
Expand Down
82 changes: 57 additions & 25 deletions src/chigame/games/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@
from django.contrib.auth.mixins import UserPassesTestMixin
from django.core.exceptions import PermissionDenied
from django.db.models import Q
from django.http import HttpResponseRedirect
from django.shortcuts import get_object_or_404, redirect, render, reverse
from django.urls import reverse_lazy
from django.utils.decorators import method_decorator
from django.views.generic import CreateView, DeleteView, DetailView, ListView, UpdateView
from django_tables2 import SingleTableView

from chigame.users.models import User

from .forms import GameForm
from .models import Game, Lobby, Tournament
from .tables import LobbyTable
Expand Down Expand Up @@ -114,18 +117,16 @@ def wrapper(request, *args, **kwargs):

class TournamentListView(ListView):
model = Tournament
queryset = Tournament.objects.prefetch_related("matches").all()
template_name = "tournaments/tournament_list.html"
context_object_name = "tournament_list"

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
# Additional context can be added if needed
return context
def get_queryset(self):
# If the user is staff, show all tournaments
if self.request.user.is_staff:
return Tournament.objects.prefetch_related("matches").all()

# check if user is staff member
def test_func(self):
return self.request.user.is_staff
# For non-staff users, show only tournaments they are part of
return Tournament.objects.prefetch_related("matches").filter(players=self.request.user)


class TournamentDetailView(DetailView):
Expand All @@ -134,7 +135,6 @@ class TournamentDetailView(DetailView):
context_object_name = "tournament"


@method_decorator(staff_required, name="dispatch")
class TournamentCreateView(CreateView):
model = Tournament
template_name = "tournaments/tournament_create.html"
Expand All @@ -148,20 +148,35 @@ class TournamentCreateView(CreateView):
"rules",
"draw_rules",
"num_winner",
"matches",
"players",
]
# Note: "winner" is not included in the fields because it is not
# supposed to be set by the user. It will be set automatically
# when the tournament is over.
# Note: we may remove the "matches" field later for the same reason,
# but we keep it for now because it is convenient for testing.

def form_valid(self, form):
user = self.request.user
if user.tokens < 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like you are removing tokens for staff as well which makes it so staff can only create one tournament. I'm getting the same error regardless of whether I am creating a tournament from a staff account or not. Also, I'd prefer if you had the redirection before the user tries to fill our the form. Maybe you could do if user.tokens and user.is_staff and return redirect immediately.

# Raise a PermissionDenied exception to show the forbidden page
raise PermissionDenied("You do not have enough tokens to create a tournament.")

# Deduct a token from the user's account
user.tokens -= 1
user.save()

# Save the form instance but don't commit to the database yet
tournament = form.save(commit=False)
tournament.created_by = user
tournament.save()

# Add the creator to the players list
tournament.players.add(user)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line seems to be making it so that only the creator is added to the players list.
players = form.cleaned_data['players'] tournament.players.add(*players)
will fix this.


# Redirect to the tournament's detail page
self.object = tournament
return HttpResponseRedirect(self.get_success_url())

def get_success_url(self):
return reverse_lazy("tournament-detail", kwargs={"pk": self.object.pk})
return reverse("tournament-detail", kwargs={"pk": self.object.pk})


@method_decorator(staff_required, name="dispatch")
class TournamentUpdateView(UpdateView):
model = Tournament
template_name = "tournaments/tournament_update.html"
Expand All @@ -175,17 +190,34 @@ class TournamentUpdateView(UpdateView):
"rules",
"draw_rules",
"num_winner",
"matches",
Copy link
Contributor

Choose a reason for hiding this comment

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

Your PR description says staff members should be able to add/remove players, but you removed players and matches, so it doesn't allow for this functionality.

"players",
]
# Note: "winner" is not included in the fields because it is not
# supposed to be set by the user. It will be set automatically
# when the tournament is over.
# Note: we may remove the "matches" field later for the same reason,
# but we keep it for now because it is convenient for testing.

def dispatch(self, request, *args, **kwargs):
# Get the tournament object
tournament = self.get_object()

# Check if the current user is the creator of the tournament
if tournament.created_by != request.user and not request.user.is_staff:
raise PermissionDenied("You do not have permission to edit this tournament.")

# Continue with the normal flow
return super().dispatch(request, *args, **kwargs)

def get_success_url(self):
return reverse_lazy("tournament-detail", kwargs={"pk": self.object.pk})
return reverse("tournament-detail", kwargs={"pk": self.object.pk})


def distribute_tokens():
Copy link
Contributor

Choose a reason for hiding this comment

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

you might want to check this link for an alternative way for implementing distribute token:

https://chat.openai.com/share/e3e3871f-4acd-4959-94a3-d260c84f8ede

No matter which way you choose, please make sure to well-document your approach to the issue this PR is related to.

# Placeholder for future date check
# thirty_days_ago = datetime.now() - timedelta(days=30)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm we don't have to have a placeholder for future date check. We can just manually distribute tokens on a certain date every month to all users. Please see above link for a reference. Once this is addressed, we are good to go

# users = User.objects.filter(tokens__lt=3, last_token_distribution__lt=thirty_days_ago)

users = User.objects.filter(tokens__lt=3)
for user in users:
# Add future logic for updating last_token_distribution
# user.last_token_distribution = datetime.now()
user.tokens += 1
user.save()


@method_decorator(staff_required, name="dispatch")
Expand Down
3 changes: 1 addition & 2 deletions src/templates/tournaments/tournament_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ <h1>{{ tournament.name }}</h1>
<a href="{% url 'users:detail' player.id %}">{{ player.email }}</a>
{% endif %}
</p>
<!-- You can add more fields as needed -->
</li>
{% endfor %}
</ul>
Expand Down Expand Up @@ -67,7 +66,7 @@ <h1>{{ tournament.name }}</h1>
{% else %}
<p>No matches yet.</p>
{% endif %}
{% if user.is_staff %}
{% if request.user == tournament.created_by or request.user.is_staff %}
<p>
<a href="{% url 'tournament-update' tournament.id %}">Update</a>
</p>
Expand Down
12 changes: 8 additions & 4 deletions src/templates/tournaments/tournament_list.html
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,13 @@ <h3>
<ul>
{% for player in tournament.get_all_players %}
<li>
<h3>
<a href="{% url 'users:detail' player.id %}">{{ player.name }}</a>
</h3>
<p>
{% if player.name %}
<a href="{% url 'users:detail' player.id %}">{{ player.name }}</a>
{% else %}
<a href="{% url 'users:detail' player.id %}">{{ player.email }}</a>
{% endif %}
</p>
</li>
{% endfor %}
</ul>
Expand All @@ -41,7 +45,7 @@ <h3>
<li>No tournaments available.</li>
{% endfor %}
</ul>
{% if user.is_staff %}
{% if request.user.is_authenticated %}
<p>
<a href="{% url 'tournament-create' %}">Create a new tournament</a>
</p>
Expand Down