From 07ac32aa9e3097c488358c830fa087e875b70f0b Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Sat, 14 Sep 2024 17:50:39 +0200 Subject: [PATCH] Mid Septemer Update (#848) * Feat(kontres)/add image to bookable item (#785) * added optional image to bookable item model * added update method in serializer to handle new images * linting * remove update method for images * Feat(kontres)/add approved by (#786) * added approved by field * endpoint will now set approved by * serializer will return full user object in approved_by_detail * created test for approved by * migration * remove unnecessary code * removed write-only field in approved-by context * Create minutes for Codex (#787) * init * format * Feat(minute)/viewset (#788) * added richer reponse on post and put * added to admin panel * added filter for minute * Feat(kontres)/add notification (#790) * created methods for sending notification to admin and user * endpoint will now send notification if needed * add migrations for new notification types * Memberships with fines activated (#791) init * Feat(user)/user bio (#758) * Created model, serializer and view for user-bio * Created user bio model and made migrations * Created user bio serializer + viewsets + added new endpoint * Tested create method + added bio serializer to user serializer * Format * Created update method and started testing * Debugging test failures in user retrieve * fixed model error * Created user_bio_factory + started testing put method * Created fixture for UserBio * Created custom excpetion for duplicate user bio * Added permissions and inherited from BaseModel * Modularized serializer for bio * Use correct serializers in viewset + added destroy method * Finished testing bio viewset integration + format * Changed environent file to .env to avoid pushing up keys * Fix: Flipped assertion statement in test, since user bio should not be deleted * skiped buggy test from kontres * added mark to pytest.skip * Moved keys to .env file and reverted docker variables * Skip buggy kontres test * format * Added str method to user_bio * Removed unused imports * format * Changed user relation to a OneToOne-field (same affect as ForeignKey(unique=True) + removed check for duplicate bio in serializer * Migrations + changed assertion status code in duplicate bio test (could try catch in serializer to produce 400 status code) * format * format * Changed limit for description 50 -> 500 + migrations * Migrate * added id to serializer * merged leaf nodes in migrations * format --------- Co-authored-by: Ester2109 <126612066+Ester2109@users.noreply.github.com> Co-authored-by: Mads Nylund Co-authored-by: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Co-authored-by: Tam Le * Update CHANGELOG.md * added filter for allowed photos for user (#794) added filter for allowed photos * Upped payment time when coming from waiting list (#796) * fixed paymenttime saved to db (#798) * fixed bug (#800) * Disallow users to unregister when payment is done (#802) added 400 status code for deleting paid registration * update changelog * Added serializer for category in event (#804) added serializer for category in event * Permission middelware (#806) * added a check for existing user and id on request * format * Permission refactor of QR Codes (#807) * added permissions to qr code and refactored viewset * format * removed unused imports * Permissions for payment orders (#808) * added read permissions * added permissions for payment order and tests * format * chore(iac): updated docs and force https (#810) chore: updated docs and force https * feat(iac): add terraform guardrails so index don't nuke our infra (#811) feat: add guardrails so index don't fup * Automatic registration for new users with Feide (#809) * started on feide registration endpoint * made endpoint for creating user with Feide * added test for parse group * finished * format * removes three years if in digtrans * changelog update * Feide env variables Terraform (#814) added feid env variables * added delete endpoint for file (#815) * added delete endpoint for file * Trigger Build * changed workflow to checkout v4 * changed from docker-compose to docker compose * Update CHANGELOG.md * format * format * fixed permission for committee leaders for group forms * updated csv for forms (#818) * Permission for group forms and news (#820) added permission for committees to create news, and all leaders of groups to create group forms * Update reservation_seralizer.py (#822) * Update reservation_seralizer.py * Fixed linting * Put a band aid on it *smack* * Removed blank line.. * ???? * Group ownership of Minutes (#847) * Refactor MinuteFactory to include group field, and added validation for checkin group access * added validation for POST request * Changed endpoint response (#846) * Changed endpoint response * Fixed test * Update test_reservation_integration.py --------- Co-authored-by: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> * updated changelog.md --------- Co-authored-by: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Co-authored-by: haruixu <114171733+haruixu@users.noreply.github.com> Co-authored-by: Ester2109 <126612066+Ester2109@users.noreply.github.com> Co-authored-by: Tam Le Co-authored-by: martcl Co-authored-by: Frikk Balder <33499052+MindChirp@users.noreply.github.com> --- CHANGELOG.md | 3 + app/common/enums.py | 16 ++ app/common/permissions.py | 18 ++ app/content/factories/minute_factory.py | 2 + app/content/migrations/0066_minute_group.py | 27 +++ .../migrations/0067_alter_minute_group.py | 28 +++ app/content/models/minute.py | 32 +++- app/content/serializers/minute.py | 20 +- app/content/views/minute.py | 18 +- app/kontres/views/reservation.py | 2 +- app/tests/content/test_minute_integration.py | 176 +++++++++++++++--- .../kontres/test_reservation_integration.py | 4 +- 12 files changed, 303 insertions(+), 43 deletions(-) create mode 100644 app/content/migrations/0066_minute_group.py create mode 100644 app/content/migrations/0067_alter_minute_group.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 94a8fb12..48850105 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ ## Neste versjon +## Versjon 2024.09.14 +- ⚡**Codex**. Det er nå et skille mellom dokumenter opprettet av Drift og Index. + ## Versjon 2024.08.21 - ⚡**Nyheter**. Ledere av komiteer kan nå opprette nyheter. - ⚡**Forms**. Alle ledere av grupper kan nå opprette gruppe spørreskjemaer. diff --git a/app/common/enums.py b/app/common/enums.py index 7e3bd389..15f8eb05 100644 --- a/app/common/enums.py +++ b/app/common/enums.py @@ -1,5 +1,7 @@ from enum import Enum +from django.db import models + from enumchoicefield import ChoiceEnum @@ -44,6 +46,7 @@ class Groups(ChoiceEnum): REDAKSJONEN = "Redaksjonen" FONDET = "Forvaltningsgruppen" PLASK = "Plask" + DRIFT = "Drift" class AppModel(ChoiceEnum): @@ -106,3 +109,16 @@ class StrikeEnum(ChoiceEnum): LATE = "LATE" BAD_BEHAVIOR = "BAD_BEHAVIOR" EVAL_FORM = "EVAL_FORM" + + +class CodexGroups(models.TextChoices): + DRIFT = "Drift" + INDEX = "Index" + + @classmethod + def all(cls) -> list: + return [cls.DRIFT, cls.INDEX] + + @classmethod + def reverse(cls) -> list: + return [cls.INDEX, cls.DRIFT] diff --git a/app/common/permissions.py b/app/common/permissions.py index a7aa169a..c830d196 100644 --- a/app/common/permissions.py +++ b/app/common/permissions.py @@ -63,6 +63,24 @@ def check_has_access(groups_with_access, request): return False +def check_has_full_access(groups_with_access: list[str], request): + """Check if user has access to all groups""" + set_user_id(request) + user = request.user + + if not user: + return False + + try: + groups = map(str, groups_with_access) + return user and user.memberships.filter( + group__slug__iregex=r"(" + "|".join(groups) + ")" + ).count() == len(groups_with_access) + except Exception as e: + capture_exception(e) + return False + + def set_user_id(request): # If the id and user of the request is already set, return if (hasattr(request, "id") and request.id) and ( diff --git a/app/content/factories/minute_factory.py b/app/content/factories/minute_factory.py index 84377af9..dbc811e8 100644 --- a/app/content/factories/minute_factory.py +++ b/app/content/factories/minute_factory.py @@ -3,6 +3,7 @@ from app.content.factories.user_factory import UserFactory from app.content.models.minute import Minute +from app.group.factories.group_factory import GroupFactory class MinuteFactory(DjangoModelFactory): @@ -12,3 +13,4 @@ class Meta: title = factory.Faker("sentence", nb_words=4) content = factory.Faker("text") author = factory.SubFactory(UserFactory) + group = factory.SubFactory(GroupFactory) diff --git a/app/content/migrations/0066_minute_group.py b/app/content/migrations/0066_minute_group.py new file mode 100644 index 00000000..cc51ea22 --- /dev/null +++ b/app/content/migrations/0066_minute_group.py @@ -0,0 +1,27 @@ +# Generated by Django 4.2.5 on 2024-09-12 14:16 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("group", "0018_fine_defense"), + ("content", "0065_merge_0060_minute_tag_0064_alter_userbio_description"), + ] + + operations = [ + migrations.AddField( + model_name="minute", + name="group", + field=models.ForeignKey( + blank=True, + default=None, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="meeting_minutes", + to="group.group", + ), + ), + ] diff --git a/app/content/migrations/0067_alter_minute_group.py b/app/content/migrations/0067_alter_minute_group.py new file mode 100644 index 00000000..7a8eecb2 --- /dev/null +++ b/app/content/migrations/0067_alter_minute_group.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.5 on 2024-09-13 06:11 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("group", "0018_fine_defense"), + ("content", "0066_minute_group"), + ] + + operations = [ + migrations.AlterField( + model_name="minute", + name="group", + field=models.ForeignKey( + blank=True, + choices=[("Drift", "Drift"), ("Index", "Index")], + default="Index", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="meeting_minutes", + to="group.group", + ), + ), + ] diff --git a/app/content/models/minute.py b/app/content/models/minute.py index 2aa8d26a..ea3939df 100644 --- a/app/content/models/minute.py +++ b/app/content/models/minute.py @@ -1,15 +1,16 @@ from django.db import models -from app.common.enums import AdminGroup -from app.common.permissions import BasePermissionModel +from app.common.enums import CodexGroups +from app.common.permissions import BasePermissionModel, check_has_access from app.content.enums import MinuteTagEnum from app.content.models.user import User +from app.group.models import Group from app.util.models import BaseModel class Minute(BaseModel, BasePermissionModel): - write_access = (AdminGroup.INDEX,) - read_access = (AdminGroup.INDEX,) + write_access = CodexGroups.all() + read_access = CodexGroups.all() title = models.CharField(max_length=200) content = models.TextField(default="", blank=True) @@ -24,6 +25,22 @@ class Minute(BaseModel, BasePermissionModel): on_delete=models.SET_NULL, related_name="meeting_minutes", ) + group = models.ForeignKey( + Group, + blank=True, + null=True, + default=CodexGroups.INDEX, + choices=CodexGroups.choices, + on_delete=models.SET_NULL, + related_name="meeting_minutes", + ) + + @classmethod + def has_create_permission(cls, request): + data = request.data + if "group" in data: + return check_has_access([data["group"]], request) + return False @classmethod def has_update_permission(cls, request): @@ -41,10 +58,13 @@ def has_object_read_permission(self, request): return self.has_read_permission(request) def has_object_update_permission(self, request): - return self.has_write_permission(request) + data = request.data + if "group" in data: + return check_has_access([data["group"]], request) + return False def has_object_destroy_permission(self, request): - return self.has_write_permission(request) + return check_has_access([self.group.slug], request) def has_object_retrieve_permission(self, request): return self.has_read_permission(request) diff --git a/app/content/serializers/minute.py b/app/content/serializers/minute.py index f490195d..78eeb3d8 100644 --- a/app/content/serializers/minute.py +++ b/app/content/serializers/minute.py @@ -1,6 +1,7 @@ from rest_framework import serializers from app.content.models import Minute, User +from app.group.serializers import SimpleGroupSerializer class SimpleUserSerializer(serializers.ModelSerializer): @@ -12,7 +13,7 @@ class Meta: class MinuteCreateSerializer(serializers.ModelSerializer): class Meta: model = Minute - fields = ("title", "content", "tag") + fields = ("title", "content", "tag", "group") def create(self, validated_data): author = self.context["request"].user @@ -22,16 +23,26 @@ def create(self, validated_data): class MinuteSerializer(serializers.ModelSerializer): author = SimpleUserSerializer(read_only=True) + group = SimpleGroupSerializer(read_only=True) class Meta: model = Minute - fields = ("id", "title", "content", "author", "created_at", "updated_at", "tag") + fields = ( + "id", + "title", + "content", + "author", + "created_at", + "updated_at", + "tag", + "group", + ) class MinuteUpdateSerializer(serializers.ModelSerializer): class Meta: model = Minute - fields = ("id", "title", "content", "tag") + fields = ("id", "title", "content", "tag", "group") def update(self, instance, validated_data): return super().update(instance, validated_data) @@ -39,7 +50,8 @@ def update(self, instance, validated_data): class MinuteListSerializer(serializers.ModelSerializer): author = SimpleUserSerializer(read_only=True) + group = SimpleGroupSerializer(read_only=True) class Meta: model = Minute - fields = ("id", "title", "author", "created_at", "updated_at", "tag") + fields = ("id", "title", "author", "created_at", "updated_at", "tag", "group") diff --git a/app/content/views/minute.py b/app/content/views/minute.py index 3cc14914..8ebef660 100644 --- a/app/content/views/minute.py +++ b/app/content/views/minute.py @@ -1,9 +1,15 @@ from django_filters.rest_framework import DjangoFilterBackend from rest_framework import filters, status +from rest_framework.exceptions import NotFound from rest_framework.response import Response +from app.common.enums import CodexGroups from app.common.pagination import BasePagination -from app.common.permissions import BasicViewPermission +from app.common.permissions import ( + BasicViewPermission, + check_has_access, + check_has_full_access, +) from app.common.viewsets import BaseViewSet from app.content.filters import MinuteFilter from app.content.models import Minute @@ -30,6 +36,16 @@ class MinuteViewSet(BaseViewSet): "author__user_id", ] + def get_queryset(self): + if check_has_full_access(CodexGroups.all(), self.request): + return self.queryset + elif check_has_access([CodexGroups.DRIFT], self.request): + return self.queryset.filter(group=CodexGroups.DRIFT) + elif check_has_access([CodexGroups.INDEX], self.request): + return self.queryset.filter(group=CodexGroups.INDEX) + + raise NotFound() + def get_serializer_class(self): if hasattr(self, "action") and self.action == "list": return MinuteListSerializer diff --git a/app/kontres/views/reservation.py b/app/kontres/views/reservation.py index 1a0165fd..f15e7b44 100644 --- a/app/kontres/views/reservation.py +++ b/app/kontres/views/reservation.py @@ -85,5 +85,5 @@ def update(self, request, *args, **kwargs): def destroy(self, request, *args, **kwargs): super().destroy(self, request, *args, **kwargs) return Response( - {"detail": "Reservasjonen ble slettet."}, status=status.HTTP_204_NO_CONTENT + {"detail": "Reservasjonen ble slettet."}, status=status.HTTP_200_OK ) diff --git a/app/tests/content/test_minute_integration.py b/app/tests/content/test_minute_integration.py index a0b92573..3a6c9844 100644 --- a/app/tests/content/test_minute_integration.py +++ b/app/tests/content/test_minute_integration.py @@ -2,7 +2,10 @@ import pytest -from app.util.test_utils import get_api_client +from app.common.enums import CodexGroups +from app.content.factories import MinuteFactory +from app.group.models import Group +from app.util.test_utils import add_user_to_group_with_name, get_api_client API_MINUTE_BASE_URL = "/minutes/" @@ -11,12 +14,20 @@ def get_minute_detail_url(minute): return f"{API_MINUTE_BASE_URL}{minute.id}/" -def get_minute_post_data(): - return {"title": "Test Minute", "content": "This is a test minute."} +def get_minute_post_data(group=None): + return { + "title": "Test Minute", + "content": "This is a test minute.", + "group": group.slug if group else None, + } -def get_minute_put_data(): - return {"title": "Test Minute update", "content": "This is a test minute update."} +def get_minute_put_data(group=None): + return { + "title": "Test Minute update", + "content": "This is a test minute update.", + "group": group.slug if group else None, + } @pytest.mark.django_db @@ -31,41 +42,111 @@ def test_create_minute_as_member(member): @pytest.mark.django_db -def test_create_minute_as_index_member(index_member): +@pytest.mark.parametrize("codex_group", CodexGroups.all()) +def test_create_minute_as_codex_member(member, codex_group): """An index member should be able to create a minute""" url = API_MINUTE_BASE_URL - client = get_api_client(user=index_member) - data = get_minute_post_data() + add_user_to_group_with_name(member, codex_group) + client = get_api_client(user=member) + group = Group.objects.get(slug=codex_group) + data = get_minute_post_data(group) response = client.post(url, data) assert response.status_code == status.HTTP_201_CREATED +@pytest.mark.django_db +@pytest.mark.parametrize( + ("codex_group", "swap"), (CodexGroups.all(), CodexGroups.reverse()) +) +def test_create_to_another_group_as_codex_member(member, codex_group, swap): + """A codex member should not be able to create a minute to another group""" + add_user_to_group_with_name(member, codex_group) + + url = API_MINUTE_BASE_URL + client = get_api_client(user=member) + group = Group.objects.get_or_create(slug=swap, name=swap)[0] + data = get_minute_post_data(group) + response = client.post(url, data) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + @pytest.mark.django_db def test_update_minute_as_member(member, minute): """A member should not be able to update a minute""" url = get_minute_detail_url(minute) client = get_api_client(user=member) - data = get_minute_put_data() + index = Group.objects.get_or_create(slug="index", name="index")[0] + data = get_minute_put_data(index) response = client.put(url, data) assert response.status_code == status.HTTP_403_FORBIDDEN @pytest.mark.django_db -def test_update_minute_as_index_member(index_member, minute): - """An index member should be able to update a minute""" - minute.author = index_member +@pytest.mark.parametrize( + ("codex_group", "swap"), (CodexGroups.all(), CodexGroups.reverse()) +) +def test_update_to_another_group_as_codex_member(member, minute, codex_group, swap): + """A codex member should not be able to update a minute to another group""" + add_user_to_group_with_name(member, codex_group) + group = Group.objects.get(slug=codex_group) + + minute.author = member + minute.group = group minute.save() + url = get_minute_detail_url(minute) - client = get_api_client(user=index_member) - data = get_minute_put_data() + client = get_api_client(user=member) + + swap_group = Group.objects.get_or_create(slug=swap, name=swap)[0] + + data = get_minute_put_data(swap_group) + response = client.put(url, data) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +@pytest.mark.parametrize("codex_group", CodexGroups.all()) +def test_update_own_group_minute_as_codex_member(member, minute, codex_group): + """A codex member should be able to update a minute that belongs to their group""" + add_user_to_group_with_name(member, codex_group) + group = Group.objects.get(slug=codex_group) + + minute.author = member + minute.group = group + minute.save() + + url = get_minute_detail_url(minute) + client = get_api_client(user=member) + data = get_minute_put_data(group) response = client.put(url, data) assert response.status_code == status.HTTP_200_OK assert response.data["title"] == data["title"] +@pytest.mark.django_db +@pytest.mark.parametrize( + ("codex_group", "swap"), (CodexGroups.all(), CodexGroups.reverse()) +) +def test_update_another_group_minute_as_codex_member(member, minute, codex_group, swap): + """A codex member should not be able to update a minute that belongs to another group""" + add_user_to_group_with_name(member, codex_group) + + minute.group = Group.objects.get_or_create(slug=swap, name=swap)[0] + minute.save() + + url = get_minute_detail_url(minute) + client = get_api_client(user=member) + data = get_minute_put_data() + response = client.put(url, data) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + @pytest.mark.django_db def test_delete_minute_as_member(member, minute): """A member should not be able to delete a minute""" @@ -77,17 +158,40 @@ def test_delete_minute_as_member(member, minute): @pytest.mark.django_db -def test_delete_minute_as_index_member(index_member, minute): - """An index member should be able to delete a minute""" - minute.author = index_member +@pytest.mark.parametrize("codex_group", CodexGroups.all()) +def test_delete_minute_as_codex_member(member, minute, codex_group): + """A codex member should be able to delete a minute""" + add_user_to_group_with_name(member, codex_group) + + minute.author = member + minute.group = Group.objects.get(slug=codex_group) minute.save() + url = get_minute_detail_url(minute) - client = get_api_client(user=index_member) + client = get_api_client(user=member) response = client.delete(url) assert response.status_code == status.HTTP_200_OK +@pytest.mark.django_db +@pytest.mark.parametrize( + ("codex_group", "swap"), (CodexGroups.all(), CodexGroups.reverse()) +) +def test_delete_another_group_minute_as_codex_member(member, minute, codex_group, swap): + """A codex member should not be able to delete a minute that belongs to another group""" + add_user_to_group_with_name(member, codex_group) + + minute.group = Group.objects.get_or_create(slug=swap, name=swap)[0] + minute.save() + + url = get_minute_detail_url(minute) + client = get_api_client(user=member) + response = client.delete(url) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + @pytest.mark.django_db def test_list_minutes_as_member(member): """A member should not be able to list minutes""" @@ -99,15 +203,25 @@ def test_list_minutes_as_member(member): @pytest.mark.django_db -def test_list_minutes_as_index_member(index_member, minute): - """An index member should be able to list minutes""" - minute.author = index_member - minute.save() +@pytest.mark.parametrize("codex_group", CodexGroups.all()) +def test_list_minutes_as_codex_member(member, codex_group): + """A codex member should be able to list minutes from their own group""" + add_user_to_group_with_name(member, codex_group) + group = Group.objects.get(slug=codex_group) + + MinuteFactory.create_batch(5, group=group) + MinuteFactory.create_batch(5) + url = API_MINUTE_BASE_URL - client = get_api_client(user=index_member) + client = get_api_client(user=member) response = client.get(url) + count = response.data["count"] + results = response.data["results"] + assert response.status_code == status.HTTP_200_OK + assert count == 5 + assert all([minute["group"]["slug"] == codex_group.lower() for minute in results]) @pytest.mark.django_db @@ -121,13 +235,17 @@ def test_retrieve_minute_as_member(member, minute): @pytest.mark.django_db -def test_retrieve_minute_as_index_member(index_member, minute): - """An index member should be able to retrieve a minute""" - minute.author = index_member - minute.save() +@pytest.mark.parametrize("codex_group", CodexGroups.all()) +def test_retrieve_minute_as_codex_member(member, codex_group): + """A codex member should be able to retrieve a minute from their own group""" + add_user_to_group_with_name(member, codex_group) + group = Group.objects.get(slug=codex_group) + + minute = MinuteFactory(group=group) + url = get_minute_detail_url(minute) - client = get_api_client(user=index_member) + client = get_api_client(user=member) response = client.get(url) assert response.status_code == status.HTTP_200_OK - assert response.data["id"] == minute.id + assert response.data["group"]["slug"] == codex_group.lower() diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py index 81ffaae6..0ccdc82b 100644 --- a/app/tests/kontres/test_reservation_integration.py +++ b/app/tests/kontres/test_reservation_integration.py @@ -160,7 +160,7 @@ def test_member_deleting_own_reservation(member, reservation): reservation.save() client = get_api_client(user=member) response = client.delete(f"/kontres/reservations/{reservation.id}/", format="json") - assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.status_code == status.HTTP_200_OK assert response.data["detail"] == "Reservasjonen ble slettet." @@ -394,7 +394,7 @@ def test_admin_can_delete_any_reservation(admin_user, reservation): f"/kontres/reservations/{reservation.id}/", format="json", ) - assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.status_code == status.HTTP_200_OK @pytest.mark.django_db