From 005431d6f6e07ebcba9321c71a3cc45a02bf09c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reynir=20Bj=C3=B6rnsson?= Date: Tue, 24 Aug 2021 18:07:58 +0200 Subject: [PATCH 01/12] team shifts: implement marking shifts for sale A common occurence is that not enough people volunteer for shifts at first, and then the few volunteers take all the shifts (too many). Then late comers aren't able to take any shifts. This commit implements a third state for shift assignment: for sale. A shift marked for sale means the team member is able to take the shift if need be, but would rather someone else takes the shift. Another team member will be able to take shifts at the same time even if the required number of people are met. The current semantics is for-sale shifts are replaced eagerly. Another possible implementation would be to only replace for-sale shifts if the number of required people is met. --- src/teams/models.py | 20 +++++++++++++++++++- src/teams/templates/team_shift_list.html | 8 ++++++-- src/teams/templates/team_user_shifts.html | 6 +++++- src/teams/urls.py | 5 +++++ src/teams/views/shifts.py | 20 ++++++++++++++++++++ 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/teams/models.py b/src/teams/models.py index 6e0862f41..3366926f6 100644 --- a/src/teams/models.py +++ b/src/teams/models.py @@ -401,7 +401,7 @@ class Meta: shift_range = DateTimeRangeField() - team_members = models.ManyToManyField(TeamMember, blank=True) + team_members = models.ManyToManyField(TeamMember, blank=True, through=TeamShiftAssignment) people_required = models.IntegerField(default=1) @@ -420,3 +420,21 @@ def __str__(self): @property def users(self): return [member.user for member in self.team_members.all()] + +class TeamShiftAssignment(CampRelatedModel): + team_shift = models.ForeignKey( + "teams.TeamShift", + on_delete=models.CASCADE, + help_text="The shift", + ) + + team_member = models.ForeignKey( + "teams.TeamMember", + on_delete=models.CASCADE, + help_text="The team member on shift", + ) + + for_sale = models.BooleanField( + default=False, + help_text="Is the shift assignment for sale?", + ) diff --git a/src/teams/templates/team_shift_list.html b/src/teams/templates/team_shift_list.html index 360e66a9a..e8bbf0254 100644 --- a/src/teams/templates/team_shift_list.html +++ b/src/teams/templates/team_shift_list.html @@ -50,7 +50,7 @@

{{ shift.people_required }} {% for member in shift.team_members.all %} - {{ member.user.profile.get_public_credit_name }}{% if not forloop.last %},{% endif %} + {{ member.user.profile.get_public_credit_name }}{% if member.for_sale %} (for sale!){% endif %}{% if not forloop.last %},{% endif %} {% empty %} None! {% endfor %} @@ -71,7 +71,11 @@

href="{% url 'teams:shift_member_drop' camp_slug=camp.slug team_slug=team.slug pk=shift.pk %}"> Unassign me - {% elif shift.people_required > shift.team_members.count %} + + Sell shift + + {% elif shift.people_required > shift.team_members.filter(for_sale=False).count %} Assign me diff --git a/src/teams/templates/team_user_shifts.html b/src/teams/templates/team_user_shifts.html index 745fd2bb6..e3311b903 100644 --- a/src/teams/templates/team_user_shifts.html +++ b/src/teams/templates/team_user_shifts.html @@ -34,6 +34,10 @@

{{ shift.shift_range.upper|date:'H:i' }} + + + Sell shift @@ -45,4 +49,4 @@

-{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/src/teams/urls.py b/src/teams/urls.py index 92ef4e03b..33bdc7c77 100644 --- a/src/teams/urls.py +++ b/src/teams/urls.py @@ -179,6 +179,11 @@ MemberDropsShift.as_view(), name="shift_member_drop", ), + path( + "sell", + MemberSellsShift.as_view(), + name="shift_member_sell", + ), ] ), ), diff --git a/src/teams/views/shifts.py b/src/teams/views/shifts.py index eaa44fc8d..72d0ede97 100644 --- a/src/teams/views/shifts.py +++ b/src/teams/views/shifts.py @@ -274,6 +274,9 @@ def get(self, request, **kwargs): request, template.render(Context({"shifts": overlapping_shifts})) ) else: + # Remove at most one shift assignment for sale + for shift_assignment in shift.team_members.filter(for_sale=True)[:1]: + shift_assignment.delete() shift.team_members.add(team_member) kwargs.pop("pk") @@ -297,6 +300,23 @@ def get(self, request, **kwargs): return HttpResponseRedirect(reverse("teams:shifts", kwargs=kwargs)) +class MemberSellsShift(LoginRequiredMixin, CampViewMixin, View): + + http_methods = ["get"] + + def get(self, request, **kwargs): + shift = TeamShift.objects.get(id=kwargs["pk"]) + team = Team.objects.get(camp=self.camp, slug=kwargs["team_slug"]) + + team_member = TeamMember.objects.get(team=team, user=request.user) + + shift_assignment = shift.team_members.get(team_member = team_member.pk) + shift_assignment.for_sale = True + shift_assignment.save() + + kwargs.pop("pk") + + return HttpResponseRedirect(reverse("teams:shifts", kwargs=kwargs)) class UserShifts(CampViewMixin, TemplateView): template_name = "team_user_shifts.html" From c81dfca9ee7e2c07b0e6c59f9d26bb1fd12ec3ce Mon Sep 17 00:00:00 2001 From: "Rudy (zarya)" Date: Mon, 9 Jun 2025 21:09:31 +0200 Subject: [PATCH 02/12] Cleaned up and made it work, including the migration marigoround --- ...ftassignment_teamshift_team_members_new.py | 33 +++++++++++++ .../0065_teamshiftassignment_migrate_data.py | 19 ++++++++ .../0066_remove_teamshift_team_members.py | 17 +++++++ ...team_members_new_teamshift_team_members.py | 18 +++++++ .../0068_alter_teamshift_team_members.py | 18 +++++++ src/teams/models.py | 48 ++++++++++++------- src/teams/templates/team_shift_list.html | 10 ++-- src/teams/tests/test_shift_views.py | 38 +++++++++++++++ src/teams/urls.py | 1 + src/teams/views/shifts.py | 7 +-- src/utils/bootstrap/base.py | 1 + 11 files changed, 184 insertions(+), 26 deletions(-) create mode 100644 src/teams/migrations/0064_teamshiftassignment_teamshift_team_members_new.py create mode 100644 src/teams/migrations/0065_teamshiftassignment_migrate_data.py create mode 100644 src/teams/migrations/0066_remove_teamshift_team_members.py create mode 100644 src/teams/migrations/0067_rename_team_members_new_teamshift_team_members.py create mode 100644 src/teams/migrations/0068_alter_teamshift_team_members.py diff --git a/src/teams/migrations/0064_teamshiftassignment_teamshift_team_members_new.py b/src/teams/migrations/0064_teamshiftassignment_teamshift_team_members_new.py new file mode 100644 index 000000000..333c2977d --- /dev/null +++ b/src/teams/migrations/0064_teamshiftassignment_teamshift_team_members_new.py @@ -0,0 +1,33 @@ +# Generated by Django 4.2.21 on 2025-06-09 15:29 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0063_alter_team_member_group'), + ] + + operations = [ + migrations.CreateModel( + name='TeamShiftAssignment', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', models.DateTimeField(auto_now_add=True)), + ('updated', models.DateTimeField(auto_now=True)), + ('for_sale', models.BooleanField(default=False, help_text='Is the shift assignment for sale?')), + ('team_member', models.ForeignKey(help_text='The team member on shift', on_delete=django.db.models.deletion.CASCADE, to='teams.teammember')), + ('team_shift', models.ForeignKey(help_text='The shift', on_delete=django.db.models.deletion.CASCADE, to='teams.teamshift')), + ], + options={ + 'abstract': False, + }, + ), + migrations.AddField( + model_name='teamshift', + name='team_members_new', + field=models.ManyToManyField(blank=True, related_name='team_members_new', through='teams.TeamShiftAssignment', to='teams.teammember'), + ), + ] diff --git a/src/teams/migrations/0065_teamshiftassignment_migrate_data.py b/src/teams/migrations/0065_teamshiftassignment_migrate_data.py new file mode 100644 index 000000000..19c42b47a --- /dev/null +++ b/src/teams/migrations/0065_teamshiftassignment_migrate_data.py @@ -0,0 +1,19 @@ +from django.db import migrations, models +import django.db.models.deletion + +def migrate_assignments(apps, schema_editor): + TeamShift = apps.get_model("teams", "TeamShift") + for teamshift in TeamShift.objects.all(): + members = teamshift.team_members.all() + teamshift.team_members_new.set(members) + teamshift.save() + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0064_teamshiftassignment_teamshift_team_members_new'), + ] + + operations = [ + migrations.RunPython(migrate_assignments), + ] diff --git a/src/teams/migrations/0066_remove_teamshift_team_members.py b/src/teams/migrations/0066_remove_teamshift_team_members.py new file mode 100644 index 000000000..cc06a03fe --- /dev/null +++ b/src/teams/migrations/0066_remove_teamshift_team_members.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.21 on 2025-06-09 15:37 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0065_teamshiftassignment_migrate_data'), + ] + + operations = [ + migrations.RemoveField( + model_name='teamshift', + name='team_members', + ), + ] diff --git a/src/teams/migrations/0067_rename_team_members_new_teamshift_team_members.py b/src/teams/migrations/0067_rename_team_members_new_teamshift_team_members.py new file mode 100644 index 000000000..0dda9d3d8 --- /dev/null +++ b/src/teams/migrations/0067_rename_team_members_new_teamshift_team_members.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.21 on 2025-06-09 15:38 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0066_remove_teamshift_team_members'), + ] + + operations = [ + migrations.RenameField( + model_name='teamshift', + old_name='team_members_new', + new_name='team_members', + ), + ] diff --git a/src/teams/migrations/0068_alter_teamshift_team_members.py b/src/teams/migrations/0068_alter_teamshift_team_members.py new file mode 100644 index 000000000..5a7973ea2 --- /dev/null +++ b/src/teams/migrations/0068_alter_teamshift_team_members.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.21 on 2025-06-09 15:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0067_rename_team_members_new_teamshift_team_members'), + ] + + operations = [ + migrations.AlterField( + model_name='teamshift', + name='team_members', + field=models.ManyToManyField(blank=True, through='teams.TeamShiftAssignment', to='teams.teammember'), + ), + ] diff --git a/src/teams/models.py b/src/teams/models.py index e3684098a..34a95dff0 100644 --- a/src/teams/models.py +++ b/src/teams/models.py @@ -548,6 +548,31 @@ class TaskComment( comment = models.TextField() +class TeamShiftAssignment(CampRelatedModel): + """Model for storing the for_sale state of the shift assignment.""" + team_shift = models.ForeignKey( + "teams.TeamShift", + on_delete=models.CASCADE, + help_text="The shift", + ) + + team_member = models.ForeignKey( + "teams.TeamMember", + on_delete=models.CASCADE, + help_text="The team member on shift", + ) + + for_sale = models.BooleanField( + default=False, + help_text="Is the shift assignment for sale?", + ) + + @property + def camp(self) -> Camp: + """All CampRelatedModels must have a camp FK or a camp property.""" + return self.team_shift.camp + + class TeamShift(ExportModelOperationsMixin("team_shift"), CampRelatedModel): """Model for team shifts.""" class Meta: @@ -563,7 +588,7 @@ class Meta: shift_range = DateTimeRangeField() - team_members = models.ManyToManyField(TeamMember, blank=True, through=TeamShiftAssignment) + team_members = models.ManyToManyField("teams.TeamMember", blank=True, through=TeamShiftAssignment) people_required = models.IntegerField(default=1) @@ -583,20 +608,7 @@ def users(self) -> list[TeamMember]: """Returns a list of team members on this shift.""" return [member.user for member in self.team_members.all()] -class TeamShiftAssignment(CampRelatedModel): - team_shift = models.ForeignKey( - "teams.TeamShift", - on_delete=models.CASCADE, - help_text="The shift", - ) - - team_member = models.ForeignKey( - "teams.TeamMember", - on_delete=models.CASCADE, - help_text="The team member on shift", - ) - - for_sale = models.BooleanField( - default=False, - help_text="Is the shift assignment for sale?", - ) + @property + def for_sale_count(self) -> int: + """Returns a count of shifts for sale.""" + return self.team_members.filter(teamshiftassignment__for_sale=False).count diff --git a/src/teams/templates/team_shift_list.html b/src/teams/templates/team_shift_list.html index 701f6600b..abf6443fb 100644 --- a/src/teams/templates/team_shift_list.html +++ b/src/teams/templates/team_shift_list.html @@ -55,11 +55,11 @@

{{ shift.shift_range.upper|date:'H:i' }} - {{ shift.people_required }} + {{ shift.team_members.count }} of {{ shift.people_required }} assigned - {% for member in shift.team_members.all %} - {{ member.user.profile.get_public_credit_name }}{% if member.for_sale %} (for sale!){% endif %}{% if not forloop.last %},{% endif %} + {% for member in shift.teamshiftassignment_set.all %} + {{ member.team_member.user.profile.get_public_credit_name }}{% if member.for_sale %} (for sale!){% endif %}{% if not forloop.last %},{% endif %} {% empty %} None! {% endfor %} @@ -82,9 +82,9 @@

- Sell shift + Mark for sale - {% elif shift.people_required > shift.team_members.count %} + {% elif shift.people_required > shift.for_sale_count %} Assign me diff --git a/src/teams/tests/test_shift_views.py b/src/teams/tests/test_shift_views.py index a7fb212af..37a069694 100644 --- a/src/teams/tests/test_shift_views.py +++ b/src/teams/tests/test_shift_views.py @@ -209,6 +209,7 @@ def test_team_shift_actions(self) -> None: ) team_shift_2.save() + # Test taking a shift url = reverse("teams:shift_member_take", kwargs={ "team_slug": team_shift_1.team.slug, "camp_slug": self.camp.slug, @@ -225,6 +226,7 @@ def test_team_shift_actions(self) -> None: matches = [s for s in rows if "Unassign me" in str(s)] self.assertEqual(len(matches), 1, "team shift assign failed") + # Test taking a double shift url = reverse("teams:shift_member_take", kwargs={ "team_slug": team_shift_1.team.slug, "camp_slug": self.camp.slug, @@ -241,6 +243,42 @@ def test_team_shift_actions(self) -> None: matches = [s for s in rows if "overlapping" in str(s)] self.assertEqual(len(matches), 1, "team shift double assign failed to fail") + # Test selling a shift + url = reverse("teams:shift_member_sell", kwargs={ + "team_slug": team_shift_1.team.slug, + "camp_slug": self.camp.slug, + "pk": team_shift_1.pk, + }) + response = self.client.get( + path=url, + follow=True, + ) + assert response.status_code == 200 + content = response.content.decode() + soup = BeautifulSoup(content, "html.parser") + rows = soup.select_one("table#main_table > tbody > tr:nth-of-type(1) td:nth-of-type(4)") + matches = [s for s in rows if "for sale!" in str(s)] + self.assertEqual(len(matches), 1, "team shift sell failed") + + # Test taking a sellable shift with user1 + self.client.force_login(self.users[1]) # Noc team member + url = reverse("teams:shift_member_take", kwargs={ + "team_slug": team_shift_1.team.slug, + "camp_slug": self.camp.slug, + "pk": team_shift_1.pk, + }) + response = self.client.get( + path=url, + follow=True, + ) + assert response.status_code == 200 + content = response.content.decode() + soup = BeautifulSoup(content, "html.parser") + rows = soup.select_one("table#main_table > tbody > tr:nth-of-type(1) td:nth-of-type(5)") + matches = [s for s in rows if "Unassign me" in str(s)] + self.assertEqual(len(matches), 1, "team shift assign failed") + + # Test dropping a shift url = reverse("teams:shift_member_drop", kwargs={ "team_slug": team_shift_1.team.slug, "camp_slug": self.camp.slug, diff --git a/src/teams/urls.py b/src/teams/urls.py index 2db849d6f..36730a787 100644 --- a/src/teams/urls.py +++ b/src/teams/urls.py @@ -20,6 +20,7 @@ from teams.views.members import TeamMemberRemoveView from teams.views.members import TeamMembersView from teams.views.shifts import MemberDropsShift +from teams.views.shifts import MemberSellsShift from teams.views.shifts import MemberTakesShift from teams.views.shifts import ShiftCreateMultipleView from teams.views.shifts import ShiftCreateView diff --git a/src/teams/views/shifts.py b/src/teams/views/shifts.py index da8a43cc2..1ad1585ef 100644 --- a/src/teams/views/shifts.py +++ b/src/teams/views/shifts.py @@ -26,6 +26,7 @@ from teams.models import Team from teams.models import TeamMember from teams.models import TeamShift +from teams.models import TeamShiftAssignment from utils.mixins import IsTeamPermContextMixin from .mixins import EnsureTeamLeadMixin @@ -334,8 +335,8 @@ def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: ) else: # Remove at most one shift assignment for sale - for shift_assignment in shift.team_members.filter(for_sale=True)[:1]: - shift_assignment.delete() + for shift_assignment in shift.team_members.filter(teamshiftassignment__for_sale=True)[:1]: + shift.team_members.remove(shift_assignment) shift.team_members.add(team_member) kwargs.pop("pk") @@ -370,7 +371,7 @@ def get(self, request, **kwargs): team_member = TeamMember.objects.get(team=team, user=request.user) - shift_assignment = shift.team_members.get(team_member = team_member.pk) + shift_assignment = TeamShiftAssignment.objects.get(team_member = team_member, team_shift = shift) shift_assignment.for_sale = True shift_assignment.save() diff --git a/src/utils/bootstrap/base.py b/src/utils/bootstrap/base.py index 99a372c70..61144b076 100644 --- a/src/utils/bootstrap/base.py +++ b/src/utils/bootstrap/base.py @@ -2177,6 +2177,7 @@ def bootstrap_tests(self) -> None: self.camp = self.camps[1][0] self.add_team_permissions(self.camp) + self.camp.activate_team_permissions() self.teams = teams[self.camp.camp.lower.year] for member in TeamMember.objects.filter(team__camp=self.camp): member.save() From 43b80bbecb9d21907f445ab48037a32123f23c9d Mon Sep 17 00:00:00 2001 From: "Rudy (zarya)" Date: Tue, 10 Jun 2025 19:40:05 +0200 Subject: [PATCH 03/12] Some more fixes, changed the wording in the button --- src/teams/models.py | 25 +++- src/teams/templates/team_shift_list.html | 12 +- src/teams/tests/test_shift_views.py | 163 ++++++++++++++--------- src/teams/views/shifts.py | 31 ++++- 4 files changed, 150 insertions(+), 81 deletions(-) diff --git a/src/teams/models.py b/src/teams/models.py index 34a95dff0..1e36669da 100644 --- a/src/teams/models.py +++ b/src/teams/models.py @@ -1,4 +1,5 @@ """All models for teams application.""" + from __future__ import annotations import logging @@ -6,14 +7,11 @@ from django.conf import settings from django.contrib.auth.models import Group -from django.contrib.auth.models import Permission -from django.contrib.contenttypes.models import ContentType from django.contrib.postgres.fields import DateTimeRangeField from django.db import models from django.urls import reverse_lazy from django_prometheus.models import ExportModelOperationsMixin -from camps.models import Permission as CampPermission from utils.models import CampRelatedModel from utils.models import CreatedUpdatedModel from utils.models import UUIDModel @@ -58,6 +56,7 @@ class Team(ExportModelOperationsMixin("team"), CampRelatedModel): """Model for team.""" + camp = models.ForeignKey( "camps.Camp", related_name="teams", @@ -211,6 +210,7 @@ class Team(ExportModelOperationsMixin("team"), CampRelatedModel): class Meta: """Meta.""" + ordering: ClassVar[list[str]] = ["name"] unique_together = (("name", "camp"), ("slug", "camp")) @@ -394,6 +394,7 @@ def tasker_permission_set(self) -> str: class TeamMember(ExportModelOperationsMixin("team_member"), CampRelatedModel): """Model for team member.""" + user = models.ForeignKey( "auth.User", on_delete=models.PROTECT, @@ -425,6 +426,7 @@ class TeamMember(ExportModelOperationsMixin("team_member"), CampRelatedModel): class Meta: """Meta.""" + ordering: ClassVar[list[str]] = ["-lead", "-approved"] def __str__(self) -> str: @@ -471,6 +473,7 @@ def update_group_membership(self, deleted=False) -> None: class TeamTask(ExportModelOperationsMixin("team_task"), CampRelatedModel): """Model for team tasks.""" + team = models.ForeignKey( "teams.Team", related_name="tasks", @@ -498,6 +501,7 @@ class TeamTask(ExportModelOperationsMixin("team_task"), CampRelatedModel): class Meta: """Meta.""" + ordering: ClassVar[list[str]] = ["completed", "when", "name"] unique_together = (("name", "team"), ("slug", "team")) @@ -539,6 +543,7 @@ class TaskComment( CreatedUpdatedModel, ): """Model for task comments.""" + task = models.ForeignKey( "teams.TeamTask", on_delete=models.PROTECT, @@ -550,6 +555,7 @@ class TaskComment( class TeamShiftAssignment(CampRelatedModel): """Model for storing the for_sale state of the shift assignment.""" + team_shift = models.ForeignKey( "teams.TeamShift", on_delete=models.CASCADE, @@ -575,8 +581,10 @@ def camp(self) -> Camp: class TeamShift(ExportModelOperationsMixin("team_shift"), CampRelatedModel): """Model for team shifts.""" + class Meta: """Meta.""" + ordering = ("shift_range",) team = models.ForeignKey( @@ -609,6 +617,11 @@ def users(self) -> list[TeamMember]: return [member.user for member in self.team_members.all()] @property - def for_sale_count(self) -> int: - """Returns a count of shifts for sale.""" - return self.team_members.filter(teamshiftassignment__for_sale=False).count + def for_sale_users(self) -> list[TeamMember]: + """Returns a list of team members on this shift.""" + return [member.user for member in self.team_members.filter(teamshiftassignment__for_sale=True)] + + @property + def shifts_available(self) -> int: + """Returns the number of available shifts.""" + return self.team_members.filter(teamshiftassignment__for_sale=False).count() diff --git a/src/teams/templates/team_shift_list.html b/src/teams/templates/team_shift_list.html index abf6443fb..c251519ea 100644 --- a/src/teams/templates/team_shift_list.html +++ b/src/teams/templates/team_shift_list.html @@ -80,11 +80,13 @@

href="{% url 'teams:shift_member_drop' camp_slug=camp.slug team_slug=team.slug pk=shift.pk %}"> Unassign me - - Mark for sale - - {% elif shift.people_required > shift.for_sale_count %} + {% if user not in shift.for_sale_users %} + + Make available + + {% endif %} + {% elif shift.people_required > shift.shifts_available %} Assign me diff --git a/src/teams/tests/test_shift_views.py b/src/teams/tests/test_shift_views.py index 37a069694..a39e5ff8b 100644 --- a/src/teams/tests/test_shift_views.py +++ b/src/teams/tests/test_shift_views.py @@ -31,33 +31,42 @@ def setUpTestData(cls) -> None: def test_team_shift_view_permissions(self) -> None: """Test the team shift view permissions.""" - self.client.force_login(self.users[0]) # Non noc team member + self.client.force_login(self.users[0]) # Non noc team member # Test access control to the views - url = reverse("teams:shift_create", kwargs={ - "team_slug": self.teams["noc"].slug, - "camp_slug": self.camp.slug, - }) + url = reverse( + "teams:shift_create", + kwargs={ + "team_slug": self.teams["noc"].slug, + "camp_slug": self.camp.slug, + }, + ) response = self.client.get(path=url) assert response.status_code == 302 def test_team_user_shift_view(self) -> None: """Test the user shift view.""" - self.client.force_login(self.users[4]) # Noc teamlead - url = reverse("teams:user_shifts", kwargs={ - "camp_slug": self.camp.slug, - }) + self.client.force_login(self.users[4]) # Noc teamlead + url = reverse( + "teams:user_shifts", + kwargs={ + "camp_slug": self.camp.slug, + }, + ) response = self.client.get(path=url) assert response.status_code == 200 def test_team_shift_views(self) -> None: """Test the team shift views.""" - self.client.force_login(self.users[4]) # Noc teamlead + self.client.force_login(self.users[4]) # Noc teamlead # Test creating a shift - url = reverse("teams:shift_create", kwargs={ - "team_slug": self.teams["noc"].slug, - "camp_slug": self.camp.slug, - }) + url = reverse( + "teams:shift_create", + kwargs={ + "team_slug": self.teams["noc"].slug, + "camp_slug": self.camp.slug, + }, + ) response = self.client.get(url) assert response.status_code == 200 @@ -67,7 +76,7 @@ def test_team_shift_views(self) -> None: "from_datetime": self.camp.buildup.lower.date(), "to_datetime": self.camp.buildup.lower + timezone.timedelta(hours=1), "people_required": 1, - }, + }, follow=True, ) assert response.status_code == 200 @@ -84,7 +93,7 @@ def test_team_shift_views(self) -> None: "from_datetime": self.camp.buildup.lower, "to_datetime": self.camp.buildup.lower, "people_required": 1, - }, + }, follow=True, ) assert response.status_code == 200 @@ -101,7 +110,7 @@ def test_team_shift_views(self) -> None: "from_datetime": self.camp.buildup.lower + timezone.timedelta(hours=1), "to_datetime": self.camp.buildup.lower, "people_required": 1, - }, + }, follow=True, ) assert response.status_code == 200 @@ -112,10 +121,13 @@ def test_team_shift_views(self) -> None: self.assertEqual(len(matches), 1, "team shift Start can not be before to end") # Test Creating multiple shifts - url = reverse("teams:shift_create_multiple", kwargs={ - "team_slug": self.teams["noc"].slug, - "camp_slug": self.camp.slug, - }) + url = reverse( + "teams:shift_create_multiple", + kwargs={ + "team_slug": self.teams["noc"].slug, + "camp_slug": self.camp.slug, + }, + ) response = self.client.get(url) assert response.status_code == 200 @@ -127,7 +139,7 @@ def test_team_shift_views(self) -> None: "shift_length": 60, "number_of_shifts": 10, "people_required": 5, - }, + }, follow=True, ) assert response.status_code == 200 @@ -143,11 +155,14 @@ def test_team_shift_views(self) -> None: shift_id = int(shift_link["href"].split("/")[5]) # Test the update view - url = reverse("teams:shift_update", kwargs={ - "team_slug": self.teams["noc"].slug, - "camp_slug": self.camp.slug, - "pk": shift_id, - }) + url = reverse( + "teams:shift_update", + kwargs={ + "team_slug": self.teams["noc"].slug, + "camp_slug": self.camp.slug, + "pk": shift_id, + }, + ) from_datetime = self.camp.buildup.lower to_datetime = from_datetime + timezone.timedelta(hours=2) @@ -160,7 +175,7 @@ def test_team_shift_views(self) -> None: "from_datetime": from_datetime, "to_datetime": to_datetime, "people_required": 2, - }, + }, follow=True, ) assert response.status_code == 200 @@ -168,14 +183,21 @@ def test_team_shift_views(self) -> None: content = response.content.decode() soup = BeautifulSoup(content, "html.parser") row = soup.select_one("table#main_table > tbody > tr:nth-of-type(1) td:nth-of-type(3)").get_text(strip=True) - self.assertEqual(row, "2", "team shift people required count does not return 2 entries after update") + self.assertEqual( + row, + "0 of 2 assigned", + "team shift people required count does not return 2 entries after update", + ) # Test the delete view - url = reverse("teams:shift_delete", kwargs={ - "team_slug": self.teams["noc"].slug, - "camp_slug": self.camp.slug, - "pk": shift_id, - }) + url = reverse( + "teams:shift_delete", + kwargs={ + "team_slug": self.teams["noc"].slug, + "camp_slug": self.camp.slug, + "pk": shift_id, + }, + ) response = self.client.get(url) assert response.status_code == 200 @@ -188,7 +210,7 @@ def test_team_shift_views(self) -> None: def test_team_shift_actions(self) -> None: """Test the team shift actions.""" - self.client.force_login(self.users[4]) # Noc teamlead + self.client.force_login(self.users[4]) # Noc teamlead team_shift_1 = TeamShift( team=self.teams["noc"], @@ -210,11 +232,14 @@ def test_team_shift_actions(self) -> None: team_shift_2.save() # Test taking a shift - url = reverse("teams:shift_member_take", kwargs={ - "team_slug": team_shift_1.team.slug, - "camp_slug": self.camp.slug, - "pk": team_shift_1.pk, - }) + url = reverse( + "teams:shift_member_take", + kwargs={ + "team_slug": team_shift_1.team.slug, + "camp_slug": self.camp.slug, + "pk": team_shift_1.pk, + }, + ) response = self.client.get( path=url, follow=True, @@ -227,11 +252,14 @@ def test_team_shift_actions(self) -> None: self.assertEqual(len(matches), 1, "team shift assign failed") # Test taking a double shift - url = reverse("teams:shift_member_take", kwargs={ - "team_slug": team_shift_1.team.slug, - "camp_slug": self.camp.slug, - "pk": team_shift_2.pk, - }) + url = reverse( + "teams:shift_member_take", + kwargs={ + "team_slug": team_shift_1.team.slug, + "camp_slug": self.camp.slug, + "pk": team_shift_2.pk, + }, + ) response = self.client.get( path=url, follow=True, @@ -243,12 +271,15 @@ def test_team_shift_actions(self) -> None: matches = [s for s in rows if "overlapping" in str(s)] self.assertEqual(len(matches), 1, "team shift double assign failed to fail") - # Test selling a shift - url = reverse("teams:shift_member_sell", kwargs={ - "team_slug": team_shift_1.team.slug, - "camp_slug": self.camp.slug, - "pk": team_shift_1.pk, - }) + # Test selling a shift + url = reverse( + "teams:shift_member_sell", + kwargs={ + "team_slug": team_shift_1.team.slug, + "camp_slug": self.camp.slug, + "pk": team_shift_1.pk, + }, + ) response = self.client.get( path=url, follow=True, @@ -261,12 +292,15 @@ def test_team_shift_actions(self) -> None: self.assertEqual(len(matches), 1, "team shift sell failed") # Test taking a sellable shift with user1 - self.client.force_login(self.users[1]) # Noc team member - url = reverse("teams:shift_member_take", kwargs={ - "team_slug": team_shift_1.team.slug, - "camp_slug": self.camp.slug, - "pk": team_shift_1.pk, - }) + self.client.force_login(self.users[1]) # Noc team member + url = reverse( + "teams:shift_member_take", + kwargs={ + "team_slug": team_shift_1.team.slug, + "camp_slug": self.camp.slug, + "pk": team_shift_1.pk, + }, + ) response = self.client.get( path=url, follow=True, @@ -278,12 +312,15 @@ def test_team_shift_actions(self) -> None: matches = [s for s in rows if "Unassign me" in str(s)] self.assertEqual(len(matches), 1, "team shift assign failed") - # Test dropping a shift - url = reverse("teams:shift_member_drop", kwargs={ - "team_slug": team_shift_1.team.slug, - "camp_slug": self.camp.slug, - "pk": team_shift_1.pk, - }) + # Test dropping a shift + url = reverse( + "teams:shift_member_drop", + kwargs={ + "team_slug": team_shift_1.team.slug, + "camp_slug": self.camp.slug, + "pk": team_shift_1.pk, + }, + ) response = self.client.get( path=url, diff --git a/src/teams/views/shifts.py b/src/teams/views/shifts.py index 1ad1585ef..b635b37b0 100644 --- a/src/teams/views/shifts.py +++ b/src/teams/views/shifts.py @@ -1,4 +1,5 @@ """View for shifts in teams application.""" + from __future__ import annotations from typing import TYPE_CHECKING @@ -41,6 +42,7 @@ class ShiftListView(LoginRequiredMixin, CampViewMixin, IsTeamPermContextMixin, ListView): """Shift list view.""" + model = TeamShift template_name = "team_shift_list.html" context_object_name = "shifts" @@ -67,7 +69,7 @@ def date_choices(camp: Camp) -> list: minute_choices = [] # To begin with we assume a shift can not be shorter than an hour shift_minimum_length = 60 - while index * shift_minimum_length < 60: # noqa: PLR2004 + while index * shift_minimum_length < 60: # noqa: PLR2004 minutes = int(index * shift_minimum_length) minute_choices.append(minutes) index += 1 @@ -95,12 +97,14 @@ def get_time_choices(date: str) -> list: class ShiftForm(forms.ModelForm): """Form for shifts.""" + class Meta: """Meta.""" + model = TeamShift fields = ("from_datetime", "to_datetime", "people_required") - def __init__(self, instance: TeamShift|None=None, **kwargs) -> None: + def __init__(self, instance: TeamShift | None = None, **kwargs) -> None: """Method for setting up the form.""" camp = kwargs.pop("camp") super().__init__(instance=instance, **kwargs) @@ -152,6 +156,7 @@ def save(self, commit=True) -> TeamShift: class ShiftCreateView(LoginRequiredMixin, CampViewMixin, EnsureTeamLeadMixin, IsTeamPermContextMixin, CreateView): """View for creating a single shift.""" + model = TeamShift template_name = "team_shift_form.html" form_class = ShiftForm @@ -185,6 +190,7 @@ def get_success_url(self) -> str: class ShiftUpdateView(LoginRequiredMixin, CampViewMixin, EnsureTeamLeadMixin, IsTeamPermContextMixin, UpdateView): """View for updating a single shift.""" + model = TeamShift template_name = "team_shift_form.html" form_class = ShiftForm @@ -210,6 +216,7 @@ def get_success_url(self) -> str: class ShiftDeleteView(LoginRequiredMixin, CampViewMixin, EnsureTeamLeadMixin, IsTeamPermContextMixin, DeleteView): """View for deleting a shift.""" + model = TeamShift template_name = "team_shift_confirm_delete.html" active_menu = "shifts" @@ -231,7 +238,8 @@ def get_success_url(self) -> str: class MultipleShiftForm(forms.Form): """Form for creating multple shifts.""" - def __init__(self, instance: dict|None=None, **kwargs) -> None: + + def __init__(self, instance: dict | None = None, **kwargs) -> None: """Method for form init setting camp to kwargs.""" camp = kwargs.pop("camp") super().__init__(**kwargs) @@ -250,6 +258,7 @@ def __init__(self, instance: dict|None=None, **kwargs) -> None: class ShiftCreateMultipleView(LoginRequiredMixin, CampViewMixin, EnsureTeamLeadMixin, IsTeamPermContextMixin, FormView): """View for creating multiple shifts.""" + template_name = "team_shift_form.html" form_class = MultipleShiftForm active_menu = "shifts" @@ -305,6 +314,7 @@ def get_context_data(self, **kwargs) -> dict: class MemberTakesShift(LoginRequiredMixin, CampViewMixin, View): """View for adding a user to a shift.""" + http_methods = ("get",) def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: @@ -334,7 +344,8 @@ def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: template.render(Context({"shifts": overlapping_shifts})), ) else: - # Remove at most one shift assignment for sale + # Remove at most one shift assignment for sale if any + # When a shift is for sale and a user presses assign its first assigning the for sale one for shift_assignment in shift.team_members.filter(teamshiftassignment__for_sale=True)[:1]: shift.team_members.remove(shift_assignment) shift.team_members.add(team_member) @@ -346,6 +357,7 @@ def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: class MemberDropsShift(LoginRequiredMixin, CampViewMixin, View): """View for remove a user from a shift.""" + http_methods = ("get",) def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: @@ -361,17 +373,20 @@ def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: return HttpResponseRedirect(reverse("teams:shifts", kwargs=kwargs)) + class MemberSellsShift(LoginRequiredMixin, CampViewMixin, View): + """View for making a shift available for other user to take.""" - http_methods = ["get"] + http_methods = ("get",) - def get(self, request, **kwargs): + def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: + """Method for making a shift available for other user to take.""" shift = TeamShift.objects.get(id=kwargs["pk"]) team = Team.objects.get(camp=self.camp, slug=kwargs["team_slug"]) team_member = TeamMember.objects.get(team=team, user=request.user) - shift_assignment = TeamShiftAssignment.objects.get(team_member = team_member, team_shift = shift) + shift_assignment = TeamShiftAssignment.objects.get(team_member=team_member, team_shift=shift) shift_assignment.for_sale = True shift_assignment.save() @@ -379,8 +394,10 @@ def get(self, request, **kwargs): return HttpResponseRedirect(reverse("teams:shifts", kwargs=kwargs)) + class UserShifts(CampViewMixin, TemplateView): """View for showing shifts for current user.""" + template_name = "team_user_shifts.html" def get_context_data(self, **kwargs) -> dict: From 54ce6fc0fe929ab97696fcd2f7a308a9ff8dae6e Mon Sep 17 00:00:00 2001 From: "Rudy (zarya)" Date: Sun, 22 Jun 2025 09:04:49 +0200 Subject: [PATCH 04/12] Change the shift actions from get to post with a confirm --- .../templates/team_shift_confirm_action.html | 14 +++ src/teams/tests/test_shift_views.py | 17 +++- src/teams/views/shifts.py | 94 ++++++++++++++----- 3 files changed, 96 insertions(+), 29 deletions(-) create mode 100644 src/teams/templates/team_shift_confirm_action.html diff --git a/src/teams/templates/team_shift_confirm_action.html b/src/teams/templates/team_shift_confirm_action.html new file mode 100644 index 000000000..ed4b2242c --- /dev/null +++ b/src/teams/templates/team_shift_confirm_action.html @@ -0,0 +1,14 @@ +{% extends 'team_base.html' %} + +{% block title %} + {{ action }} | {{ block.super }} +{% endblock %} + +{% block team_content %} + +
{% csrf_token %} +

{{ action }}

+ +
+ +{% endblock %} diff --git a/src/teams/tests/test_shift_views.py b/src/teams/tests/test_shift_views.py index a39e5ff8b..5604c1cc7 100644 --- a/src/teams/tests/test_shift_views.py +++ b/src/teams/tests/test_shift_views.py @@ -242,6 +242,10 @@ def test_team_shift_actions(self) -> None: ) response = self.client.get( path=url, + ) + assert response.status_code == 200 + response = self.client.post( + path=url, follow=True, ) assert response.status_code == 200 @@ -260,7 +264,7 @@ def test_team_shift_actions(self) -> None: "pk": team_shift_2.pk, }, ) - response = self.client.get( + response = self.client.post( path=url, follow=True, ) @@ -282,6 +286,10 @@ def test_team_shift_actions(self) -> None: ) response = self.client.get( path=url, + ) + assert response.status_code == 200 + response = self.client.post( + path=url, follow=True, ) assert response.status_code == 200 @@ -301,7 +309,7 @@ def test_team_shift_actions(self) -> None: "pk": team_shift_1.pk, }, ) - response = self.client.get( + response = self.client.post( path=url, follow=True, ) @@ -321,9 +329,12 @@ def test_team_shift_actions(self) -> None: "pk": team_shift_1.pk, }, ) - response = self.client.get( path=url, + ) + assert response.status_code == 200 + response = self.client.post( + path=url, follow=True, ) assert response.status_code == 200 diff --git a/src/teams/views/shifts.py b/src/teams/views/shifts.py index b635b37b0..8c0e6c12b 100644 --- a/src/teams/views/shifts.py +++ b/src/teams/views/shifts.py @@ -36,7 +36,7 @@ from django.db.models import QuerySet from django.http import HttpRequest from django.http import HttpResponse - + from django.forms import ModelForm from camps.models import Camp @@ -312,21 +312,34 @@ def get_context_data(self, **kwargs) -> dict: return context -class MemberTakesShift(LoginRequiredMixin, CampViewMixin, View): +class MemberTakesShift(LoginRequiredMixin, CampViewMixin, UpdateView): """View for adding a user to a shift.""" + model = TeamShift + fields = [] + template_name = "team_shift_confirm_action.html" + context_object_name = "shifts" + active_menu = "shifts" - http_methods = ("get",) + def get_context_data(self, **kwargs) -> dict[str, object]: + """Method for setting the page context data.""" + context = super().get_context_data(**kwargs) + context['action'] = f"Are you sure you want to take this {self.object}?" + context["team"] = Team.objects.get( + camp=self.camp, + slug=self.kwargs["team_slug"], + ) + return context - def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: + def form_valid(self, form: ModelForm[TeamShift]) -> HttpResponseRedirect: """Method for adding user to a shift.""" - shift = TeamShift.objects.get(id=kwargs["pk"]) - team = Team.objects.get(camp=self.camp, slug=kwargs["team_slug"]) + shift = self.object + team = self.object.team - team_member = TeamMember.objects.get(team=team, user=request.user) + team_member = TeamMember.objects.get(team=team, user=self.request.user) overlapping_shifts = TeamShift.objects.filter( team__camp=self.camp, - team_members__user=request.user, + team_members__user=self.request.user, shift_range__overlap=shift.shift_range, ) @@ -340,7 +353,7 @@ def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: """, ) messages.error( - request, + self.request, template.render(Context({"shifts": overlapping_shifts})), ) else: @@ -350,49 +363,78 @@ def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: shift.team_members.remove(shift_assignment) shift.team_members.add(team_member) - kwargs.pop("pk") + self.kwargs.pop("pk") - return HttpResponseRedirect(reverse("teams:shifts", kwargs=kwargs)) + return HttpResponseRedirect(reverse("teams:shifts", kwargs=self.kwargs)) -class MemberDropsShift(LoginRequiredMixin, CampViewMixin, View): +class MemberDropsShift(LoginRequiredMixin, CampViewMixin, UpdateView): + model = TeamShift + fields = [] + template_name = "team_shift_confirm_action.html" + context_object_name = "shifts" + active_menu = "shifts" """View for remove a user from a shift.""" - http_methods = ("get",) + def get_context_data(self, **kwargs) -> dict[str, object]: + """Method for setting the page context data.""" + context = super().get_context_data(**kwargs) + context['action'] = f"Are you sure you want to drop this {self.object}?" + context["team"] = Team.objects.get( + camp=self.camp, + slug=self.kwargs["team_slug"], + ) + return context - def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: + def form_valid(self, form: ModelForm[TeamShift]) -> HttpResponseRedirect: """Method to remove user from shift.""" - shift = TeamShift.objects.get(id=kwargs["pk"]) - team = Team.objects.get(camp=self.camp, slug=kwargs["team_slug"]) + shift = self.object + team = Team.objects.get(camp=self.camp, slug=self.kwargs["team_slug"]) - team_member = TeamMember.objects.get(team=team, user=request.user) + team_member = TeamMember.objects.get(team=team, user=self.request.user) shift.team_members.remove(team_member) - kwargs.pop("pk") + self.kwargs.pop("pk") - return HttpResponseRedirect(reverse("teams:shifts", kwargs=kwargs)) + return HttpResponseRedirect(reverse("teams:shifts", kwargs=self.kwargs)) -class MemberSellsShift(LoginRequiredMixin, CampViewMixin, View): +class MemberSellsShift(LoginRequiredMixin, CampViewMixin, UpdateView): """View for making a shift available for other user to take.""" + model = TeamShift + fields = [] + template_name = "team_shift_confirm_action.html" + context_object_name = "shifts" + active_menu = "shifts" + """View for making a shift available for others shift.""" + + def get_context_data(self, **kwargs) -> dict[str, object]: + """Method for setting the page context data.""" + context = super().get_context_data(**kwargs) + context['action'] = f"Are you sure you want to this {self.object} available to others?" + context["team"] = Team.objects.get( + camp=self.camp, + slug=self.kwargs["team_slug"], + ) + return context http_methods = ("get",) - def get(self, request: HttpRequest, **kwargs) -> HttpResponseRedirect: + def form_valid(self, form: ModelForm[TeamShift]) -> HttpResponseRedirect: """Method for making a shift available for other user to take.""" - shift = TeamShift.objects.get(id=kwargs["pk"]) - team = Team.objects.get(camp=self.camp, slug=kwargs["team_slug"]) + shift = self.object + team = Team.objects.get(camp=self.camp, slug=self.kwargs["team_slug"]) - team_member = TeamMember.objects.get(team=team, user=request.user) + team_member = TeamMember.objects.get(team=team, user=self.request.user) shift_assignment = TeamShiftAssignment.objects.get(team_member=team_member, team_shift=shift) shift_assignment.for_sale = True shift_assignment.save() - kwargs.pop("pk") + self.kwargs.pop("pk") - return HttpResponseRedirect(reverse("teams:shifts", kwargs=kwargs)) + return HttpResponseRedirect(reverse("teams:shifts", kwargs=self.kwargs)) class UserShifts(CampViewMixin, TemplateView): From 076f605c888505249447b958af9185429c196bd6 Mon Sep 17 00:00:00 2001 From: "Rudy (zarya)" Date: Sun, 22 Jun 2025 12:02:56 +0200 Subject: [PATCH 05/12] Add Cancel button --- src/teams/templates/team_shift_confirm_action.html | 1 + 1 file changed, 1 insertion(+) diff --git a/src/teams/templates/team_shift_confirm_action.html b/src/teams/templates/team_shift_confirm_action.html index ed4b2242c..c8f3d2290 100644 --- a/src/teams/templates/team_shift_confirm_action.html +++ b/src/teams/templates/team_shift_confirm_action.html @@ -9,6 +9,7 @@
{% csrf_token %}

{{ action }}

+ Cancel
{% endblock %} From ef56aafafb65f2d034cea1e2dfca3985d839c7fe Mon Sep 17 00:00:00 2001 From: Thomas Steen Rasmussen Date: Sun, 22 Jun 2025 12:42:36 +0200 Subject: [PATCH 06/12] Update src/teams/models.py --- src/teams/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/teams/models.py b/src/teams/models.py index 1e36669da..e29b768d8 100644 --- a/src/teams/models.py +++ b/src/teams/models.py @@ -554,7 +554,7 @@ class TaskComment( class TeamShiftAssignment(CampRelatedModel): - """Model for storing the for_sale state of the shift assignment.""" + """Through model for the shift<>member m2m storing the for_sale state of the shift assignment.""" team_shift = models.ForeignKey( "teams.TeamShift", From 56015c228da716297508524c3ff3ae1296fd4f53 Mon Sep 17 00:00:00 2001 From: Thomas Steen Rasmussen Date: Sun, 22 Jun 2025 12:44:17 +0200 Subject: [PATCH 07/12] Update src/teams/models.py --- src/teams/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/teams/models.py b/src/teams/models.py index e29b768d8..fcde45e76 100644 --- a/src/teams/models.py +++ b/src/teams/models.py @@ -618,7 +618,7 @@ def users(self) -> list[TeamMember]: @property def for_sale_users(self) -> list[TeamMember]: - """Returns a list of team members on this shift.""" + """Returns a list of team members who marked this shift for sale.""" return [member.user for member in self.team_members.filter(teamshiftassignment__for_sale=True)] @property From 506b98848ca1f7f305ecd6a475caeac6d4ab7055 Mon Sep 17 00:00:00 2001 From: "Rudy (zarya)" Date: Sun, 22 Jun 2025 13:15:56 +0200 Subject: [PATCH 08/12] Fix property naming --- src/teams/models.py | 4 ++-- src/teams/templates/team_shift_list.html | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/teams/models.py b/src/teams/models.py index fcde45e76..e51884ff4 100644 --- a/src/teams/models.py +++ b/src/teams/models.py @@ -622,6 +622,6 @@ def for_sale_users(self) -> list[TeamMember]: return [member.user for member in self.team_members.filter(teamshiftassignment__for_sale=True)] @property - def shifts_available(self) -> int: - """Returns the number of available shifts.""" + def shifts_taken(self) -> int: + """Returns the number of taken shifts that are not for sale.""" return self.team_members.filter(teamshiftassignment__for_sale=False).count() diff --git a/src/teams/templates/team_shift_list.html b/src/teams/templates/team_shift_list.html index c251519ea..e5ebfa162 100644 --- a/src/teams/templates/team_shift_list.html +++ b/src/teams/templates/team_shift_list.html @@ -86,7 +86,7 @@

Make available {% endif %} - {% elif shift.people_required > shift.shifts_available %} + {% elif shift.people_required > shift.shifts_taken %} Assign me From 6812b26ba5c771b22bf1601f1a41ccb6037dd251 Mon Sep 17 00:00:00 2001 From: Thomas Steen Rasmussen Date: Sun, 22 Jun 2025 15:41:35 +0200 Subject: [PATCH 09/12] Update src/teams/views/shifts.py --- src/teams/views/shifts.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/teams/views/shifts.py b/src/teams/views/shifts.py index 8c0e6c12b..13cccbb74 100644 --- a/src/teams/views/shifts.py +++ b/src/teams/views/shifts.py @@ -407,7 +407,6 @@ class MemberSellsShift(LoginRequiredMixin, CampViewMixin, UpdateView): template_name = "team_shift_confirm_action.html" context_object_name = "shifts" active_menu = "shifts" - """View for making a shift available for others shift.""" def get_context_data(self, **kwargs) -> dict[str, object]: """Method for setting the page context data.""" From ce8985b8a0e2d7ed3ba16f43d8e9bcd5aa9ba044 Mon Sep 17 00:00:00 2001 From: "Rudy (zarya)" Date: Sun, 22 Jun 2025 19:41:49 +0200 Subject: [PATCH 10/12] Add updated_at and sort so oldest is picked first --- .../0069_teamshiftassignment_updated_at.py | 18 ++++++++++++++++++ src/teams/models.py | 2 ++ src/teams/views/shifts.py | 17 ++++------------- 3 files changed, 24 insertions(+), 13 deletions(-) create mode 100644 src/teams/migrations/0069_teamshiftassignment_updated_at.py diff --git a/src/teams/migrations/0069_teamshiftassignment_updated_at.py b/src/teams/migrations/0069_teamshiftassignment_updated_at.py new file mode 100644 index 000000000..7f0353772 --- /dev/null +++ b/src/teams/migrations/0069_teamshiftassignment_updated_at.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.21 on 2025-06-22 17:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0068_alter_teamshift_team_members'), + ] + + operations = [ + migrations.AddField( + model_name='teamshiftassignment', + name='updated_at', + field=models.DateTimeField(auto_now=True), + ), + ] diff --git a/src/teams/models.py b/src/teams/models.py index e51884ff4..d5c10343f 100644 --- a/src/teams/models.py +++ b/src/teams/models.py @@ -573,6 +573,8 @@ class TeamShiftAssignment(CampRelatedModel): help_text="Is the shift assignment for sale?", ) + updated_at = models.DateTimeField(auto_now=True) + @property def camp(self) -> Camp: """All CampRelatedModels must have a camp FK or a camp property.""" diff --git a/src/teams/views/shifts.py b/src/teams/views/shifts.py index 13cccbb74..d0840cd91 100644 --- a/src/teams/views/shifts.py +++ b/src/teams/views/shifts.py @@ -324,10 +324,7 @@ def get_context_data(self, **kwargs) -> dict[str, object]: """Method for setting the page context data.""" context = super().get_context_data(**kwargs) context['action'] = f"Are you sure you want to take this {self.object}?" - context["team"] = Team.objects.get( - camp=self.camp, - slug=self.kwargs["team_slug"], - ) + context["team"] = self.object.team return context def form_valid(self, form: ModelForm[TeamShift]) -> HttpResponseRedirect: @@ -359,7 +356,7 @@ def form_valid(self, form: ModelForm[TeamShift]) -> HttpResponseRedirect: else: # Remove at most one shift assignment for sale if any # When a shift is for sale and a user presses assign its first assigning the for sale one - for shift_assignment in shift.team_members.filter(teamshiftassignment__for_sale=True)[:1]: + for shift_assignment in shift.team_members.filter(teamshiftassignment__for_sale=True).order_by('teamshiftassignment__updated_at')[:1]: shift.team_members.remove(shift_assignment) shift.team_members.add(team_member) @@ -380,10 +377,7 @@ def get_context_data(self, **kwargs) -> dict[str, object]: """Method for setting the page context data.""" context = super().get_context_data(**kwargs) context['action'] = f"Are you sure you want to drop this {self.object}?" - context["team"] = Team.objects.get( - camp=self.camp, - slug=self.kwargs["team_slug"], - ) + context["team"] = self.object.team return context def form_valid(self, form: ModelForm[TeamShift]) -> HttpResponseRedirect: @@ -412,10 +406,7 @@ def get_context_data(self, **kwargs) -> dict[str, object]: """Method for setting the page context data.""" context = super().get_context_data(**kwargs) context['action'] = f"Are you sure you want to this {self.object} available to others?" - context["team"] = Team.objects.get( - camp=self.camp, - slug=self.kwargs["team_slug"], - ) + context["team"] = self.object.team return context http_methods = ("get",) From 3cc4e7e85193a028ed8945a49d8cfd81ad72e582 Mon Sep 17 00:00:00 2001 From: "Rudy (zarya)" Date: Fri, 27 Jun 2025 07:41:26 +0200 Subject: [PATCH 11/12] Add ensure team member mixin to all shift actions --- src/teams/tests/test_shift_views.py | 29 +++++++++++++++++++++++++++++ src/teams/views/mixins.py | 17 +++++++++++++++++ src/teams/views/shifts.py | 7 ++++--- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/teams/tests/test_shift_views.py b/src/teams/tests/test_shift_views.py index 5604c1cc7..86500024d 100644 --- a/src/teams/tests/test_shift_views.py +++ b/src/teams/tests/test_shift_views.py @@ -343,3 +343,32 @@ def test_team_shift_actions(self) -> None: rows = soup.select_one("table#main_table > tbody > tr:nth-of-type(1) td:nth-of-type(5)") matches = [s for s in rows if "Assign me" in str(s)] self.assertEqual(len(matches), 1, "team shift unassign failed") + + # Test taking a shift as a user not on this team + self.client.force_login(self.users[3]) # User not on the NOC team + url = reverse( + "teams:shift_member_take", + kwargs={ + "team_slug": team_shift_1.team.slug, + "camp_slug": self.camp.slug, + "pk": team_shift_1.pk, + }, + ) + response = self.client.get( + path=url, + follow=True + ) + assert response.status_code == 200 + soup = BeautifulSoup(content, "html.parser") + rows = soup.select("div.alert.alert-danger") + matches = [s for s in rows if "No thanks" in str(s)] + self.assertEqual(len(matches), 0, "team shift authorization failed") + response = self.client.post( + path=url, + follow=True, + ) + assert response.status_code == 200 + soup = BeautifulSoup(content, "html.parser") + rows = soup.select("div.alert.alert-danger") + matches = [s for s in rows if "No thanks" in str(s)] + self.assertEqual(len(matches), 0, "team shift authorization failed") diff --git a/src/teams/views/mixins.py b/src/teams/views/mixins.py index 2ffc3388f..b8ab1b0a6 100644 --- a/src/teams/views/mixins.py +++ b/src/teams/views/mixins.py @@ -33,6 +33,23 @@ def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: return super().dispatch(request, *args, **kwargs) +class EnsureTeamMemberMixin: + """Use to make sure request.user has team member permission for the team specified by kwargs['team_slug'].""" + + def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: + """Method to make sure request.user has team member permission for the team specified by kwargs['team_slug'].""" + self.team = Team.objects.get(slug=kwargs["team_slug"], camp=self.camp) + if self.team.member_permission_set not in request.user.get_all_permissions(): + messages.error(request, "No thanks") + return redirect( + "teams:general", + camp_slug=self.camp.slug, + team_slug=self.team.slug, + ) + + return super().dispatch(request, *args, **kwargs) + + class EnsureTeamMemberLeadMixin(SingleObjectMixin): """Use to make sure request.user has team lead permission for the team which TeamMember belongs to.""" diff --git a/src/teams/views/shifts.py b/src/teams/views/shifts.py index d0840cd91..fb34e3708 100644 --- a/src/teams/views/shifts.py +++ b/src/teams/views/shifts.py @@ -31,6 +31,7 @@ from utils.mixins import IsTeamPermContextMixin from .mixins import EnsureTeamLeadMixin +from .mixins import EnsureTeamMemberMixin if TYPE_CHECKING: from django.db.models import QuerySet @@ -312,7 +313,7 @@ def get_context_data(self, **kwargs) -> dict: return context -class MemberTakesShift(LoginRequiredMixin, CampViewMixin, UpdateView): +class MemberTakesShift(LoginRequiredMixin, CampViewMixin, EnsureTeamMemberMixin, UpdateView): """View for adding a user to a shift.""" model = TeamShift fields = [] @@ -365,7 +366,7 @@ def form_valid(self, form: ModelForm[TeamShift]) -> HttpResponseRedirect: return HttpResponseRedirect(reverse("teams:shifts", kwargs=self.kwargs)) -class MemberDropsShift(LoginRequiredMixin, CampViewMixin, UpdateView): +class MemberDropsShift(LoginRequiredMixin, CampViewMixin, EnsureTeamMemberMixin, UpdateView): model = TeamShift fields = [] template_name = "team_shift_confirm_action.html" @@ -394,7 +395,7 @@ def form_valid(self, form: ModelForm[TeamShift]) -> HttpResponseRedirect: return HttpResponseRedirect(reverse("teams:shifts", kwargs=self.kwargs)) -class MemberSellsShift(LoginRequiredMixin, CampViewMixin, UpdateView): +class MemberSellsShift(LoginRequiredMixin, CampViewMixin, EnsureTeamMemberMixin, UpdateView): """View for making a shift available for other user to take.""" model = TeamShift fields = [] From a068b2b42b05cfa1583928ae7237b30c1a135190 Mon Sep 17 00:00:00 2001 From: "Rudy (zarya)" Date: Fri, 27 Jun 2025 10:55:52 +0200 Subject: [PATCH 12/12] Improve tests to test for 403's --- src/teams/tests/test_shift_views.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/teams/tests/test_shift_views.py b/src/teams/tests/test_shift_views.py index 86500024d..03126edd0 100644 --- a/src/teams/tests/test_shift_views.py +++ b/src/teams/tests/test_shift_views.py @@ -208,6 +208,22 @@ def test_team_shift_views(self) -> None: rows = soup.select("table#main_table > tbody > tr") self.assertEqual(len(rows), 10, "team shift list does not return 10 entries after delete") + # Test creating a shift as a member not a lead. + self.client.force_login(self.users[1]) # Noc team member + url = reverse( + "teams:shift_create", + kwargs={ + "team_slug": self.teams["noc"].slug, + "camp_slug": self.camp.slug, + }, + ) + response = self.client.get(url, follow=True) + assert response.status_code == 200 + soup = BeautifulSoup(content, "html.parser") + rows = soup.select("div.alert.alert-danger") + matches = [s for s in rows if "No thanks" in str(s)] + self.assertEqual(len(matches), 0, "team shift create authorization failed incorrect") + def test_team_shift_actions(self) -> None: """Test the team shift actions.""" self.client.force_login(self.users[4]) # Noc teamlead @@ -362,7 +378,7 @@ def test_team_shift_actions(self) -> None: soup = BeautifulSoup(content, "html.parser") rows = soup.select("div.alert.alert-danger") matches = [s for s in rows if "No thanks" in str(s)] - self.assertEqual(len(matches), 0, "team shift authorization failed") + self.assertEqual(len(matches), 0, "team shift authorization failed incorrect") response = self.client.post( path=url, follow=True, @@ -371,4 +387,4 @@ def test_team_shift_actions(self) -> None: soup = BeautifulSoup(content, "html.parser") rows = soup.select("div.alert.alert-danger") matches = [s for s in rows if "No thanks" in str(s)] - self.assertEqual(len(matches), 0, "team shift authorization failed") + self.assertEqual(len(matches), 0, "team shift authorization failed incorrect")