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

jb-01 · 2023-11-22T17:56:46Z

Description:

This PR relates to issue #172 and introduces several key updates to the tournament management system, focusing on the capabilities of non-staff users. The changes are aimed at providing non-staff users with greater autonomy in creating and managing tournaments while maintaining necessary oversight and control mechanisms. The updates include modifications to both the front-end (CRUD pages) and the back-end (Tournament model and related views).

Key Changes:

Tournament Creation with Token Usage: Non-staff users can now create tournaments, deducting one token from their account per tournament created. This enforces a token-based permission system, integrating with the User team's token management functionality.
Partial Edit Control for Tournament Creators: Creators of a tournament (non-staff users who created a tournament) have been granted partial editing control over their tournaments. This includes updating certain attributes of the tournament. However, they cannot add or remove matches or players – actions which require staff intervention.
Access Control on Tournaments: Non-staff users can only access tournaments they are part of. The TournamentListView has been modified to filter out tournaments that the user is not participating in, enhancing privacy and focus.
Restriction on Tournament Deletion: Non-staff users are not permitted to delete or disable tournaments they created. Such actions must be routed through staff users due to the complex backend events triggered by a tournament's deletion.
Model Enhancements: The Tournament model has been updated to include a created_by field, ensuring clarity on ownership. This field is critical for implementing the above functionalities, particularly around editing permissions and access control.

Technical Details:

TournamentCreateView and TournamentUpdateView have been modified to check the user's token balance and ownership, respectively. TournamentCreateView deducts one token for each tournament created.
The Tournament model's created_by field links to the User model, establishing tournament ownership.
The get_queryset method in TournamentListView filters tournaments based on user participation and staff status.
distribute_tokens() is a future-proof function for automatically reloading each user’s token balance after 30 days. It has been commented out so that the Users group can make necessary modifications to their User model beforehand.

MuyanXie · 2023-11-23T02:44:25Z

src/chigame/games/views.py

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


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.

MuyanXie · 2023-11-23T02:46:53Z

src/chigame/games/views.py

+
+def distribute_tokens():
+    # Placeholder for future date check
+    # thirty_days_ago = datetime.now() - timedelta(days=30)


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

MuyanXie

Well done! This PR looks good to me. Please check comments in case it might provide some insights. Once you've done documenting the distribute_token method and address the date to add token placeholder, I'll go ahead and approve this PR.

jb-01 · 2023-11-24T19:38:51Z

Thanks for the commentary @MuyanXie. The distribute_token() function is commented out in this PR because I cannot change the User model without the Users group discretion. This prevents me from manually adding a last_token_addition attribute which is necessary for a full implementation of distribute_token(). However, I am leaving it in this PR so that the Users group can easily add it in the future.

MuyanXie · 2023-11-24T20:58:45Z

That makes sense. I'll go ahead and approve this PR

elizabethli31

This is a big PR, and clearly a lot of thought has been put in for all the functionalities. A few of the defined changes aren't properly working (see comments), but really great work with this one so far! Also, make sure you update with dev because I think you might get some migration conflicts.

elizabethli31 · 2023-11-25T20:33:31Z

src/chigame/games/views.py

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


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.

elizabethli31 · 2023-11-25T20:37:41Z

src/chigame/games/views.py

@@ -175,17 +190,34 @@ class TournamentUpdateView(UpdateView):
        "rules",
        "draw_rules",
        "num_winner",
-        "matches",


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.

elizabethli31 · 2023-11-25T20:49:38Z

src/chigame/games/views.py

+        tournament.save()
+
+        # Add the creator to the players list
+        tournament.players.add(user)


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.

…rnaments/nonstaff-create-edit

jb-01 · 2023-11-30T19:26:37Z

Thanks @elizabethli31. I've implemented each of your recommended changes for token subtraction, admin players, and users. I've also committed some changes related to merging with the dev branch.

Please take a look.

elizabethli31

Token usage for creation, access control, and tournament deletion all work well! I would prefer if you could just remove the delete button for non-staff users instead of leading to a forbidden page (or at least it should be redirecting the user back to the tournament detail page with an error message).
A couple of things I'd like to see in the next commit:

Partial edit control needs to be fixed (removing or adding players). See my comment for more details
Instead of taking users to a forbidden page (after trying to create without enough tokens, updating, deleting, etc.), I'd like for it to redirect back to the tournaments page instead. Reference some previous PRs from your team for code that includes messages.error and return(redirect)

elizabethli31 · 2023-12-03T00:48:41Z

src/chigame/games/views.py

+            raise PermissionDenied("You do not have permission to edit this tournament.")
+
+        # Continue with the normal flow
+        return super().dispatch(request, *args, **kwargs)

    def form_valid(self, form):


I am getting an error when I try to remove players and save the updated tournament (both as a non-staff and staff).

It's coming from line 357 (related_match = current_tournament.matches.get). This is happening because when I create a tournament, the matches aren't being created for some reason (it could be that your PR isn't up to date because previously this worked fine).

Also, when I try to add players, I am getting "You cannot add new players to the tournament after it has started." even if my tournament has not started yet.

elizabethli31 · 2023-12-03T01:04:10Z

Also please rebase first because Mike and Sam added a pretty significant change with registration periods!

jb-01 · 2023-12-05T05:51:44Z

Hi @elizabethli31 I've implemented the following changes:

Hidden delete button for non-staff users to avoid forbidden page.
Fixed partial edit control for adding/removing players in tournaments.
Implemented redirection to tournaments page with error messages instead of forbidden page.
Corrected validation for adding players to unstarted tournaments.

elizabethli31

Token usage works and errors are handled well! I'm still able to add players even if I'm not a staff. Also, please add a check somewhere in the detail view that doesn't allow players to access tournaments they are not in. Even though the tournaments are hidden from the list view, players can still access the detail view of them just by going to the specific url. Unless the functionality you want is for players to be able to find them somehow but not view them as a list. Additionally, on some pages, I'm seeing two update buttons which I think is the result of a merge with dev. Could you fix that too?

elizabethli31 · 2023-12-05T16:58:12Z

src/chigame/games/views.py

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


You need to check somewhere here that I am staff to edit the players too. Currently, I'm able to update the players even if I'm not staff

jb-01 · 2023-12-06T01:25:38Z

Hello @elizabethli31 please see the changes to my migrations and views page for the requested changes.

elizabethli31

LGTM!

jb-01 · 2023-12-06T23:41:16Z

Closing Statement:

@jb-01 contributed to the code for the models and CRUD pages to directly support non-staff functionality.
@MikeChen012345 advised the addition of the created_by attribute in our updated model classes for tournaments.
@Riyush worked on how to structure the user interactions for staff and non-staff users, specifying the frontend behavior.

@MuyanXie reviewed our code and provided feedback related to properly distributing tokens.

elizabethli31 · 2023-12-09T21:15:19Z

Issue Score: Satisfactory

Comments:
Great work implementing this PR. Points are docked for the PR being rubber-stamped, as I had to do a significant amount of testing and requesting changes after it was approved.

jb-01 added 3 commits November 21, 2023 22:40

updated CRUD pages

d7aff1f

updated CRUD pages

c354bec

updated CRUD pages

2b4c5c7

jb-01 requested review from MuyanXie and elizabethli31 November 22, 2023 17:56

jb-01 assigned jb-01, MikeChen012345 and MuyanXie Nov 22, 2023

updated CRUD pages

1da52e2

jb-01 added the x/tournaments label Nov 22, 2023

jb-01 assigned Riyush and unassigned MuyanXie Nov 22, 2023

MuyanXie reviewed Nov 23, 2023

View reviewed changes

MuyanXie requested changes Nov 23, 2023

View reviewed changes

MuyanXie self-requested a review November 24, 2023 20:57

MuyanXie approved these changes Nov 24, 2023

View reviewed changes

elizabethli31 requested changes Nov 25, 2023

View reviewed changes

MuyanXie linked an issue Nov 28, 2023 that may be closed by this pull request

Add support for non-admin users to create or edit tournaments #172

Closed

MuyanXie added this to the 2023/Sprint 3 milestone Nov 28, 2023

jb-01 added 5 commits November 30, 2023 12:43

tokens, users, and adding players issues fixed

8068409

Merge branch 'dev' into tournaments/nonstaff-create-edit

a9f8ee0

merge issues

4dfc17c

Merge branch 'dev' of https://github.com/uchicago-cs/chigame into tou…

acec84d

…rnaments/nonstaff-create-edit

merge with dev

12678f3

elizabethli31 requested changes Dec 3, 2023

View reviewed changes

jb-01 added 5 commits December 4, 2023 23:12

issues addressed

5ed8686

issues addressed

1746964

issues addressed

85e8c28

Merge branch 'dev' into tournaments/nonstaff-create-edit

d7c8d06

issues addressed

f7f31d8

jb-01 added 2 commits December 4, 2023 23:54

Merge branch 'dev' into tournaments/nonstaff-create-edit

8925aac

pass tests

aeb28c8

elizabethli31 requested changes Dec 5, 2023

View reviewed changes

jb-01 added 6 commits December 5, 2023 18:32

issues

b2e3cd5

Merge branch 'dev' into tournaments/nonstaff-create-edit

ae99927

merge

098d96f

migrations

f58270a

migrations

73fd459

remove DS_Store

b3e979f

jb-01 and others added 2 commits December 5, 2023 19:32

issues

0955080

Merge branch 'dev' into tournaments/nonstaff-create-edit

ce705b9

elizabethli31 approved these changes Dec 6, 2023

View reviewed changes

elizabethli31 merged commit b142257 into dev Dec 6, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jb-01 commented Nov 22, 2023

MuyanXie Nov 23, 2023

MuyanXie Nov 23, 2023

MuyanXie left a comment

jb-01 commented Nov 24, 2023

MuyanXie commented Nov 24, 2023

elizabethli31 left a comment

elizabethli31 Nov 25, 2023

elizabethli31 Nov 25, 2023

elizabethli31 Nov 25, 2023

jb-01 commented Nov 30, 2023

elizabethli31 left a comment

elizabethli31 Dec 3, 2023

elizabethli31 Dec 3, 2023

elizabethli31 commented Dec 3, 2023

jb-01 commented Dec 5, 2023

elizabethli31 left a comment •

edited

Loading

elizabethli31 Dec 5, 2023

jb-01 commented Dec 6, 2023

elizabethli31 left a comment

jb-01 commented Dec 6, 2023

elizabethli31 commented Dec 9, 2023

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


		def distribute_tokens():

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

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

Conversation

jb-01 commented Nov 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MuyanXie left a comment

Choose a reason for hiding this comment

jb-01 commented Nov 24, 2023

MuyanXie commented Nov 24, 2023

elizabethli31 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jb-01 commented Nov 30, 2023

elizabethli31 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elizabethli31 commented Dec 3, 2023

jb-01 commented Dec 5, 2023

elizabethli31 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jb-01 commented Dec 6, 2023

elizabethli31 left a comment

Choose a reason for hiding this comment

jb-01 commented Dec 6, 2023

elizabethli31 commented Dec 9, 2023

elizabethli31 left a comment •

edited

Loading